libusb @1.0.24: cannot parse USB.h with mainline gcc versions due to variant pragma handling not accepted by gcc

[dgonyier (Dwaine Gonyier)]

Summary of log output:

checking if /usr/bin/gcc-4.2 supports -std=gnu11... no
checking if /usr/bin/gcc-4.2 supports -std=c11... no
configure: error: compiler with C11 support is required to build libusb
Command failed:  cd "/opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_devel_libusb/libusb/work/libusb-1.0.24" && ./autogen.sh --prefix=/opt/local
Exit code: 1
Error: Failed to configure libusb, consult /opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_devel_libusb/libusb/work/libusb-1.0.24/config.log
Error: Failed to configure libusb: configure failure: command execution failed
DEBUG: Error code: NONE
DEBUG: Backtrace: configure failure: command execution failed
DEBUG:     while executing
DEBUG: "$procedure $targetname"
Error: See /opt/local/var/macports/logs/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_devel_libusb/libusb/main.log for details.

Hardware/OS:

$ port version
Version: 2.6.4

$ sw_vers
ProductName:    Mac OS X
ProductVersion: 10.5.8
BuildVersion:   9L31a

$ machine
ppc7450

$ sysctl hw.model
hw.model: PowerBook5,8

[dgonyier (Dwaine Gonyier)]

main.log

[dgonyier (Dwaine Gonyier)]

Build worked with @1.0.23

[ryandesign (Ryan Carsten Schmidt)]

We see the same error on the 10.7 buildbot worker: ​https://build.macports.org/builders/ports-10.7_x86_64-builder/builds/37525/steps/install-port/logs/stdio

And with libusb-devel also: ​https://build.macports.org/builders/ports-10.7_x86_64-builder/builds/37526/steps/install-port/logs/stdio

So I guess it needs configure.c_standard 2011.

[kencu (Ken)]

[879f32dff1d745bfdd0836b249932234d3f7d27d/macports-ports]

[kencu (Ken)]

buils is still broken with gcc7, on Tiger at least.

[michaelld (Michael Dickens)]

Thanks for getting the fix in place @kencu! I had one lined up & ready to commit but didn't find the time over the weekend to do so!

[michaelld (Michael Dickens)]

Houston, we have a problem! ... with libusb 1.0.24 on older OSX that is ... here's my findings from OSX 10.5.8 PPC ... will be roughly the same with any older OSX version:

1) libusb 1.0.24 requires C11 / GNU11 for the C standard. 1.0.23 did not. Here is the commit between 1.0.23 and 1.0.24: ​https://github.com/libusb/libusb/commit/9a1bc8cafb904c20a869c74ad6d657686a1c4bb1

2) IOKit on older OSX uses #pragma that requires Apple extension to GCC via apple-gcc-4.2 or the GCC that came with Xcode for this OSX, or Clang parsing; see e.g.: ​https://github.com/libusb/libusb/commit/9a1bc8cafb904c20a869c74ad6d657686a1c4bb1

3) GCC provides PPC support, while for Clang it is just whatever compiler came with Xcode from Apple; modern Clang does not yet support PPC & may never do so.

Combining: To get C11 / GNU11 support required by (1), we require a reasonably recent vanilla GCC per (3), but that GCC will not provide the #pragma required by IOKit per (2). We cannot modify the system framework header directly to fix the issue as suggested in (2).

... but ... maybe we could provide the fixed header in the legacy support headers and make sure it is searched before the IOKit one ... maybe? hmmmm ... that's worth a try IMHO; might work. I can't think of another option here beyond capping libusb to 1.0.23 for the affected systems. What do you think @kencu ?

[kencu (Ken)]

Indeed, I have pegged back my libusb to 1.0.23 while I found some time to sort this one out.

I'm not quite sure what exactly the #pragma issue is, and whether we can work around that...

[kencu (Ken)]

Here's the upstream bug report for mainline gcc. It's been an issue for 10 years:

<​https://gcc.gnu.org/bugzilla/show_bug.cgi?id=50909>

Hmmm. ... not 100% sure how to approach this.

We could modify the existing USB.h header on relevant systems (well all systems, to be honest, that try to use USB.h with mainline gcc will see this, but practically it happens on 10.4 and 10.5), and put the modified USB.h header in legacysupport, as per the workaround in that ticket:

ie in USB.h, change all the implementations of this

#pragma options align=reset

to

#if defined(__clang__) || defined(__llvm__)
    #pragma options align=reset
#else
    #pragma pack()
#endif

[kencu (Ken)]

Here is the upstream suggested patch to (some version of) gcc to make it accept the clang pragma variation, that at least one person said "Doesn't look right".

<​https://gcc.gnu.org/bugzilla/attachment.cgi?id=25656>

[kencu (Ken)]

a PR that implements a peg for older systems <​https://github.com/macports/macports-ports/pull/9519>

Not 100% accurate -- the peg should be for any non-apple gcc versions -- but that is too much of a PITA to do, so we do this instead, for now.

Alternatively, we could do one of these:

