Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#56737 closed defect (fixed)

graphene @1.8.2 +universal: graphene-1.0.pc differs and cannot be merged

Reported by: kencu (Ken) Owned by: dbevans (David B. Evans)
Priority: Normal Milestone:
Component: ports Version: 2.5.2
Keywords: Cc: MarcusCalhoun-Lopez (Marcus Calhoun-Lopez), arietis (Sergei Guselnikov), hapaguy (Brian Kurt Fujikawa), dershow, Ionic (Mihai Moldovan), MaddTheSane (C.W. Betts), geoffgrimwood, mopihopi, thetrial (alabay)
Port: graphene

Description (last modified by kencu (Ken))

I've had graphene built as +universal since forever I believe, but on this last update it fails to build. I see it was recently altered to help it build universal. I'm not sure how I had it built universal before, in fact, but I did.

The error comes with three text files that differ between builds. The following fixes it, possibly incorrectly:

# gobject-introspection uses g-ir-scanner, which uses $CC from env
if {[variant_isset universal]} {
    foreach arch ${configure.universal_archs} {
        lappend merger_build_env(${arch})  CC='${configure.cc} -arch ${arch}'
        lappend merger_destroot_env(${arch})  CC='${configure.cc} -arch ${arch}'
    }
+    global merger_dont_diff
+    lappend merger_dont_diff    "${prefix}/lib/pkgconfig/graphene-1.0.pc"
+    lappend merger_dont_diff    "${prefix}/lib/pkgconfig/graphene-gobject-1.0.pc"
+    lappend merger_dont_diff    "${prefix}/lib/graphene-1.0/include/graphene-config.h"
} else {

Attachments (1)

graphene.182.universal.log (53.8 KB) - added by kencu (Ken) 2 years ago.

Download all attachments as: .zip

Change History (26)

Changed 2 years ago by kencu (Ken)

Attachment: graphene.182.universal.log added

comment:1 Changed 2 years ago by MaddTheSane (C.W. Betts)

Looking at the difference between the pkgconfig files generated, it looks like the 32-bit version doesn't use SSE, but the 64-bit version does.

comment:2 Changed 2 years ago by arietis (Sergei Guselnikov)

Cc: arietis added

comment:3 Changed 2 years ago by hapaguy (Brian Kurt Fujikawa)

Cc: hapaguy added

comment:4 Changed 2 years ago by MarcusCalhoun-Lopez (Marcus Calhoun-Lopez)

There is a pull request that will hopefully fix this problem.

comment:5 Changed 2 years ago by dershow

Cc: dershow added

comment:6 Changed 2 years ago by Ionic (Mihai Moldovan)

Cc: Ionic added

The mentioned PR just enables SSE2 checking globally. Yes, that probably fixes the issue for x86_64 and x86 universal builds, because it will be enabled for both arches (since we don't have x86-family CPUs that are old enough to not support SSE2), but... will likewise fail on universal builds mixing x86 or x86_64 and ppc, since the latter does not know or implement SSE.

The authors required at least GCC 4.2 for SSE2 support on non-x86_64 platforms originally and later bumped up the requirement for SSE2 on x86 to GCC 4.9, with a reasoning that does sound a bit shady. I do not understand why they bumped up the requirement if it worked with GCC 4.2 (coincidentally one of the versions we particularly care about, since some older platforms do have GCC 4.2 as the system compiler), but maybe it did cause runtime problems I'm not aware of.

Instead of a workaround like this, wouldn't it be better to fix the actual problem? For that matter, what IS the actual problem? Why does merging these files fail? Is there anything we can do about this and support universal builds that have diverging CPU features?

comment:7 Changed 2 years ago by Ionic (Mihai Moldovan)

Well, of course. The base problem is that the muniversal PortGroup does not know how to handle diverging .pc files. We should probably fix this instead.

Linux distributions put different-arch libraries (including .pc files) in a special directory (often called lib32 for 32bit vs. 64bit support).

We don't do this for binaries on OS X, since we can have fat files. We do have to jump through hoops for header files, though, so we'd need the same thing for .pc files. Sadly, we can't just install a pseudo-.pc-file that then includes others based on architecture, so this will be a bit more difficult and would need base support...

comment:8 Changed 2 years ago by Ionic (Mihai Moldovan)

The next best alternative would be to rather disable SSE2 detection globally and just error out #ifdef __APPLE__. That's probably most compatible.

Fixing the .pc problem is more convoluted.

comment:9 Changed 2 years ago by MaddTheSane (C.W. Betts)

Cc: MaddTheSane added

comment:10 Changed 2 years ago by geoffgrimwood

Cc: geoffgrimwood added

comment:11 Changed 2 years ago by kencu (Ken)

Googling around pkg-config leads to lots of discussions and threads on how pkg-config might be modified or enhanced to include knowledge of the host and build systems and fix this "cross-compiling" problem. These discussions go back many years, so we're certainly not the first to stumble across them.

If pkg-config supported include files, we could sort this out pretty easily, but it doesn't AFAIK.

There are proposals to add a -host flag to pkg-config that could be used as a director to metadata to find the arch-specific one you want.

More common seems to be to tuck the .pc files into triple folders that are specified to come earlier in the pkg-config search path. That might actually be something we could do in the muniversal portgroup and in base, but it's not a trivial project.

In this case, our option seems to be to force SSE2 for everyone and then figure out how to fix the ones that break doing that, or alternatively to shut it off for everyone, which is easier fer sure, but dumbs down graphene a bit on the 99% of MacPorts systems that can support SSE2.

Last edited 2 years ago by kencu (Ken) (previous) (diff)

comment:12 Changed 2 years ago by mopihopi

Cc: mopihopi added

comment:13 Changed 2 years ago by kencu (Ken)

had duplicate 56483

comment:14 Changed 2 years ago by thetrial (alabay)

Cc: thetrial added

comment:15 in reply to:  11 Changed 2 years ago by ryandesign (Ryan Schmidt)

Replying to kencu:

The error comes with three text files that don't lipo, of course.

The muniversal portgroup knows how to merge Mach-O files using lipo, and some text files, including .h files, so excluding the .h file should not have been necessary.

Of course it does not support merging .pc files because pkg-config doesn't offer any conditional syntax in .pc files, nor does it know what arch you're going to build for. The build system must therefore generate identical .pc files for each arch, or you will get the error you got.

Replying to kencu:

In this case, our option seems to be to force SSE2 for everyone and then figure out how to fix the ones that break doing that, or alternatively to shut it off for everyone, which is easier fer sure, but dumbs down graphene a bit on the 99% of MacPorts systems that can support SSE2.

Presumably you're saying that this 99% of systems will be building for x86_64 only. Assuming the above PR solves the problem for i386, that only leaves ppc systems. The port could disable SSE2 for the Intel archs when building universal and universal_archs contains the string "ppc".

comment:16 in reply to:  11 ; Changed 2 years ago by Ionic (Mihai Moldovan)

Disabling the check breakout (i.e., the proposal of the PR) for i386 and x86_64 (assuming that meson will then determine that SSE2 support is available since we don't have i386-based platforms) and forcing the breakout of universal_archs contains ppc (thus disabling SSE2 completely) sounds like a good compromise.

We'll just need the second part as well.

Slightly OT for this port: Replying to kencu:

Googling around pkg-config leads to lots of discussions and threads on how pkg-config might be modified or enhanced to include knowledge of the host and build systems and fix this "cross-compiling" problem. These discussions go back many years, so we're certainly not the first to stumble across them. There are proposals to add a -host flag to pkg-config that could be used as a director to metadata to find the arch-specific one you want.

Most systems don't need support for that in pkg-config itself.

If pkg-config supported include files, we could sort this out pretty easily, but it doesn't AFAIK.

Well, even then, we'd need conditional include directives.

More common seems to be to tuck the .pc files into triple[t] folders that are specified to come earlier in the pkg-config search path. That might actually be something we could do in the muniversal portgroup and in base, but it's not a trivial project.

Yeah, that needs proper base support. Not just the muniversal PortGroup, since muniversal is a hack when the simple universal features in base just aren't up to the task at hand (most often due to build system inflexibilities).

Putting non-base-arch .pc files into a special "triplet" (or actually just lib<arch> directory) would be good enough. During build runs, base must then add this (arch-dependent) specific directory to PKG_CONFIG_PATH. Not terribly difficult, but definitely a bigger task.

comment:17 in reply to:  16 Changed 2 years ago by kencu (Ken)

Replying to Ionic:

Slightly OT for this port...

Yes, I just explored that because you had requested a generic fix be developed above. It sounds like it's too much trouble for what it's worth, I agree -- if anyone was going to do that project, it would have made more sense to do it when Intel/PPC builds were the common +universal combo.

triple[t] folders

Well there indeed it looks like the internet is pretty much cleanly split on whether to call it a "target triple"

https://clang.llvm.org/docs/CrossCompilation.html#target-triple

or a "target triplet"

<https://www.gnu.org/software/autoconf/manual/autoconf-2.65/html_node/Specifying-Target-Triplets.html>

or both in the same document:

<http://flash.uchicago.edu/~jbgallag/configure.html>

All winners!

Last edited 2 years ago by ryandesign (Ryan Schmidt) (previous) (diff)

comment:18 Changed 2 years ago by kencu (Ken)

Description: modified (diff)

comment:19 Changed 2 years ago by ryandesign (Ryan Schmidt)

Summary: graphene 1.8.2: difficulty building +universalgraphene @1.8.2 +universal: graphene-1.0.pc differs and cannot be merged

comment:20 Changed 2 years ago by MarcusCalhoun-Lopez (Marcus Calhoun-Lopez)

Resolution: fixed
Status: assignedclosed

In 72c820b27ee817a55317ae20d8417a818a56af61/macports-ports (master):

graphene: fix detection of SSE

Fixes #56737

comment:21 Changed 2 years ago by Ionic (Mihai Moldovan)

Resolution: fixed
Status: closedreopened

As I've said before, this does not fix the issue for ppc* and x86-based universal builds. Reopening...

comment:22 Changed 2 years ago by Ken <21211439+kencu@…>

Resolution: fixed
Status: reopenedclosed

In f0cee08a90d6cd650ab0e5c663c89ec58072a057/macports-ports (master):

graphene: fix build with ppc

per Marcus, see PR
closes: #56737

comment:23 Changed 2 years ago by Ionic (Mihai Moldovan)

Ah, that makes more sense. As long as people don't need mix-arch universal builds. Probably no one nowadays does so, so we're in the clear. Thanks for the second commit.

comment:24 Changed 2 years ago by kencu (Ken)

looks like it died due to lack of posix memalign on 10.5.

Last edited 2 years ago by kencu (Ken) (previous) (diff)

comment:25 Changed 2 years ago by Ionic (Mihai Moldovan)

It's also failing on the 10.6 builders for some reason (looks MMX-related?)

Last edited 2 years ago by Ionic (Mihai Moldovan) (previous) (diff)
Note: See TracTickets for help on using tickets.