Discussion:
bug#25633: porting gzip to Visual Studio 2015 failed due to redesign of CRT
(too old to reply)
Eric Blake
2017-02-06 18:05:49 UTC
Permalink
Raw Message
[adding gnulib]
Hi,
I tried to compile gzip with visual studio 2015. Unfortunately, a few files could not be ported. Microsoft has redesigned the core CRT which affects the visibility of (hidden/internals) of e.g. the FILE type. None of the internals of the FILE type is not visible anymore (contrary to Visual Studio 2013 and before). This affects e.g. freadingc, fpurge.c, fseeko.c, fseterr.c.
If something else than gcc/glibc is used, then each change - up to everything that the OS/libc/compiler vendor found as useful - may break gzip. Is it not bad programming practice to do so?
It may be undesirable programming practice, but it beats the alternative
of not compiling at all, and is forced upon us because we WANT access to
these useful low-level details of FILE streams for optimal behavior,
even though those details are not standardized and therefore not
universally available without resorting to peeking inside the struct.
1. why does gzip use and rely on these internals?
gzip is just using gnulib code; but gnulib does it because there are
some algorithms that really do require more information about the
current state of the FILE's buffers than what you can get through just
the standardized putc/getc interfaces.

If you can help port gnulib to the newer visual studio 2015, I'm sure
patches are welcome.
2. Is it intended that gzip should NOT compile on anything else where gcc/glibc is not used?
No; gnulib tries to make the various FILE manipulation functions
available on as many platform/compiler combinations as possible. It's
sometimes an arms race when platforms change, but we welcome patches to
catch back up to the current state of things.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Bruno Haible
2017-02-06 20:00:48 UTC
Permalink
Raw Message
Post by Eric Blake
I tried to compile gzip with visual studio 2015. Unfortunately, a few files
could not be ported. Microsoft has redesigned the core CRT which affects
the visibility of (hidden/internals) of e.g. the FILE type. None of the
internals of the FILE type is not visible anymore (contrary to Visual
Studio 2013 and before). This affects e.g. freadingc, fpurge.c, fseeko.c,
fseterr.c.
gnulib has been updated to work with this platform on 2016-12-13, see
http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=commit;h=5506db6b006274762bf5ea1d23feb7a9801529c8
Post by Eric Blake
If you can help port gnulib to the newer visual studio 2015, I'm sure
patches are welcome.
...
we welcome patches to catch back up to the current state of things.
While in general this is a good advise, in this case it's not needed. All
the reporter needs is a new gzip snapshot tarball that uses gnulib version
2016-12-14 or newer.

Bruno
Bruno Haible
2017-02-07 13:08:56 UTC
Permalink
Raw Message
Hello Kees,
If you like, I can share some additional changes I've made, e.g. Visual Studio
does not like -g flag, a WIN32 define that should be _WIN32 (a CL.exe built-in
macro), a workaround for missing #include_next directive (which is not
supported by Visual Studio), a struct that is in sys/time.h instead of time.h
(utimens.c). Please let me know the preferred format of changed. Is a UNIX
style patch ok (using gzip-1.8.18-00e6 as reference)?
Most of these troubles should go away if you use the 'compile' and 'ar-lib'
auxiliary scripts, as described in section 2 of
http://git.savannah.gnu.org/gitweb/?p=gperf.git;a=blob_plain;f=README.windows

These instructions were tested with GNU gperf and a couple of other GNU
packages. They should be applicable to GNU gzip as well.
Background: I tried to get the Visual Studio build working by performing the
a. open a Visual Studio command prompt (that have set LIB, INCLUDE etc
environment variables)
b. from within this prompt, start Cygwin shell
c. run CC=cl.exe ./configure
It is in this step 'c' that the 'compile' auxiliary script becomes useful.