• patch the USB.h header and use it in legacysupport (we are getting close to doing that sort of thing)
• patch gcc to add the variant pragma handling
• patch gcc to use it's own "fixincludes" to fix up the USB.h header in the same exact manner we are talking about fixing in in legacysupport
• fix clang-7.0 to properly output macho objects with the PowerPC ABI ...

[michaelld (Michael Dickens)]

FYI adding in a fixed IOKit/usb/USB.h to include/LegacySupport gets over the immediate issue. After that there's some more build errors .... so thinking your PR is the way to go at the moment.

[kencu (Ken)]

+1

[michaelld (Michael Dickens)]

In 653135a509291c0299d389f9c6933cdfe15bf372/macports-ports (master):

libusb: add patch to fix size of const array to build using older GCC in C11

Ref: #61868

[michaelld (Michael Dickens)]

OK so the build issue is these 2 lines: ​https://github.com/libusb/libusb/blob/master/libusb/os/darwin_usb.c#L2150-L2151

  const unsigned char max_transfer_type = LIBUSB_TRANSFER_TYPE_BULK_STREAM;
  const char *transfer_types[max_transfer_type + 1] = {"control", "isoc", "bulk", "interrupt", "bulk-stream"};

LIBUSB_TRANSFER_TYPE_BULK_STREAM is part of an enum ​https://github.com/libusb/libusb/blob/master/libusb/libusb.h#L1098-L1113 , and -could- thus be taken as a constant by the compiler. Clang indeed does this, but GCC at least through version 7 doesn't! The error could be more obvious, too ... took some sleuthing to figure out that the compiler was referring to the value max_transfer_type as being not taken as a constant. Luckily this issue is easily worked around via changing the 2nd line to:

  const char *transfer_types[] = {"control", "isoc", "bulk", "interrupt", "bulk-stream", 0};

Either way should work, and the code makes sense either way. I just pushed a patch to use this latter method since it is "more correct" IMHO.

[michaelld (Michael Dickens)]

With this change + adding in a fixed-up USB.h header to LegacySupport, libusb 1.0.24 now builds fine on OSX 10.5 PPC. I'll submit this change upstream as well as start a PR for the USB.h addition to LegacySupport ... that one will be a little tricky since my current best fix is to copy USB.h and edit it ... which I'm guessing means we need the latest version, somehow tailored for different OSX versions ... guessing ... maybe @kencu has some ideas here?

[kencu (Ken)]

I'm thinking we copy gcc's fixincludes methodology, copying in the header that needs tweaking during the installation/activation of legacysupport and then applying a patch to it.

How does that sound?

[kencu (Ken)]

there will be some difficulties if we ever want to do both things -- what legacysupport does now, using include_next, but also modify the header too...

seems we can only pick one or the other without getting overly confusing about it...

[michaelld (Michael Dickens)]

Replying to kencu:

I'm thinking we copy gcc's fixincludes methodology, copying in the header that needs tweaking during the installation/activation of legacysupport and then applying a patch to it.

How does that sound?

I like this idea! I'm guessing this would work much better than trying to create a generic multi-OS version of USB.h .. though that could be done if this method proves too arduous!

[michaelld (Michael Dickens)]

Looks like this issue is present in USB.h through macOS 10.14 ... it does not seem to be present in 10.15 or 11.* ... actual testing on 10.14 shows build failures using gcc9 and gcc10 ... my my my! So, the trigger should be if the C compiler matches "gcc*" then force the LegacySupport", which would install and do a "fixincludes" on USB.h ... that should do the trick!

[kencu (Ken)]

I'm actually surprised to hear it's not every os version. I wonder what they did in 10.15 and 11.x to allow gcc not to barf on USB.h there.

We should probably copy what Apple did, if they did something to USB.h to fix this.

[michaelld (Michael Dickens)]

Upstream PR for the const array size issue: ​https://github.com/libusb/libusb/pull/833

[michaelld (Michael Dickens)]

Replying to kencu:

I'm actually surprised to hear it's not every os version. I wonder what they did in 10.15 and 11.x to allow gcc not to barf on USB.h there.

We should probably copy what Apple did, if they did something to USB.h to fix this.

Yeah I was too! I'll look into a diff to see if we can just use the newer version ... but I'm guessing no & that it makes more sense to do a fixincludes on the header like what GCC does.

[ballapete (Peter "Pete" Dyballa)]

For me on PPC Leopard configure runs fine with GCC7, but later this happens:

	/bin/sh ../libtool  --tag=CC   --mode=compile /opt/local/bin/gcc-mp-7 -DHAVE_CONFIG_H -I. -I..   -I/opt/local/include -std=gnu11  -Wall -Wextra -Wshadow -Wunused -Wwrite-strings -Werror=format-security -Werror=implicit-function-declaration -Werror=implicit-int -Werror=init-self -Werror=missing-prototypes -Werror=strict-prototypes -Werror=undef -Werror=uninitialized -fvisibility=hidden -pthread -pipe -Os -arch ppc -MT os/darwin_usb.lo -MD -MP -MF $depbase.Tpo -c -o os/darwin_usb.lo os/darwin_usb.c &&\
	mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  /opt/local/bin/gcc-mp-7 -DHAVE_CONFIG_H -I. -I.. -I/opt/local/include -std=gnu11 -Wall -Wextra -Wshadow -Wunused -Wwrite-strings -Werror=format-security -Werror=implicit-function-declaration -Werror=implicit-int -Werror=init-self -Werror=missing-prototypes -Werror=strict-prototypes -Werror=undef -Werror=uninitialized -fvisibility=hidden -pthread -pipe -Os -arch ppc -MT os/darwin_usb.lo -MD -MP -MF os/.deps/darwin_usb.Tpo -c os/darwin_usb.c  -fno-common -DPIC -o os/.libs/darwin_usb.o
