tcl.h typedefs Tcl_StatBuf to struct stat/stat64 but does not include <sys/stat.h>
this leads to compilation error in tclCmdAH.c because the storage size of variables of type Tcl_StatBuf is unknown.
The unix-dependent includes come through tclUnixPort.h, included (through tclPort.h) by tclInt.h.
So it is not as plain as a failure to include an obviously needed system header, but rather some ifdeffery that fails in this specific case (that I cannot reproduce). Any specific info re the OS and/or configure/-D flags ?
this was experienced using musl libc 0.9.8 on i386
since musl always uses LARGEFILE64, it #defines stat64 stat
Please attach the Makefile generated by Tcl's configure in this environment.
tcl Makefile as generated on sabotage linux i386
here's the relevant header part in musl libc:
i added -D_GNU_SOURCE to the CFLAGS when launching configure, so this part got visible.
tcl.h takes the typedef struct stat64 Tcl_StatBuf route. however without seeing the macro in sys/stat.h this cannot succeed, because stat64 is only a macro.
So what ? Now your sys/stat.h is included, and stat64 gets mapped to stat, Tcl ends up needing a "struct stat" somewhere. I assume it is in <bits/stat.h> as in most unices, isn't it.
To make this exchange more efficient, please provide the generated Makefile without any tweaks.
Patching tcl.h or forcing CFLAGS sound brittle ; better fix the root problem in the configure logic.
Ok, just seen the attachment, sorry ;)
Still, there might be a side effect to -D_GNU_SOURCE. Please give the vanilla Makefile, the one lacking HAVE_SYS_STAT.
i never used any other Makefile
adding -D_GNU_SOURCE to CFLAGS is something i automatically do always
so it was not added specifically to circumvent some problems here.
this is the exact Makefile that causes the build failure.
musl's sys/stat.h includes an arch specific bits/stat.h that provides the actual definition of struct stat.
however it is not legal to include bits/stat.h directly, this is an internal header!
if you do this, that would explain the breakage.
looking at tclUnixPort.h, it appears to me that sys/stat.h should be included before any stat-related ifdeffery
there is code that defines things to be stat64, but since sys/stat.h is included later, the stat64 macro does not get expanded.
> it is not legal to include bits/stat.h directly,
> this is an internal header! -if you do this,
> that would explain the breakage
Ahem. Do you realize that this is the deed of (your/musl's) sys/stat.h ? Tcl just includes <sys/stat.h> just as everybody else.
>looking at tclUnixPort.h, it appears to me that sys/stat.h should be
>included before any stat-related ifdeffery
>there is code that defines things to be stat64, but since sys/stat.h is
>included later, the stat64 macro does not get expanded.
Red herring. This code is just a pair of prototypes, passing opaque pointers. Even if I agree that they don't benefit from the macro, there is no harm done beyond a possible warning.
If you want to proceed on this, please give the error message that you go when trying to compile with the vanilla Makefile generated by configure (not the one tweaked with whatever habits you have).
i mean this part here
# define TclOSstat stat64
# define TclOSlstat lstat64
# define TclOSstat stat
# define TclOSlstat lstat
the inclusion of stat.h comes later
so whatever this tries to do, it doesn't see the stat64 macro and thus may fail.
it is probably not related to the error i'm seeing.
however it is very likely that the problem we're facing here has the same cause:
tcl.h gets included before <sys/stat.h>, so the macro doesnt get expanded
and the compiler fails to find the struct stat64.
again i have not tweaked anything.
this is the Makefile that gets created when running CFLAGS=-D_GNU_SOURCE ./configure.
i never used anything else.
it is perfectly legal to define _GNU_SOURCE, the libc is free to do it automatically for example by defining it in features, and so am i.
since stat64 is completely non-standard, the implementation is free to define it as macro so you better make sure it works, by putting the inclusion of sys/stat.h before your typedefs.
here's the bug: tclPort.h
# include "tclUnixPort.h"
it should include tclUnixPort.h before tcl.h, problem solved
and for maximum portability put the include block in tclUnixport before any typedefs and macro definitions
Transferring to Jan Nijtmans, who did a similar move in a specific case (cygwin).
Jan, any comment on the side-effects you feared, so that you didn't generalize ?
(I'm referring to ad7cfb6ee3 where you moved tclWinPort above tcl.h, which looks like a good idea of wider applicability ;)
> Jan, any comment on the side-effects you feared, so that you didn't
> generalize ?
Well, that change was for fixing a compiler warming with mingw, not
for cygwin, but I agree with your analysis. It would be good to
include sys/stat.h before tcl.h. The problem with this is
the compat/*.h files, which currently need tcl.h to be
included first. On cygwin/linux/solaris those are not used,
so the move would be safe. But on older systems .....
Another remark: The inclusion order didn't change
between Tcl 8.5 and 8.6 (I can see in Makefile.tcl
that the error occurred in 8.6, but I am assuming
the same 'bug' is in 8.5 as well).
The best solution here might be to move the
sys/stat.h include from tcl*Port.h to tclInt.h.
That has the least probability to break
on any other platform.
rofl0r, Alex, would that work for you? Any
problems you can see with this change?
Sorry, but digging a platform-specific header out and sticking it on top of the main generic one doesn't sound like a solid basis... Instead, I'd rather understand in detail why some parts of tcl*Port (like the compat/ ones) depend on being after tcl.h, and either stop this dependency, or move them in a separate file, to be included after tcl.h
Can you please give these details (or just the kind of macros from tcl.h that are involed) ?
Only skimming the comments here. One thing to keep in mind
with any editing of the #include dependency graph is to preserve
the goals of the 8.5 era "header reform." See 922727.
Thanks, Don! But I'm fully aware of this header reform, and I
don't think that my latest suggestions violatest this:
I don't see any danger in this, but please put in your view!
thanks njitmans for picking this up. however your fix is only a workaround this specific issue, but it still leaves everything else broken.
this should be fixed properly by including all system headers before doing anything that depends on the types and other things defined in those headers, as ferrieux correctly noted.
njitmans: i think it would help if you could answer ferrieux' question:
"Can you please give these details (or just the kind of macros from tcl.h
that are involed) ?"