Bruno
Eric Blake
2017-02-07 14:24:31 UTC
Permalink
Raw Message
Based on what I found today: the changes at low-level stdio in the FILE struct are now complable (I did not have yet a chance to let pass gzip our own internal tests, will do so later today or tomorrow).
1. frexp.c is not needed, as Visual Studio already provides frexp() function via system libraries.
But probably with bugs. And even if it is not needed on your platform,
it is part of the tarball to replace broken frexp() on systems where it
is buggy. Part of configure determines if it is needed on your platform.
2. memchr.c is not needed, as Visual Studio already provides memchr() function via system libraries.
Likewise.
3. gzip.c/h, util.c: the strlwr() function conflicts with Visual Studio one. Added HAVE_STRLWR define in config.h and omit implementation + prototype.
4. lseek.c: need to include winsock2.h before including windows.h
5. utimens.c: need to include sys/times.c on Windows to get struct utimbuf + need to define HAVE_STRUCT_UTIMBUF in config.h +
6. unzip.c: added xalloc.h to have consistent prototype for xalloc-die() (and removed this prototype from gzip.h). See also #9 which probably explains why a prototype exists in two places? Anyhow, util.c already included xalloc.h.
7. config.h (stored in clearcase as config_win32.h): added typedefs for uid_t and gid_t.
8. tailor.h/utimes.c: HAVE_SYS_UTIME_H is defined, but not used. Should this not become part of configure and add HAVE_SYS_UTIME_H define in config.h + adjust code in utimens.c?
9. util.c/xalloc-die.c: both contain a xalloc_die() function. I guess this is intentionally?
For t#1, #2, #3, #5, #7, #8: I lack the knowledge to adjust configure in such way that correct defines in config.h are generated. Also the source code need to be adjusted (rely on system headers instead of own prototypes if there is a working 'system' counterpart. I added myself HAVE_STRLWR in config.h.
I'm less certain on the solutions for the others, although patches to
gnulib are welcome.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Kees Dekker
2017-02-08 16:03:10 UTC
Permalink
Raw Message
Post by Eric Blake
1. frexp.c is not needed, as Visual Studio already provides frexp() function via system libraries.
But probably with bugs. And even if it is not needed on your platform,
it is part of the tarball to replace broken frexp() on systems where it
is buggy. Part of configure determines if it is needed on your platform.
Possibly, but why is the chance on bugs on Microsoft's frexp() higher? The only (unfortunately common) practice of Microsoft is that they may differ from 'standard' behavior on Linux.
Post by Eric Blake
I'm less certain on the solutions for the others, although patches to
gnulib are welcome.
I'm not sure whether I've the knowledge to do so, but today I tried to get gdiff also compilable in Cygwin shell (with all required environment variables used by Visual Studio's cl.exe properly set) + CC=cl.exe ./configure
And funny enough, I'm recognizing things. Gdiff also uses gnulib. A piece of the output is:

checking absolute name of <wchar.h>... ""

So if a system header is not found, it expands to an empty string. Because Microsoft Visual Studio does not recognize the include_next pragma, I used the following trick:
The #include[_next] "" directive was replaced by #include "../ucrt/headerfile". This works well for the Windows 10 SDK. If an older Visual Studion version is used, use #include "../include/headerfile", where <headerfile> can be replaced by any system header (e.g. wchar.h or sys/types.h).

What is the recommended to get this behavior changed? It requires changes in configure to detect whether a ‘dotdot’ construct works for visual studio (and only of compiler matches with cl[.exe]) and if the header indeed exists (Windows has e.g. no dirent.h and much other UNIX style headers).
Bruno Haible
2017-02-08 22:44:45 UTC
Permalink
Raw Message
Hi Kees,
1. frexp.c is not needed, as Visual Studio already provides frexp() function via system libraries.
The configure log says:
checking whether frexp works... guessing no
...
checking whether frexpl works... guessing no

Why "guessing"? Apparently AC_RUN_IFELSE refuses to run the
test program when compiling to mingw on a Cygwin host. Argh.

But it's documented in doc/posix-functions/frexp.texi
https://www.gnu.org/software/gnulib/manual/html_node/frexp.html

Do you have data that shows that MSVC14's frexp() behaves better than
the one in MSVC 9?
2. memchr.c is not needed, as Visual Studio already provides memchr() function via system libraries.
The configure log says:
checking whether memchr works... guessing no
Again the AC_RUN_IFELSE problem.
3. gzip.c/h, util.c: the strlwr() function conflicts with Visual Studio one. Added HAVE_STRLWR define in config.h and omit implementation + prototype.
That makes you depend on what Microsoft happens to implement in its
library. https://msdn.microsoft.com/en-us/library/ms235455.aspx
4. lseek.c: need to include winsock2.h before including windows.h
I'm not observing this problem with building this gzip snapshot.
5. utimens.c: need to include sys/times.c on Windows to get struct utimbuf + need to define HAVE_STRUCT_UTIMBUF in config.h +
Likewise: I'm not observing this problem.
7. config.h (stored in clearcase as config_win32.h): added typedefs for uid_t and gid_t.
config.h is autogenerated with my README.windows instructions.
9. util.c/xalloc-die.c: both contain a xalloc_die() function. I guess this is intentionally?
The one in util.c is meant to override the one from gnulib. This is normally
achieved by having the gnulib code linked as a library.

My build attempt aborted like this:
C:\cygwin64\home\bruno\gzip-1.8.18-00e6\gzip.c(1900): error C2146: syntax error: missing ')' before identifier 'uid'
C:\cygwin64\home\bruno\gzip-1.8.18-00e6\gzip.c(1900): error C2081: 'uid_t': name in formal parameter list illegal
C:\cygwin64\home\bruno\gzip-1.8.18-00e6\gzip.c(1900): error C2061: syntax error: identifier 'uid'
Indeed all uses of uid_t and gid_t need to be revisited when porting to native Windows.

Bruno
Kees Dekker
2017-02-09 07:36:28 UTC
Permalink
Raw Message
Hi Bruno,

Thanks for reply.
1. frexp.c is not needed, as Visual Studio already provides frexp() function via system libraries.
...
Do you have data that shows that MSVC14's frexp() behaves better than the one in MSVC 9?
Do you have advice how I can check this quickly? Visual Studio 2015 is VC14, so may be 5 versions ahead, something may be fixed? If you have a test code, I will check for you.
4. lseek.c: need to include winsock2.h before including windows.h
I'm not observing this problem with building this gzip snapshot.
I have used configure to let it generate the required headers, but finally, all was imported in a visual studio project, so my final compile is using visual studio itself (and therefore may be using slightly different defaults, as I use some base property sheets to globally define some settings, e.g. whole program optimization for optimized builds and some other defines and settings).
5. utimens.c: need to include sys/times.c on Windows to get struct utimbuf + need to define HAVE_STRUCT_UTIMBUF in config.h +
Likewise: I'm not observing this problem.
Probably same cause, due to my native visual studio compilation.
7. config.h (stored in clearcase as config_win32.h): added typedefs for uid_t and gid_t.
config.h is autogenerated with my README.windows instructions.
There is no README.windows file in the tar.gz file of the gzip sources, so it would be very handy to add it there. This email chain told me that there is something like README.windows for gnulib....
9. util.c/xalloc-die.c: both contain a xalloc_die() function. I guess this is intentionally?
The one in util.c is meant to override the one from gnulib. This is normally achieved by having the gnulib code linked as a library.
But it IMO still strange to have two different prototypes for a similar function (that got implemented in two places). If these are harmonized, then there is no problem. The order of including header files may now conflict with the implementation that comes first...

Kees

Kees Dekker
2017-02-07 07:54:21 UTC
Permalink
Raw Message
Are the mentioned patches in the snapshot that was provided by Jim?
In any case, I will check today. Thanks for prompt replies.
If you like, I can share some additional changes I've made, e.g. Visual Studio does not like -g flag, a WIN32 define that should be _WIN32 (a CL.exe built-in macro), a workaround for missing #include_next directive (which >is not supported by Visual Studio), a struct that is in sys/time.h instead of time.h (utimens.c). Please let me know the preferred format of changed. Is a UNIX style patch ok (using gzip-1.8.18-00e6 as reference)?
I'm not familiar enough with configure to be able to add 'auto-detect' rules for e.g. adding a HAVE_SYS_UTIME_H directive (needed in utimens.c to get struct utimbuf). The same applies to detect whether -g flag is (not) >supported by the used compiler.
Sorry, I forgot to tell one other issue (with the risk of confusing others):
Inconsistent usage in xalloc.h:56 about _Noreturn and gzip.h:325: ATTRIBUTE_NORETURN.
I would suggest to require to include xalloc.h if you need to use xalloc() or variants and not adding the same prototype in gzip.h. This requires to add xalloc.h in some places.
Visual Studio 2015 complains about different linkage if these two prototypes will be mixed.
a. open a Visual Studio command prompt (that have set LIB, INCLUDE etc environment variables)
b. from within this prompt, start Cygwin shell
c. run CC=cl.exe ./configure
d. import result into a visual studio project file (this step is only needed for us to incorporate the gzip build in our own build).
Regards,
Kees
Loading...