In file included from /System/Library/Frameworks/IOKit.framework/Headers/usb/IOUSBLib.h:27:0,
                 from os/darwin_usb.h:30,
                 from os/darwin_usb.c:49:
/System/Library/Frameworks/IOKit.framework/Headers/usb/USB.h:594:9: error: too many #pragma options align=reset
 #pragma options align=reset
         ^~~~~~~
/System/Library/Frameworks/IOKit.framework/Headers/usb/USB.h:612:9: error: too many #pragma options align=reset
 #pragma options align=reset
         ^~~~~~~
/System/Library/Frameworks/IOKit.framework/Headers/usb/USB.h:632:9: error: too many #pragma options align=reset
 #pragma options align=reset
         ^~~~~~~
In file included from os/darwin_usb.h:26:0,
                 from os/darwin_usb.c:49:
os/darwin_usb.c: In function 'darwin_get_cached_device':
os/darwin_usb.c:1011:16: warning: format '%x' expects argument of type 'unsigned int', but argument 6 has type 'UInt32 {aka long unsigned int}' [-Wformat=]
       usbi_dbg("matching sessionID/locationID 0x%" PRIx64 "/0x%x against cached device with sessionID/locationID 0x%" PRIx64 "/0x%x",
                ^
./libusbi.h:287:67: note: in definition of macro '_usbi_log'
 #define _usbi_log(ctx, level, ...) usbi_log(ctx, level, __func__, __VA_ARGS__)
                                                                   ^~~~~~~~~~~
os/darwin_usb.c:1011:7: note: in expansion of macro 'usbi_dbg'
       usbi_dbg("matching sessionID/locationID 0x%" PRIx64 "/0x%x against cached device with sessionID/locationID 0x%" PRIx64 "/0x%x",
       ^~~~~~~~
os/darwin_usb.c:1011:64: note: format string is defined here
       usbi_dbg("matching sessionID/locationID 0x%" PRIx64 "/0x%x against cached device with sessionID/locationID 0x%" PRIx64 "/0x%x",
                                                               ~^
                                                               %lx
In file included from os/darwin_usb.h:26:0,
                 from os/darwin_usb.c:49:
os/darwin_usb.c:1011:16: warning: format '%x' expects argument of type 'unsigned int', but argument 8 has type 'UInt32 {aka long unsigned int}' [-Wformat=]
       usbi_dbg("matching sessionID/locationID 0x%" PRIx64 "/0x%x against cached device with sessionID/locationID 0x%" PRIx64 "/0x%x",
                ^
                sessionID, locationID, new_device->session, new_device->location);
                                                            ~~~~~~~~~~~~~
./libusbi.h:287:67: note: in definition of macro '_usbi_log'
 #define _usbi_log(ctx, level, ...) usbi_log(ctx, level, __func__, __VA_ARGS__)
                                                                   ^~~~~~~~~~~
os/darwin_usb.c:1011:7: note: in expansion of macro 'usbi_dbg'
       usbi_dbg("matching sessionID/locationID 0x%" PRIx64 "/0x%x against cached device with sessionID/locationID 0x%" PRIx64 "/0x%x",
       ^~~~~~~~
os/darwin_usb.c:1011:131: note: format string is defined here
       usbi_dbg("matching sessionID/locationID 0x%" PRIx64 "/0x%x against cached device with sessionID/locationID 0x%" PRIx64 "/0x%x",
                                                                                                                                  ~^
                                                                                                                                  %lx
make[2]: *** [os/darwin_usb.lo] Error 1
make[2]: Leaving directory `/opt/local/var/macports/build/_opt_local_var_macports_sources_nue.de.rsync.macports.org_macports_release_tarballs_ports_devel_libusb/libusb/work/libusb-libusb-c6a35c5/libusb'

[ballapete (Peter "Pete" Dyballa)]

Main.log from PPC Leopard

[michaelld (Michael Dickens)]

@ballapete : can you try out the fix to "IOKit/USB/usb.h" as noted above in comment 10 < ​https://trac.macports.org/ticket/61868#comment:10 >? It works for me (& I think others), and even works when we use LegacySupport and place the header correctly there ... basically (as @kencu notes somewhere) we can do a GCC-style "fixincludes" on "usb.h" to fix the #pramga's ... the rest are just warnings, which can ignore reasonably safely.

[ballapete (Peter "Pete" Dyballa)]

Replying to michaelld:

@ballapete : can you try out the fix to "IOKit/USB/usb.h" as noted above in comment 10 < ​https://trac.macports.org/ticket/61868#comment:10 >?

In a few days! Right now I am upgrading Port packages on Tiger…

[ballapete (Peter "Pete" Dyballa)]

Replying to ballapete:

After automatically upgrading cctools the failure is still the same. I can start now to apply the procedure.

[ballapete (Peter "Pete" Dyballa)]

Replying to kencu:

With this change libusb @1.0.24 built:

Making all in libusb
make[2]: Entering directory `/opt/local/var/macports/build/_opt_local_var_macports_sources_nue.de.rsync.macports.org_macports_release_tarballs_ports_devel_libusb/libusb/work/libusb-libusb-c6a35c5/libusb'
depbase=`echo os/darwin_usb.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
	/bin/sh ../libtool  --tag=CC   --mode=compile /opt/local/bin/gcc-mp-7 -DHAVE_CONFIG_H -I. -I..   -I/opt/local/include -std=gnu11  -Wall -Wextra -Wshadow -Wunused -Wwrite-strings -Werror=format-security -Werror=implicit-function-declaration -Werror=implicit-int -Werror=init-self -Werror=missing-prototypes -Werror=strict-prototypes -Werror=undef -Werror=uninitialized -fvisibility=hidden -pthread -pipe -Os -arch ppc -MT os/darwin_usb.lo -MD -MP -MF $depbase.Tpo -c -o os/darwin_usb.lo os/darwin_usb.c &&\
	mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  /opt/local/bin/gcc-mp-7 -DHAVE_CONFIG_H -I. -I.. -I/opt/local/include -std=gnu11 -Wall -Wextra -Wshadow -Wunused -Wwrite-strings -Werror=format-security -Werror=implicit-function-declaration -Werror=implicit-int -Werror=init-self -Werror=missing-prototypes -Werror=strict-prototypes -Werror=undef -Werror=uninitialized -fvisibility=hidden -pthread -pipe -Os -arch ppc -MT os/darwin_usb.lo -MD -MP -MF os/.deps/darwin_usb.Tpo -c os/darwin_usb.c  -fno-common -DPIC -o os/.libs/darwin_usb.o
In file included from os/darwin_usb.h:26:0,
                 from os/darwin_usb.c:49:
os/darwin_usb.c: In function 'darwin_get_cached_device':
os/darwin_usb.c:1011:16: warning: format '%x' expects argument of type 'unsigned int', but argument 6 has type 'UInt32 {aka long unsigned int}' [-Wformat=]
       usbi_dbg("matching sessionID/locationID 0x%" PRIx64 "/0x%x against cached device with sessionID/locationID 0x%" PRIx64 "/0x%x",
                ^
./libusbi.h:287:67: note: in definition of macro '_usbi_log'
 #define _usbi_log(ctx, level, ...) usbi_log(ctx, level, __func__, __VA_ARGS__)
                                                                   ^~~~~~~~~~~
os/darwin_usb.c:1011:7: note: in expansion of macro 'usbi_dbg'
       usbi_dbg("matching sessionID/locationID 0x%" PRIx64 "/0x%x against cached device with sessionID/locationID 0x%" PRIx64 "/0x%x",
       ^~~~~~~~
os/darwin_usb.c:1011:64: note: format string is defined here
       usbi_dbg("matching sessionID/locationID 0x%" PRIx64 "/0x%x against cached device with sessionID/locationID 0x%" PRIx64 "/0x%x",
                                                               ~^
                                                               %lx
In file included from os/darwin_usb.h:26:0,
                 from os/darwin_usb.c:49:
os/darwin_usb.c:1011:16: warning: format '%x' expects argument of type 'unsigned int', but argument 8 has type 'UInt32 {aka long unsigned int}' [-Wformat=]
       usbi_dbg("matching sessionID/locationID 0x%" PRIx64 "/0x%x against cached device with sessionID/locationID 0x%" PRIx64 "/0x%x",
                ^
                sessionID, locationID, new_device->session, new_device->location);
                                                            ~~~~~~~~~~~~~
./libusbi.h:287:67: note: in definition of macro '_usbi_log'
 #define _usbi_log(ctx, level, ...) usbi_log(ctx, level, __func__, __VA_ARGS__)
                                                                   ^~~~~~~~~~~
os/darwin_usb.c:1011:7: note: in expansion of macro 'usbi_dbg'
       usbi_dbg("matching sessionID/locationID 0x%" PRIx64 "/0x%x against cached device with sessionID/locationID 0x%" PRIx64 "/0x%x",
       ^~~~~~~~
os/darwin_usb.c:1011:131: note: format string is defined here
       usbi_dbg("matching sessionID/locationID 0x%" PRIx64 "/0x%x against cached device with sessionID/locationID 0x%" PRIx64 "/0x%x",
                                                                                                                                  ~^
                                                                                                                                  %lx
os/darwin_usb.c:1070:63: warning: '%02x' directive output may be truncated writing 2 bytes into a region of size between 1 and 3 [-Wformat-truncation=]
       snprintf(new_device->sys_path, 20, "%03i-%04x-%04x-%02x-%02x", new_device->address,
                                                               ^~~~
os/darwin_usb.c:1070:42: note: directive argument in the range [0, 255]
       snprintf(new_device->sys_path, 20, "%03i-%04x-%04x-%02x-%02x", new_device->address,
                                          ^~~~~~~~~~~~~~~~~~~~~~~~~~
os/darwin_usb.c:1070:7: note: 'snprintf' output between 20 and 22 bytes into a destination of size 20
       snprintf(new_device->sys_path, 20, "%03i-%04x-%04x-%02x-%02x", new_device->address,
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                libusb_le16_to_cpu (new_device->dev_descriptor.idVendor),
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                libusb_le16_to_cpu (new_device->dev_descriptor.idProduct),
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                new_device->dev_descriptor.bDeviceClass, new_device->dev_descriptor.bDeviceSubClass);
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
libtool: compile:  /opt/local/bin/gcc-mp-7 -DHAVE_CONFIG_H -I. -I.. -I/opt/local/include -std=gnu11 -Wall -Wextra -Wshadow -Wunused -Wwrite-strings -Werror=format-security -Werror=implicit-function-declaration -Werror=implicit-int -Werror=init-self -Werror=missing-prototypes -Werror=strict-prototypes -Werror=undef -Werror=uninitialized -fvisibility=hidden -pthread -pipe -Os -arch ppc -MT os/darwin_usb.lo -MD -MP -MF os/.deps/darwin_usb.Tpo -c os/darwin_usb.c -o os/darwin_usb.o >/dev/null 2>&1
/bin/sh ../libtool  --tag=CC   --mode=link /opt/local/bin/gcc-mp-7 -std=gnu11  -Wall -Wextra -Wshadow -Wunused -Wwrite-strings -Werror=format-security -Werror=implicit-function-declaration -Werror=implicit-int -Werror=init-self -Werror=missing-prototypes -Werror=strict-prototypes -Werror=undef -Werror=uninitialized -fvisibility=hidden -pthread -pipe -Os -arch ppc -version-info 3:0:3 -no-undefined -L/opt/local/lib -Wl,-headerpad_max_install_names -arch ppc -o libusb-1.0.la -rpath /opt/local/lib core.lo descriptor.lo hotplug.lo io.lo strerror.lo sync.lo os/events_posix.lo os/threads_posix.lo os/darwin_usb.lo    -lobjc -Wl,-framework,IOKit -Wl,-framework,CoreFoundation
libtool: link: /opt/local/bin/gcc-mp-7 -dynamiclib  -o .libs/libusb-1.0.0.dylib  .libs/core.o .libs/descriptor.o .libs/hotplug.o .libs/io.o .libs/strerror.o .libs/sync.o os/.libs/events_posix.o os/.libs/threads_posix.o os/.libs/darwin_usb.o   -L/opt/local/lib -lobjc  -pthread -Os -arch ppc -Wl,-headerpad_max_install_names -arch ppc -Wl,-framework -Wl,IOKit -Wl,-framework -Wl,CoreFoundation   -pthread -install_name  /opt/local/lib/libusb-1.0.0.dylib -compatibility_version 4 -current_version 4.0 -Wl,-single_module
libtool: link: (cd ".libs" && rm -f "libusb-1.0.dylib" && ln -s "libusb-1.0.0.dylib" "libusb-1.0.dylib")
libtool: link: ar cru .libs/libusb-1.0.a  core.o descriptor.o hotplug.o io.o strerror.o sync.o os/events_posix.o os/threads_posix.o os/darwin_usb.o
libtool: link: ranlib .libs/libusb-1.0.a
libtool: link: ( cd ".libs" && rm -f "libusb-1.0.la" && ln -s "../libusb-1.0.la" "libusb-1.0.la" )
make[2]: Leaving directory `/opt/local/var/macports/build/_opt_local_var_macports_sources_nue.de.rsync.macports.org_macports_release_tarballs_ports_devel_libusb/libusb/work/libusb-libusb-c6a35c5/libusb'

[ballapete (Peter "Pete" Dyballa)]

To patch system's /System/Library/Frameworks/IOKit.framework/Headers/usb/USB.h

[michaelld (Michael Dickens)]

FYI the required change that I added a patch in MP for has been merged upstream: ​https://github.com/libusb/libusb/commit/a247f1696ed83e492a5435f338caa5a12da3fd83 ... so, the patch must be removed from the devel port when we update it. There is a release coming "soon" and so this will be part of it .. yay!

Thus, we need to fixup the Legacy project to do a "fixincludes" of sorts: for impacted systems, copy the file <IOKit/usb/USB.h> into the same directory structure in Legacy & then patch the file per what we've all noted above. @Kencu: Do you have a preference of the language used for "fixincludes"? We can do this directly in the Makefile, or create a separate script (e.g., sh), or a patch for each OSX version's USB.h ... lots of options! I think a shell script or directly in the Makefile are the most efficient, but for future such "fixincludes" what precedent do we want to set for where & how this is done?

[kencu (Ken)]

I was initially thinking to run this particular thing through the Portfile:

1. copy the file into place post-extract maybe
2. patch it with our normal patch mechanisms
3. destroot it.

As we have no buildbot for (most of) these systems, that might be OK...

we could get fancy and checksum the original header to make sure it's virgin...

But there are other ways, and that is not the only method.

For the "which" program, I just copied it into the legacysupport repo. We could just do that too, if we wanted to KISS....

Ken

[michaelld (Michael Dickens)]

Hmmm ... my vote is to keep everything LegacySupport need to do with itself, so that if somebody for some reason wants to install it separately from MP and use it then that’s an option. Moving code into the Portfile means it’s only available within MP ... makes the code almost trivial, but at the same time it’s means LegacySupport itself is not complete. It shouldn’t be difficult to augment the Makefile to run a “fixincludes” section that does this task robustly. Let me give this a go & report back.!

[kencu (Ken)]

don't forget then that each different system will need (in cases) different patches, and different patches for each different file sometimes depending on each different system.

eg...

Tiger might need 10 headers to get patches, two new headers that don't exist in the SDK, whereas Leopard needs three headers to get three other patches, whereas Lion needs one.

blah blah blah.

[michaelld (Michael Dickens)]

true ... but for this specific issue, it's a max of 1 header regardless of the OSX / MacOSX / macOS version. It's easy to test for & determine whether it is required (via a "grep" of the header for the pragma line), and that single line can be patched via a "sed" script (not a patch since that's not reliable across versions of that same file I'm pretty sure) ... again: I'll give this a go when I get a chance (soon hopefully) to see how complicated & reliable it is. yes: blah blah blah ... :)

[michaelld (Michael Dickens)]

OK so after playing around with this fixincludes concept, it's certainly doable, and robustly ... but we don't want to do it inside the Makefile: just too much code. So I'm proposing a new Makefile entry for "fixincludes:" which requires nothing and executes shell scripts for each task. This seems like a reasonable way to go about doing this. We might consider some common code down the road, but I'd guess the first few fixincludes will just get the job done. I'll hopefully get a PR in place in the next few days.

[michaelld (Michael Dickens)]

I just attached my current shell script that is to be executed by the legacy Makefile. It creates the directory include/IOKit/usb/ in the current directory, then patches (via a sed script) the header USB.h into the new directory. In my testing, this script works on 10.4 and 10.5. It will do something only if the USB.h header has the issue, and it performs basic testing on the resulting header to show that the patching works. It needs more testing to verify functionality on newer OSX / MacOSX / macOS, from 10.6 through 10.14 and beyond -- to verify for example that on 10.15 it does nothing. Next up is to integrate calling it into the legacy Makefile, which I hope is easier than creating the script in the first place!

[michaelld (Michael Dickens)]

The script needs a little work still ... need to set environment variables so that SED works ... and on Big Sur the MAJOR.MINOR isn't correct (not even sure how to determine the "minor" there ...) ... and the number of SED replacements varies from OSX to OSX (I was assuming 3 since that's what I found on 10.4 and 10.5) ... working on those ...

[michaelld (Michael Dickens)]

I just updated the script, which is significantly modified to work more robustly. It not plays nicely with various SED on 10.4, 10.5, and 10.12. Still needs testing on the other MacOSX / OSX. Works on macOS 11 but doesn't get the MAJOR.MINOR correct yet (though that doesn't matter since the header doesn't need patching).

[ballapete (Peter "Pete" Dyballa)]

With the uploaded patch to change system's /System/Library/Frameworks/IOKit.framework/Headers/usb/USB.h libusb @1.0.24 builds on PPC Tiger, Mac OS X 10.4.11.

[fhgwright (Fred Wright)]

Just to add to the confusion, I came across this GCC documentation (for GCC 6), from Fedora of all places:

​https://dmalcolm.fedorapeople.org/gcc/2015-08-31/rst-experiment/pragmas-accepted-by-gcc.html#darwin-pragmas

This claims that the relevant pragma is accepted when GCC is built for Darwin. Does this documentation lie, or are MacPorts GCCs missing some configure option to enable Darwin-related features?

BTW, the pragma is present in all OS versions 10.4-10.13 (at least), and no MacPorts GCC accepts it.

A couple of other notes:

Configure fails on 10.4, because libusb requires thread-local storage, but the compiler setup doesn't specify that.

There's some clang blacklisting claiming to be related to stdatomic.h, but stdatomic.h doesn't appear in the libusb sources at all.

[kencu (Ken)]

Fedora may be right in some respects, but this pragma issue still lives on, and has never been fixed in gcc:

​https://gcc.gnu.org/bugzilla/show_bug.cgi?id=50909

the Darwin build spec happens with the host/target options, and are correctly set, and there are no other I know of re: pragmas.

So - no bueno.

Re stdatomic.h ... as of a certain version clang turned off their (somewhat flawed) atomic support in compiler-rt, and it was broken for 32bit build targets, but I've been turning it back on gradually.

[evanmiller (Evan Miller)]

I was talking to some witches around a boiling cauldron last night and think we have come up with an elegant hack / workaround:

#pragma options align=power
#pragma options align=power
#pragma options align=power

#include <IOKit/usb/USB.h>

The issue here is that pack and options maintain separate push/pop alignment stacks, and GCC doesn't like it when you mix and match the stacks and attempt to pop an empty options stack via align=reset.

What we can do then is "pre-load" the options stack with three values of power, which corresponds to an alignment of 0 (i.e. the default). The three invocations correspond to the three align=reset values that later appear in the IOUSBLib.h file.

Both pack and options are setting the same global variable internally (maximum_field_alignment), so this arrangement ends up having the desired effect: alignment starts as 0, is set to 1 when #pragma pack(1) is encountered, and then is set back to 0 when #pragma options align=reset shows up at the end of the packed struct.

After the third align=reset, the stack is empty and the alignment state is back to where it was before the inclusion. Anway this logic seems to hold with GCC7.5, but testing will be needed with other GCC versions.

If the solution is as simple as this then we have our options of directly patching the client ports with the magic incantation, or making a very simple LegacySupport header that adds the three pragmas just before #include_next.

[evanmiller (Evan Miller)]

Proposed fix for anyone who wants to test at home:

​https://github.com/evanmiller/macports-ports/commit/60ba7329a789a5e3a19c513da12f28c65e30c3ee

[michaelld (Michael Dickens)]

Thanks @evanmiller! Very interesting & strange solution! This is certainly worth considering for libusb itself. I still like the generic solution of fixing up LegacySupport to work around this issue generically for impacted OSX. I just have not had enough time to finalize it!

[evanmiller (Evan Miller)]

All right, opened a draft PR over here just for libusb if anyone's interested: ​https://github.com/macports/macports-ports/pull/12677

How generic is your LegacySupport solution? At least here on Tiger, it appears that other Framework headers are affected by this kind of pragma intermingling (see e.g. ​here)

[mascguy (Christopher Nielsen)]

Replying to michaelld:

Thanks @evanmiller! Very interesting & strange solution! This is certainly worth considering for libusb itself. I still like the generic solution of fixing up LegacySupport to work around this issue generically for impacted OSX. I just have not had enough time to finalize it!

Agreed, this is a very unique solution. Love it!

Replying to evanmiller:

All right, opened a draft PR over here just for libusb if anyone's interested: ​https://github.com/macports/macports-ports/pull/12677

How generic is your LegacySupport solution? At least here on Tiger, it appears that other Framework headers are affected by this kind of pragma intermingling (see e.g. ​here)

If we can find a way to (carefully) apply this via LegacySupport somehow, that would be awesome! That said, if folks would like to get this fix into libusb relatively quickly, we can always reconcile later.

What do you think Michael?

[michaelld (Michael Dickens)]

OK so if we want to do this generically, we need to augment LegacySupport. -If- this solution ("hack") is generic across macOS & GCC versions, then it would be a simple addition to LegacySupport as it is static / does not require parsing the current USB.h header (as my script requires). This is a positive feature! I would prefer to not just fix libusb, but rather use LegacySupport & fix the overarching issue.

@evanmiller : if you look in LegacySupport there is code that shows how to "#include_next", which is I think what we want here. We create a file "include/IOKit/usb/USB.h" in LegacySupport that looks for GCC and its version & then does your hack if required. Then includes the next USB.h. I'm pretty sure this will work, and in this manner we can test out your hack & it would be much more robustly used!

[evanmiller (Evan Miller)]

@michaelld I think that will work. As mentioned there may be other headers with this same issue (esp. in old SDKs) but we can whack the moles as they rear up their heads.

So I'm thinking something like

#if (gcc version match)
#pragma options align=power
#pragma options align=power
#pragma options align=power
#pragma options align=power
#pragma options align=power
#endif

#include_next <IOKit/usb/USB.h>

And then ports can do

# USB.h GCC fix
legacysupport.newest_darwin_requires_legacy 18

To be even more careful, we could adjust the number of align=power instances to exactly match the number of align=resets based on the SDK version. However, I can't think of any harm in having too many, so that additional logic seems unnecessary. The most resets that I counted in the USB.h of any SDK version was five.

[michaelld (Michael Dickens)]

Yup that's what I'm talking about!

[evanmiller (Evan Miller)]

Threw together a quick PR against LegacySupport: ​https://github.com/macports/macports-legacy-support/pull/42

If we can get this merged and rolled into legacy-support-devel then people can start playing with it.

Inspecting the GCC source, it looks like the relevant pragma-handling code is basically unchanged from GCC 4.5 through 11 so hopefully the fix will work everywhere.

[evanmiller (Evan Miller)]

In 5b2fbda789465b93fdbf924bfd00c6bb3b816999/macports-legacy-support (master):

IOKit: fix USB.h inclusion with GCC 4.5+ pre-Catalina (​https://github.com/macports/macports-legacy-support/pull/42)

See: #61868

[michaelld (Michael Dickens)]

Closing as fixed. Let's please give this change some time to be out in the wild before reopening if there is an issue. But, if there is a need then please do reopen and post build log(s) showing what's going on.

[evanmiller (Evan Miller)]

Just to clarify to anyone who hasn't followed every inning of the baseball game here: this issue will be fixed once there is 1) a new legacy-support release (version 1.0.5?) followed by 2) a rev-bump of libusb. So don't get too excited until you see both of those ports in your port outdated list :-)

[fhgwright (Fred Wright)]

In that case it would be best to leave the ticket open until the fix is actually deployed.

[evanmiller (Evan Miller)]

In 8ce1ce95d7d6f0b0111c37e1a2bfb15efa844734/macports-ports (master):

libusb-devel: fix pre-Catalina build issues

Closes: #63668
See: ​https://github.com/libusb/libusb/pull/1023
See: #61868

[evanmiller (Evan Miller)]

A fix is now available for testing in the -devel ports:

1. Uninstall legacy-support
2. Install legacy-support-devel
3. Install libusb-devel

[barracuda156]

Applying the patch above did not work for me on 10.6 PPC:

In file included from os/darwin_usb.c:26:0:
/usr/include/pthread.h:361:6: note: expected 'char *' but argument is of type 'const char *'
 int  pthread_setname_np(char*);
      ^~~~~~~~~~~~~~~~~~
In file included from os/darwin_usb.h:26:0,
                 from os/darwin_usb.c:49:
os/darwin_usb.c: In function 'darwin_get_cached_device':
os/darwin_usb.c:1011:16: warning: format '%x' expects argument of type 'unsigned int', but argument 6 has type 'UInt32 {aka long unsigned int}' [-Wformat=]
       usbi_dbg("matching sessionID/locationID 0x%" PRIx64 "/0x%x against cached device with sessionID/locationID 0x%" PRIx64 "/0x%x",
                ^
./libusbi.h:287:67: note: in definition of macro '_usbi_log'
 #define _usbi_log(ctx, level, ...) usbi_log(ctx, level, __func__, __VA_ARGS__)
                                                                   ^~~~~~~~~~~
os/darwin_usb.c:1011:7: note: in expansion of macro 'usbi_dbg'
       usbi_dbg("matching sessionID/locationID 0x%" PRIx64 "/0x%x against cached device with sessionID/locationID 0x%" PRIx64 "/0x%x",
       ^~~~~~~~
os/darwin_usb.c:1011:64: note: format string is defined here
       usbi_dbg("matching sessionID/locationID 0x%" PRIx64 "/0x%x against cached device with sessionID/locationID 0x%" PRIx64 "/0x%x",
                                                               ~^
                                                               %lx
In file included from os/darwin_usb.h:26:0,
                 from os/darwin_usb.c:49:
os/darwin_usb.c:1011:16: warning: format '%x' expects argument of type 'unsigned int', but argument 8 has type 'UInt32 {aka long unsigned int}' [-Wformat=]
       usbi_dbg("matching sessionID/locationID 0x%" PRIx64 "/0x%x against cached device with sessionID/locationID 0x%" PRIx64 "/0x%x",
                ^
                sessionID, locationID, new_device->session, new_device->location);
                                                            ~~~~~~~~~~~~~
./libusbi.h:287:67: note: in definition of macro '_usbi_log'
 #define _usbi_log(ctx, level, ...) usbi_log(ctx, level, __func__, __VA_ARGS__)
                                                                   ^~~~~~~~~~~
os/darwin_usb.c:1011:7: note: in expansion of macro 'usbi_dbg'
       usbi_dbg("matching sessionID/locationID 0x%" PRIx64 "/0x%x against cached device with sessionID/locationID 0x%" PRIx64 "/0x%x",
       ^~~~~~~~
os/darwin_usb.c:1011:131: note: format string is defined here
       usbi_dbg("matching sessionID/locationID 0x%" PRIx64 "/0x%x against cached device with sessionID/locationID 0x%" PRIx64 "/0x%x",
                                                                                                                                  ~^
                                                                                                                                  %lx
make[2]: *** [os/darwin_usb.lo] Error 1
make[2]: Leaving directory `/opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_devel_libusb/libusb/work/libusb-libusb-c6a35c5/libusb'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_devel_libusb/libusb/work/libusb-libusb-c6a35c5'
make: *** [all] Error 2
make: Leaving directory `/opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_devel_libusb/libusb/work/libusb-libusb-c6a35c5'
Command failed:  cd "/opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_devel_libusb/libusb/work/libusb-1.0.24" && /usr/bin/make -j1 -w all AM_DEFAULT_VERBOSITY=1 
Exit code: 2
Error: Failed to build libusb: command execution failed
Error: See /opt/local/var/macports/logs/_opt_PPCSnowLeopardPorts_devel_libusb/libusb/main.log for details.
Error: Follow https://guide.macports.org/#project.tickets if you believe there
is a bug.
Error: Processing of port libusb failed

[evanmiller (Evan Miller)]

@barracuda156 Please try the procedure described in comment:61 (no patching required).