* Minor portability issues + fixes @ 2020-05-19 1:15 Daniel Richard G. 2020-05-19 2:49 ` Jeff King 2020-05-19 3:04 ` Carlo Marcelo Arenas Belón 0 siblings, 2 replies; 8+ messages in thread From: Daniel Richard G. @ 2020-05-19 1:15 UTC (permalink / raw) To: git Hello list, I am building Git 2.26.2 on AIX. A few compilation errors arose, but they are resolvable with a few minor changes that will improve overall portability. There were a few errors of this form: sha1-file.c: In function 'mmap_limit_check': sha1-file.c:940:12: error: 'SIZE_MAX' undeclared (first use in this function) sha1-file.c:940:12: note: each undeclared identifier is reported only once for each function it appears in SIZE_MAX is defined in stdint.h, and adding that #include fixes this. It is likely that this header is being pulled in on other platforms due to transitive dependencies, but that does not occur on AIX. The following files need #include<stdint.h>: sha1-file.c utf8.c vcs-svn/svndiff.c wrapper.c (I am not sure of the correct place to add a header like this; I have only verified that adding it to the .c file directly solves the problem.) Then, at link time, I saw this: rm -f xdiff/lib.a && ar rcs xdiff/lib.a xdiff/xdiffi.o xdiff/xprepare.o xdiff/xutils.o xdiff/xemit.o xdiff/xmerge.o xdiff/xpatience.o xdiff/xhistogram.o gcc -D_ALL_SOURCE -D_THREAD_SAFE -I. -D_LARGE_FILES -DGIT_HOST_CPU="\"00XXXXXXXXXX\"" -I/nfs/freeport/arch/aix32/include -DNO_CURL -I/nfs/freeport/arch/aix32/include -DNO_OPENSSL -DNO_D_TYPE_IN_DIRENT -DNO_NSEC -Dsockaddr_storage=sockaddr_in6 -DNO_ICONV -DOLD_ICONV -DSHA1_DC -DSHA1DC_NO_STANDARD_INCLUDES -DSHA1DC_INIT_SAFE_HASH_DEFAULT=0 -DSHA1DC_CUSTOM_INCLUDE_SHA1_C="\"cache.h\"" -DSHA1DC_CUSTOM_INCLUDE_UBC_CHECK_C="\"git-compat-util.h\"" -DSHA256_BLK -DNO_PTHREADS -DHAVE_PATHS_H -DHAVE_STRINGS_H -DHAVE_CLOCK_GETTIME -DSNPRINTF_RETURNS_BOGUS -DFREAD_READS_DIRECTORIES -DNO_STRCASESTR -DNO_STRLCPY -DNO_STRTOUMAX -DNO_SETENV -DNO_MKDTEMP -DNO_UNSETENV -DNO_PREAD -DNO_MEMMEM -DINTERNAL_QSORT -Icompat/regex -DFILENO_IS_A_MACRO -DNEED_ACCESS_ROOT_HANDLER -DDEFAULT_EDITOR='"nano"' -DDEFAULT_PAGER='"more"' -DSHELL_PATH='"/bin/sh"' -DPAGER_ENV='"LESS=FRX LV=-c"' -o git-credential-store credential-store.o common-main.o libgit.a xdiff/lib.a -L/nfs/freeport/arch/aix32/lib /nfs/freeport/arch/aix32/lib -L/nfs/freeport/arch/aix32/lib /nfs/freeport/arch/aix32/lib -lz -lintl ld: 0711-168 SEVERE ERROR: Input file: /nfs/freeport/arch/aix32/lib Input files must be regular files. collect2: error: ld returned 12 exit status Makefile:2456: recipe for target 'git-credential-store' failed gmake: *** [git-credential-store] Error 1 After some investigation, I found that this resulted from the following configure-time determination: configure:4774: WARNING: linker does not support runtime path to dynamic libraries (from line 488 of configure.ac) The problem is, in this case, CC_LD_DYNPATH is set to an empty value--- which is incorrect, in light of how it is used in the Makefile. Here is a typical example: ifdef ZLIB_PATH BASIC_CFLAGS += -I$(ZLIB_PATH)/include EXTLIBS += -L$(ZLIB_PATH)/$(lib) $(CC_LD_DYNPATH)$(ZLIB_PATH)/$(lib) endif EXTLIBS += -lz Setting that variable to an empty value causes a bare directory to be passed to the linker, which of course then errors out. I would suggest setting it to "-L" instead. (The libraries I am using are all static archives, so I am not hampered by a lack of runtime library paths.) --Daniel -- Daniel Richard G. || skunk@iSKUNK.ORG My ASCII-art .sig got a bad case of Times New Roman. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Minor portability issues + fixes 2020-05-19 1:15 Minor portability issues + fixes Daniel Richard G. @ 2020-05-19 2:49 ` Jeff King 2020-05-19 4:22 ` Daniel Richard G. 2020-05-19 3:04 ` Carlo Marcelo Arenas Belón 1 sibling, 1 reply; 8+ messages in thread From: Jeff King @ 2020-05-19 2:49 UTC (permalink / raw) To: Daniel Richard G.; +Cc: git On Mon, May 18, 2020 at 09:15:58PM -0400, Daniel Richard G. wrote: > I am building Git 2.26.2 on AIX. A few compilation errors arose, but > they are resolvable with a few minor changes that will improve overall > portability. > > There were a few errors of this form: > > sha1-file.c: In function 'mmap_limit_check': > sha1-file.c:940:12: error: 'SIZE_MAX' undeclared (first use in this function) > sha1-file.c:940:12: note: each undeclared identifier is reported only once for each function it appears in > > SIZE_MAX is defined in stdint.h, and adding that #include fixes this. It > is likely that this header is being pulled in on other platforms due to > transitive dependencies, but that does not occur on AIX. All system defines in Git should be pulled in via git-compat-util.h. That does include stdint.h, but only if NO_INTTYPES_H is defined (otherwise we prefer inttypes.h). And POSIX (2004) says: The <inttypes.h> header shall include the <stdint.h> header. But perhaps that is not so on AIX (it wouldn't be the first time we've seen a platform that does not strictly follow POSIX). Does building with: make NO_INTTYPES_H=YesPlease work? If so, then perhaps it should be added to the AIX defines in config.mak.uname. > The problem is, in this case, CC_LD_DYNPATH is set to an empty value--- > which is incorrect, in light of how it is used in the Makefile. Here is > a typical example: > > ifdef ZLIB_PATH > BASIC_CFLAGS += -I$(ZLIB_PATH)/include > EXTLIBS += -L$(ZLIB_PATH)/$(lib) $(CC_LD_DYNPATH)$(ZLIB_PATH)/$(lib) > endif > EXTLIBS += -lz > > Setting that variable to an empty value causes a bare directory to be > passed to the linker, which of course then errors out. I would suggest > setting it to "-L" instead. That would just be redundant with the earlier argument. That might be the easiest way to turn it into a noop, but we can probably do better with $(if) or similar, which would allow somebody to build with: make CC_LD_DYNPATH= even without using the autoconf script. I do wonder, though, if configure.ac could be extended for AIX to support whatever syntax the system linker uses for setting the run-path. I understand that you don't care either way about this feature, but this might be a good opportunity to fix it. -Peff ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Minor portability issues + fixes 2020-05-19 2:49 ` Jeff King @ 2020-05-19 4:22 ` Daniel Richard G. 2020-05-20 4:28 ` Jeff King 0 siblings, 1 reply; 8+ messages in thread From: Daniel Richard G. @ 2020-05-19 4:22 UTC (permalink / raw) To: Jeff King; +Cc: git Hi Jeff, On Mon, 2020 May 18 22:49-04:00, Jeff King wrote: > > All system defines in Git should be pulled in via git-compat-util.h. > That does include stdint.h, but only if NO_INTTYPES_H is defined > (otherwise we prefer inttypes.h). And POSIX (2004) says: > > The <inttypes.h> header shall include the <stdint.h> header. > > But perhaps that is not so on AIX (it wouldn't be the first time we've > seen a platform that does not strictly follow POSIX). On AIX 4.3, where I am building, inttypes.h does not #include stdint.h, and stdint.h is not present under /usr/include/. (This is an old system used for ABI compatibility-testing purposes.) This is not general to AIX, however, because on 5.3, both stdint.h and its #include are there. That said, my build is using GCC 4.7, which provides a modern stdint.h. The GCC tree contains a fixincluded version of inttypes.h, but this also lacks the <stdint.h> #include. > Does building with: > > make NO_INTTYPES_H=YesPlease > > work? > > If so, then perhaps it should be added to the AIX defines in > config.mak.uname. I've confirmed that this works. But would it not be safe to #include both inttypes.h and stdint.h explicitly if both are present, rather than cater to AIX specifically? I could see this similarly arising in, say, an old version of Solaris. > > Setting that variable to an empty value causes a bare directory to be > > passed to the linker, which of course then errors out. I would suggest > > setting it to "-L" instead. > > That would just be redundant with the earlier argument. That might be > the easiest way to turn it into a noop, but we can probably do better > with $(if) or similar, which would allow somebody to build with: > > make CC_LD_DYNPATH= > > even without using the autoconf script. That is a fair point. CC_LD_DYNPATH is used only a handful of times, so this wouldn't be much more work. > I do wonder, though, if configure.ac could be extended for AIX to > support whatever syntax the system linker uses for setting the run-path. > I understand that you don't care either way about this feature, but this > might be a good opportunity to fix it. This could be a bit complicated, I'm afraid. The applicable value for CC_LD_DYNPATH on AIX is "-Wl,-blibpath:". However, have a look at the description for this option in the ld(1) man page: libpath:Path Uses Path as the library path when writing the loader section of the output file. Path is neither checked for validity nor used when searching for libraries specified by the -l flag. Path overrides any library paths generated when the -L flag is used. If you do not specify any -L flags, or if you specify the nolibpath option, the default library path information is written in the loader section of the output file. The default library path information is the value of the LIBPATH environment variable if it is defined, and /usr/lib:/lib, otherwise. (excerpted from an AIX 7.1 system) That implies that when you use this option, you must specify not only the path you're interested in, but also /usr/lib:/lib! And when I look at an existing AIX build of ours, that makes use of Libtool, I see that it is in fact passing in a rather lengthy path for -Wl,-blibpath:, that ends with ":/usr/lib:/lib". --Daniel -- Daniel Richard G. || skunk@iSKUNK.ORG My ASCII-art .sig got a bad case of Times New Roman. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Minor portability issues + fixes 2020-05-19 4:22 ` Daniel Richard G. @ 2020-05-20 4:28 ` Jeff King 2020-05-21 4:29 ` Daniel Richard G. 0 siblings, 1 reply; 8+ messages in thread From: Jeff King @ 2020-05-20 4:28 UTC (permalink / raw) To: Daniel Richard G.; +Cc: git On Tue, May 19, 2020 at 12:22:00AM -0400, Daniel Richard G. wrote: > > If so, then perhaps it should be added to the AIX defines in > > config.mak.uname. > > I've confirmed that this works. But would it not be safe to #include > both inttypes.h and stdint.h explicitly if both are present, rather than > cater to AIX specifically? I could see this similarly arising in, say, > an old version of Solaris. Yes, as long as we check that both are present, I think that would do the right thing. And the autoconf check could cover that. I don't think there's an easy way to have a general Makefile knob that can check which files are present, though (and we generally consider the Makefile and its knobs to be the primary mechanism; autoconf to turn those knobs is generally implemented on top). So probably we'd want something like (in this order): - NEEDS_EXPLICIT_STDINT gets passed from the Makefile into the compiler via -D, which then triggers stdint.h being included unconditionally in git-compat-util.h - optionally set that in config.mak.uname for AIX (checking uname_R since it sounds like only old versions need it) - add an autoconf rule that sets it, either strictly (when a test-program decides it's needed) or loosely (when we see that it's available at all) Even just the first one would let you build by setting the knob manually; the rest is gravy on top, if you or somebody else chooses to do it. > > I do wonder, though, if configure.ac could be extended for AIX to > > support whatever syntax the system linker uses for setting the run-path. > > I understand that you don't care either way about this feature, but this > > might be a good opportunity to fix it. > > This could be a bit complicated, I'm afraid. > > The applicable value for CC_LD_DYNPATH on AIX is "-Wl,-blibpath:". > However, have a look at the description for this option in the > ld(1) man page: OK, gross. :) I agree it's not worth going too far into this rabbit hole. I do wonder if you could just be using GNU ld along with gcc, but maybe that's not practical. You should be able to build with: make CC_LD_DYNPATH=-L as a workaround, but it would be nice if the Makefile handled this correctly. It looks like CC_LD_DYNPATH gets used in a lot of places, so I suspect we'd want a Makefile function to help out. Something like: # usage: $(call linker_lib,PATH) # Create linker args for looking for libraries in PATH at both link-time # and run-time. linker_lib = -L$1 $(if $(CC_LD_DYNPATH),$(CC_LD_DYNPATH)$1) which would allow: EXTLIBS += $(call linker_lib,$(LIBPCREDIR)/$(lib)) etc. This would be our first foray into Makefile functions, but I think we've determined that most platforms have a recent enough GNU make for it to be OK (and we already require GNU make). -Peff ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Minor portability issues + fixes 2020-05-20 4:28 ` Jeff King @ 2020-05-21 4:29 ` Daniel Richard G. 2020-05-22 20:03 ` Jeff King 0 siblings, 1 reply; 8+ messages in thread From: Daniel Richard G. @ 2020-05-21 4:29 UTC (permalink / raw) To: Jeff King; +Cc: git On Wed, 2020 May 20 00:28-04:00, Jeff King wrote: > > > I've confirmed that this works. But would it not be safe to #include > > both inttypes.h and stdint.h explicitly if both are present, rather than > > cater to AIX specifically? I could see this similarly arising in, say, > > an old version of Solaris. > > Yes, as long as we check that both are present, I think that would do > the right thing. And the autoconf check could cover that. I don't think > there's an easy way to have a general Makefile knob that can check which > files are present, though (and we generally consider the Makefile and > its knobs to be the primary mechanism; autoconf to turn those knobs is > generally implemented on top). Okay, I see, so the whole phalanx of HAVE_BLAH_H symbols from Autoconf isn't available. > So probably we'd want something like (in this order): > > - NEEDS_EXPLICIT_STDINT gets passed from the Makefile into the > compiler via -D, which then triggers stdint.h being included > unconditionally in git-compat-util.h > > - optionally set that in config.mak.uname for AIX (checking uname_R > since it sounds like only old versions need it) > > - add an autoconf rule that sets it, either strictly (when a > test-program decides it's needed) or loosely (when we see that it's > available at all) > > Even just the first one would let you build by setting the knob > manually; the rest is gravy on top, if you or somebody else chooses to > do it. Hmm... that's a fairly specific knob, which I would think is less than ideal. The rest is reasonable, but would have to be written in terms of the knob. I can put something together, knowing that this is the approach you'd want to see, but it'll need some more work. Eventually, I'll need to get Git up and running on a few other old systems I have here, so that will undoubtedly figure into it. Thanks for sketching out how this should work, however; this is helpful to keep in mind. > > The applicable value for CC_LD_DYNPATH on AIX is "-Wl,-blibpath:". > > However, have a look at the description for this option in the > > ld(1) man page: > > OK, gross. :) I agree it's not worth going too far into this rabbit > hole. I do wonder if you could just be using GNU ld along with gcc, but > maybe that's not practical. Using the GNU linker can help in some cases, and hurt in others. We try to avoid it unless necessary. > You should be able to build with: > > make CC_LD_DYNPATH=-L > > as a workaround Yep, this works handily as a no-op :) > but it would be nice if the Makefile handled this correctly. It looks > like CC_LD_DYNPATH gets used in a lot of places, so I suspect we'd > want a Makefile function to help out. Something like: > > # usage: $(call linker_lib,PATH) > # Create linker args for looking for libraries in PATH at both link-time > # and run-time. > linker_lib = -L$1 $(if $(CC_LD_DYNPATH),$(CC_LD_DYNPATH)$1) > > which would allow: > > EXTLIBS += $(call linker_lib,$(LIBPCREDIR)/$(lib)) > > etc. This would be our first foray into Makefile functions, but I think > we've determined that most platforms have a recent enough GNU make for > it to be OK (and we already require GNU make). Why not just a variable to wrap the conditional? Something like CC_LD_DYNPATH_flag = $(if $(CC_LD_DYNPATH),$(CC_LD_DYNPATH),-L) ... EXTLIBS += -L$(ZLIB_PATH)/$(lib) $(CC_LD_DYNPATH_flag)$(ZLIB_PATH)/$(lib) Makefile functions are quite powerful, but they feel like a sledgehammer to this fly. (I've used them in the past to generate Make rules programmatically. It wasn't pretty, but it sure beat writing out makefile fragments and then include-ing them afterward!) --Daniel -- Daniel Richard G. || skunk@iSKUNK.ORG My ASCII-art .sig got a bad case of Times New Roman. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Minor portability issues + fixes 2020-05-21 4:29 ` Daniel Richard G. @ 2020-05-22 20:03 ` Jeff King 0 siblings, 0 replies; 8+ messages in thread From: Jeff King @ 2020-05-22 20:03 UTC (permalink / raw) To: Daniel Richard G.; +Cc: git On Thu, May 21, 2020 at 12:29:18AM -0400, Daniel Richard G. wrote: > > So probably we'd want something like (in this order): > > > > - NEEDS_EXPLICIT_STDINT gets passed from the Makefile into the > > compiler via -D, which then triggers stdint.h being included > > unconditionally in git-compat-util.h > > > > - optionally set that in config.mak.uname for AIX (checking uname_R > > since it sounds like only old versions need it) > > > > - add an autoconf rule that sets it, either strictly (when a > > test-program decides it's needed) or loosely (when we see that it's > > available at all) > > > > Even just the first one would let you build by setting the knob > > manually; the rest is gravy on top, if you or somebody else chooses to > > do it. > > Hmm... that's a fairly specific knob, which I would think is less than > ideal. The rest is reasonable, but would have to be written in terms > of the knob. Yeah, it is. I do wonder if just setting the existing NO_INTTYPES_H is a simpler solution for your case (since it sounds like that works). If you want to tweak the knob for older AIX in config.mak.uname, that would be fine (or if you think other people aren't likely to be building on the platform, maybe tweak your own "make" invocation and let this conversation serve to document for anybody else). > > but it would be nice if the Makefile handled this correctly. It looks > > like CC_LD_DYNPATH gets used in a lot of places, so I suspect we'd > > want a Makefile function to help out. Something like: > > > > # usage: $(call linker_lib,PATH) > > # Create linker args for looking for libraries in PATH at both link-time > > # and run-time. > > linker_lib = -L$1 $(if $(CC_LD_DYNPATH),$(CC_LD_DYNPATH)$1) > > > > which would allow: > > > > EXTLIBS += $(call linker_lib,$(LIBPCREDIR)/$(lib)) > > > > etc. This would be our first foray into Makefile functions, but I think > > we've determined that most platforms have a recent enough GNU make for > > it to be OK (and we already require GNU make). > > Why not just a variable to wrap the conditional? Something like > > CC_LD_DYNPATH_flag = $(if $(CC_LD_DYNPATH),$(CC_LD_DYNPATH),-L) > > ... > > EXTLIBS += -L$(ZLIB_PATH)/$(lib) $(CC_LD_DYNPATH_flag)$(ZLIB_PATH)/$(lib) I was trying to actually remove the argument, rather than turn it into a duplicate "-L". But you're probably right that it's not worth the effort. I think we could just normalize an empty CC_LD_DYNPATH to "-L" in the same variable, and then we wouldn't even have to change the sites which reference it. -Peff ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Minor portability issues + fixes 2020-05-19 1:15 Minor portability issues + fixes Daniel Richard G. 2020-05-19 2:49 ` Jeff King @ 2020-05-19 3:04 ` Carlo Marcelo Arenas Belón 2020-05-19 4:26 ` Daniel Richard G. 1 sibling, 1 reply; 8+ messages in thread From: Carlo Marcelo Arenas Belón @ 2020-05-19 3:04 UTC (permalink / raw) To: Daniel Richard G.; +Cc: git On Mon, May 18, 2020 at 09:15:58PM -0400, Daniel Richard G. wrote: > > I am building Git 2.26.2 on AIX. A few compilation errors arose, but > they are resolvable with a few minor changes that will improve overall > portability. which version of AIX is this? > There were a few errors of this form: > > sha1-file.c: In function 'mmap_limit_check': > sha1-file.c:940:12: error: 'SIZE_MAX' undeclared (first use in this function) > sha1-file.c:940:12: note: each undeclared identifier is reported only once for each function it appears in we include inttypes.h which is supposed to include stdint.h per POSIX[1] could you take a look at that header and see if there is some macro definition preventing that to happen? Carlo [1] https://pubs.opengroup.org/onlinepubs/009696799/basedefs/inttypes.h.html ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Minor portability issues + fixes 2020-05-19 3:04 ` Carlo Marcelo Arenas Belón @ 2020-05-19 4:26 ` Daniel Richard G. 0 siblings, 0 replies; 8+ messages in thread From: Daniel Richard G. @ 2020-05-19 4:26 UTC (permalink / raw) To: Carlo Marcelo Arenas Belón; +Cc: git Hi Carlo, On Mon, 2020 May 18 23:04-04:00, Carlo Marcelo Arenas Belón wrote: > On Mon, May 18, 2020 at 09:15:58PM -0400, Daniel Richard G. wrote: > > > > I am building Git 2.26.2 on AIX. A few compilation errors arose, but > > they are resolvable with a few minor changes that will improve overall > > portability. > > which version of AIX is this? 4.3, believe it or not :-] This system is used for compatibility testing of a legacy product. > we include inttypes.h which is supposed to include stdint.h per POSIX[1] > > could you take a look at that header and see if there is some macro definition > preventing that to happen? There's no #include at all, and stdint.h is not present under /usr/include/. But I am building with GCC 4.7 (as the original vendor compiler has outlived its usefulness), and that provides a modern stdint.h. The original inttypes.h is fixincluded by GCC, but this did not add the stdint.h #include. As I remarked to Jeff, would it be kosher to #include both inttypes.h and stdint.h if both are present? It seems that most of the time, the latter would be a no-op. --Daniel -- Daniel Richard G. || skunk@iSKUNK.ORG My ASCII-art .sig got a bad case of Times New Roman. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-05-22 20:03 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-19 1:15 Minor portability issues + fixes Daniel Richard G. 2020-05-19 2:49 ` Jeff King 2020-05-19 4:22 ` Daniel Richard G. 2020-05-20 4:28 ` Jeff King 2020-05-21 4:29 ` Daniel Richard G. 2020-05-22 20:03 ` Jeff King 2020-05-19 3:04 ` Carlo Marcelo Arenas Belón 2020-05-19 4:26 ` Daniel Richard G.
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).