apr: duplicate case value when compiling universal i386/x86_64 or ppc/ppc64

[pguyot (Paul Guyot)]

When compiling apr +universal with the default targets set to i386 and x86_64, I get the following compilation error:

strings/apr_snprintf.c: In function 'conv_os_thread_t':
strings/apr_snprintf.c:500: error: duplicate case value
strings/apr_snprintf.c:498: error: previously used here
strings/apr_snprintf.c: In function 'conv_os_thread_t_hex':
strings/apr_snprintf.c:671: error: duplicate case value
strings/apr_snprintf.c:669: error: previously used here

In fact, I'm wondering if apr compiles on 64bits at all. The lines go like this:

    case sizeof(apr_int32_t):
        return conv_10(u.u32, TRUE, &is_negative, buf_end, len);
    case sizeof(apr_int64_t):
        return conv_10_quad(u.u64, TRUE, &is_negative, buf_end, len);

This is on 10.5.5/x86.

[danielluke (Daniel J. Luke)]

Have you reported this upstream?

I don't think I'm going to have time to track down the issues and fix them in the port in the near term, but if you have a patch (or can get upstream to generate one), I can test and apply it.

Otherwise, when I do get some time, I'll take a look at it.

[Veence (Vincent)]

This is caused by a wrong definition in include/apr.h of apr_int32_t and apr_int64_t. They are typedef'd as "int" and "long" whereas they should be typedef'd as int32_t int64_t, etc.

However, there is another issue: the lock managing code in atomic/unix seems to be only available fror ia32 arch. Part of it is written in assembly code and does not assemble with x86_64 option. I am going to see how I managed to compile this on my NetBSD amd64 box.

[Veence (Vincent)]

Ok, I've got it. The APR_SIZEOF_VOIDP is 8 bytes long on both i386 and x86_64. Therefore, the asm code select always cmpxchgQ instructions, which operate on quad words. It is okay with x86_64 of course, but generates an error with i386 asm.

The apr.h file should be changed again to look like this:

#if defined (__x86_64__)
  #define APR_SIZEOF_VOIDP 8
#elif defined (__i386__)
  #define APR_SIZEOF_VOIDP 4
#endif

Modify as needed for PPC archs.

With these two mods (apr_int* new typedefs and APR_SIZEOF_VOIDP), the package compiles all right with +universal mod.

[danielluke (Daniel J. Luke)]

Replying to 10.50@…:

With these two mods (apr_int* new typedefs and APR_SIZEOF_VOIDP), the package compiles all right with +universal mod.

Nice! Are you planning on reporting this upstream? It would be great for future versions of apr to not need patches for this.

[Veence (Vincent)]

I can, but I'm unsure of the result. Mainly, the problem is that configure scripts checking for sizeof(something) cannot handle properly simultaneous build for 32 and 64 archs. This is also the case for the Python25 configure script for which I opened a new ticket. This requires a change in the way configure handle things (m4 files and so on), and, frankly, I don't know where to report. Meanwhile, the workaround is to use patches…

[danielluke (Daniel J. Luke)]

Replying to 10.50@…:

I can, but I'm unsure of the result. Mainly, the problem is that configure scripts checking for sizeof(something) cannot handle properly simultaneous build for 32 and 64 archs. This is also the case for the Python25 configure script for which I opened a new ticket. This requires a change in the way configure handle things (m4 files and so on), and, frankly, I don't know where to report.

The dev@… mailing list would probably be a good place. You can let them figure out how they want to make things work, but even just reporting the issue and the solution you've found would be great.

Meanwhile, the workaround is to use patches…

Yep. Do you want to upload the patches you've used, or should I try to generate them based on your comments?

[danielluke (Daniel J. Luke)]

Replying to dluke@…:

The dev@… mailing list would probably be a good place. You can let them figure out how they want to make things work, but even just reporting the issue and the solution you've found would be great.

I take that back, you probably want to open an issue in their bugzilla:

​https://issues.apache.org/bugzilla/index.cgi

If you do, please comment here with the link/bugid so I can watch it there.

Thanks!

[Veence (Vincent)]

Okay, I will do that and let you know. By the way, the "official" patch should use __LP32__ and __LP64__ instead of __i386__ and __x86_64__ respectively. Both the first macros are meant to denote 32 or 64bit environements, regardless of the CPU (so it works also on PPC)

[Veence (Vincent)]

Bug 46233 on the Apache bugzilla tracker. Cheers!

[danielluke (Daniel J. Luke)]

​https://issues.apache.org/bugzilla/show_bug.cgi?id=46233

[tobypeterson]

I hardly think this type of thing qualifies as an upstream bug. The autotools infrastructure just isn't set up to handle multiple-arch building (understandably, since that's only Mac OS X). It's pretty easy to work around, see r42691 for a simple example of how to correct configure's output after the fact.

[danielluke (Daniel J. Luke)]

Replying to toby@…:

I hardly think this type of thing qualifies as an upstream bug.

I would strongly disagree. This is the kind of thing that's really only appropriately handled by the originating project because they're the ones in the position to know what assumptions are made by the configuration process and where they impact the code. We can, of course, patch things in macports, but ideally changes to fix building should get pushed upstream so that the macports modifications are not necessary.

The autotools infrastructure just isn't set up to handle multiple-arch building

There's nothing preventing anyone from using autotools to do multi-arch builds, and unless upstream is totally uninterested in incorporating changes to make things work, I don't know why we would be hostile to getting things fixed there.

(understandably, since that's only Mac OS X). It's pretty easy to work around, see r42691 for a simple example of how to correct configure's output after the fact.

Yes, it's not hard to use macports to patch things. Of course, in that revision you add a dependency on ed instead of using patch (which macports already requires).

[tobypeterson]

I suppose my point is that it's hardly a problem unique to apr, so it doesn't make sense to file a bug against apr. MacPorts is essentially "doing it wrong" when it comes to building universal ports, because it's running the configure script and hoping that the answers are right for all of the requested architectures. Of course, the alternative isn't any better.

The unfortunate reality is that cross-compiling requires a fair amount of extra work, and that does typically include "knowing" the correct answers for targetted architectures. With this reality in mind, patching config.h after the fact is hardly the worst option. As far as a dependency on ed is concerned... well, if you can find me a unix system without ed, let me know. :)

[danielluke (Daniel J. Luke)]

Replying to toby@…:

I suppose my point is that it's hardly a problem unique to apr, so it doesn't make sense to file a bug against apr.

Except that that's the best place to fix it, and being a portable API the people who work on it are likely to be interested in making things work for multi-arch builds.

MacPorts is essentially "doing it wrong" when it comes to building universal ports, because it's running the configure script and hoping that the answers are right for all of the requested architectures. Of course, the alternative isn't any better.

The only way things get better is if people are made aware of the issue.

With this reality in mind, patching config.h after the fact is hardly the worst option.

Except that it is often insufficient since one set of config.h values might not be valid for all architectures that are being compiled for and/or there could be other files that embed configure-time assumptions that don't make sense for multi-arch builds.

As far as a dependency on ed is concerned... well, if you can find me a unix system without ed, let me know. :)

It's unlikely, but possible for it not to be there. It's also unlikely but possible that it would be broken somehow. It's usually best to limit external dependencies as much as possible. It could also be moved at some point to somewhere outside of $PATH (or $PATH could be set differently in future versions of macports).

[ryandesign (Ryan Carsten Schmidt)]

Has duplicate #17748.

It looks like Apple has applied a lot more patches than just this in the version of APR (1.2.7) included with Mac OS X 10.5.6. Here's what all they did:

​http://www.opensource.apple.com/darwinsource/tarballs/other/apr-12.tar.gz

[jmroot (Joshua Root)]

Individual patches can be browsed here, BTW: ​http://www.opensource.apple.com/darwinsource/10.5.6/apr-12/apr/files/

[jbarrett@…]

Cc Me!

[jbarrett@…]

Uses individual arch builds and merges them together to produce universal. Copied from OpenSSL port.

[ryandesign (Ryan Carsten Schmidt)]

Since MacPorts 1.7.0 has been released, we now have the merge() procedure at our disposal with which we might be able to clean this up some. Also see #17972 for another approach to this kind of universal building.

You should supply diffs, not entire Portfiles, so we can see what you're changing, and also you mustn't assume MacPorts is installed in /opt/local; use the ${prefix} variable instead.

[mtalexander (Mike Alexander)]

Patch to use muniversal to build 32/64 universal versions of apr and apr-util

[mtalexander (Mike Alexander)]

I just attached (twice, due to a browser hiccup) a patch to use the muniversal port group to build 32/64 universal versions of apr and apr-util. I don't think it is possible to build ppc/intel universal versions on one machine since the configure script needs to run programs in the target architecture.

In order to make this work I had to make one small change to muniversal. It adds a merger_build_env array similar to the merger_configure_env array. The patch to muniversal also fixes a minor typo in a debug message.

I added code to the apr portfile that knows how to merge make files and shell scripts. This could be generalized and moved into muniversal. Perhaps the current merger_dont_diff array could be generalized to a merger_diff_how array with one value being "dont" and others being things like "header file", "make file", "shell script", or whatever.

The two copies of the attachment are identical. I would delete one of them, but I can't see a way to do it.

[MarcusCalhoun-Lopez (Marcus Calhoun-Lopez)]

Cc Me!

[MarcusCalhoun-Lopez (Marcus Calhoun-Lopez)]

Replying to mta@…:

In order to make this work I had to make one small change to muniversal. It adds a merger_build_env array similar to the merger_configure_env array. The patch to muniversal also fixes a minor typo in a debug message.

I made the proposed changes to muniversal in r46169.

[ryandesign (Ryan Carsten Schmidt)]

So using the original universal variant, we can build i386/ppc, and using Mike's patch for muniversal we can build ppc/ppc64 or i386/x86_64, but not ppc/i386 or ppc/i386/ppc64/x86_64? If so, then that's one step forward, one step back, and not an improvement in my book. Does using Vincent's earlier patch in this ticket allow a full 4-way universal build? If so, that is the solution we should use.

[mtalexander (Mike Alexander)]

This is a revised version of my previous patch that fixes some problems

[mtalexander (Mike Alexander)]

I just attached a new version of my previous patch (without the muniversal patch which is now in trunk). The previous version built apr and apr-util ok, but things that depended on them didn't build right. I think this patch fixes these problems.

[(none)]

Milestone Port Bugs deleted

[ulrich.kohlhase@…]

Maports 1.710, port files updated on Tue Apr 28

Objective: universal (i386 x86_64) builds of APR since Java 1.6 is x86_64 only.

Please see the Portfile-apr-util and Portfile-apr diff files attached and comments in the port files.

[ulrich.kohlhase@…]

Cc Me!

[danielluke (Daniel J. Luke)]

Replying to ulrich.kohlhase@…:

Maports 1.710, port files updated on Tue Apr 28

Objective: universal (i386 x86_64) builds of APR since Java 1.6 is x86_64 only.

Please see the Portfile-apr-util and Portfile-apr diff files attached and comments in the port files.

Can you give a brief summary of what is different in the patches you just uploaded from the ones that mta@… uploaded?

[ulrich.kohlhase@…]

I couldn't get the universal (i386 x86_64) builds to work, neither for apr nor for apr-util using the latest patches submitted by the previous submitter. I'm using the following macports.conf settings on the build machine (OS X Server Leopard, 10.5.6, Xcode 3.1.2):

# MACOSX_DEPLOYMENT_TARGET
universal_target	10.5

# the SDK "sysroot" to use
universal_sysroot	/Developer/SDKs/MacOSX10.5.sdk

# machine architectures
universal_archs		i386 x86_64

While the apr port in itself would work fine with minor changes only (PortGroup muniversal 1.0, chmod 755 apr-1-config after install), the apr_rules.mk and libtool generated in /usr/local/share/apr-1/build will not work for a apr-util universal (i386 x86_64) build. Using the mta patches, APR libtool creates 64bit object files in the i386 work folder and the build fails.

[jochen@…]

Cc Me!

[jochen@…]

See #20326 for the "mini-patch" I needed to compile apr and apr-util universal (i386 + x86_64).

[boris.dusek@…]

Cc Me!

[markus@…]

Cc Me!

[ryandesign (Ryan Carsten Schmidt)]

Has duplicate #21040.

[nox@…]

Cc Me!

[nox@…]

Cc Me!

[nox@…]

Cc Me!

[nox@…]

Universal support without muniversal PortGroup

[nox@…]

Universal support without muniversal PortGroup

[ryandesign (Ryan Carsten Schmidt)]

Oh cool. Thanks Anthony. I was just working on this too. Looks like you beat me to it.

In my version, I was going off of Apple's patches and ed scripts here:

​http://www.opensource.apple.com/source/apr/apr-23/apr-util/

For apr, I used their fix-apr.h.ed, fix-apr_private.h.ed and fix-apr_rules.mk.ed; I didn't understand what the other files were for. It looks like your method ends up being pretty similar. Did you also base your changes off of their patches or figure it out on your own?

You may want to use Apple's more specific regex for apr_rules.mk: s/-arch [a-z0-9_]* *//g instead of your s/-arch .*//

For apr-util, I'm glad you figured out what extra environment variables to set. I couldn't see what to do based on Apple's Makefile.

I also added

configure.universal_args-delete	--disable-dependency-tracking

to both apr and apr-util because the configure scripts say it is an unknown option.

[nox@…]

Erf, I do started my work with the Apple ed scripts, but I've taken them in apr-12 from the link above, you may want to double check my patch.

[nicos_pavlov@…]

Cc Me!

[peter.royal@…]

Cc Me!

[oleg.lomaka@…]

Cc Me!

[rlhamil]

Replying to 10.50@…:

The apr.h file should be changed again to look like this:

> #if defined (__x86_64__)
>   #define APR_SIZEOF_VOIDP 8
> #elif defined (__i386__)
>   #define APR_SIZEOF_VOIDP 4
> #endif

Modify as needed for PPC archs.

Why that and not simply

#define APR_SIZEOF_VOIDP (sizeof(void *))

[rlhamil]

Replying to rlhamil@…:

Why that and not simply

#define APR_SIZEOF_VOIDP (sizeof(void *))

Oops, I see now that won't work, sizeof() not being valid in a preprocessor conditional. That too could be worked around, except that they don't always include stdint.h (older platforms?) and so they can't count on having uintptr_t later on.

Old platforms stink...

[chad@…]

Cc Me!

[bgrupe27]

Cc Me!

[whitley@…]

Cc Me!

[whitley@…]

I'll note that nox's patches for apr and apr-util work for me for a +universal build under 10.6.1. Looking forward to the official patches when they land...

[macports@…]

Cc Me!

[steven@…]

Cc Me!

[domiman@…]

Cc Me!

[nox@…]

timeout, r59152

[danielluke (Daniel J. Luke)]

Replying to nox@…:

timeout, r59152

I don't think that that timeout commit is appropriate since I have been active on this bug report and haven't disappeared (I would have appreciated at least a ping from you before you went and committed the change).

Also - why didn't you rev-bump with your commit (+universal before and after install different files, right?)

Additionally, I want to keep this open as upstream will probably act on their bug at some point and we'll want to reflect their course of action in the port.

[nox@…]

Well, you haven't said anything on this ticket since 6 months so I thought you had forgotten about it. But when I think someone has forgotten about one of his tickets, I usually ping that person, guess my usual routine went wrong, I'm sorry.

I didn't rev-bump it because I forgot it did work on non-64-bit systems.

[ryandesign (Ryan Carsten Schmidt)]

I had meant to act on this ticket also, Daniel, since you hadn't commented on the latest discussion. I had meant to go through the differences between the upstream patches for 1.2.x, on which Anthony based the patch he has now committed, and the upstream patches for 1.3.x, on which I based my patches. But as usual I got distracted with other ports in the mean time. Your update to 1.3.9 prompted me to think about it again, since it broke my patch; that may have been Anthony's motivation now as well.

[kendallb@…]

I assume this is related, but I have been trying to rebuild all my MacPorts binaries in universal mode for i387 and x86_64, as I have some nasty programs that expect a 32-bit binary, so my 64-bit pure build on Snow Leopard caused some problems. Since aprutil does not correctly configure and builds a universal right now, the libaprutil-1.dylib only ends up as 64-bit binaries, when built as universal. So when I try to build apache2 as universal, it fails as this library doesn't have the i386 images in there.

So if this is fixed, can I assume that aprutil will then build properly as a universal binary, including this library?

[kendallb@…]

Cc Me!

[kendallb@…]

Cc Me!

[kendallb@…]

Cc Me!

[kendallb@…]

Ok, I just used the patch file attached to this bug to enable universal builds on my system with a local port file, and it worked perfectly! I was able to rebuild all of MySQL, Apache2 and PHP as universal binaries, along with all the dependencies.

Can someone push this live?

[ryandesign (Ryan Carsten Schmidt)]

Replying to nox@…:

timeout, r59152

Does this actually work for you, Anthony? On my system the patch fails to apply.

--->  Computing dependencies for apr
--->  Fetching apr
--->  Verifying checksum(s) for apr
--->  Extracting apr
--->  Applying patches to apr
Error: Target org.macports.patch returned: shell command " cd "/opt/local/var/macports/build/_Users_rschmidt_macports_dports_devel_apr/work/apr-1.3.9" && /usr/bin/patch -p0 < '/Users/rschmidt/macports/dports/devel/apr/files/patch-universal.diff'" returned error 1
Command output: patching file include/apr.h.in
patching file include/arch/unix/apr_private.h.in
Hunk #2 succeeded at 751 (offset 3 lines).
Hunk #3 succeeded at 767 (offset 3 lines).
Hunk #4 succeeded at 786 (offset 3 lines).
Hunk #5 succeeded at 827 (offset 3 lines).
patching file configure
Hunk #1 FAILED at 25775.
Hunk #2 succeeded at 29232 (offset -3672 lines).
Hunk #3 FAILED at 31850.
Hunk #4 FAILED at 32286.
Hunk #5 succeeded at 29338 (offset -7705 lines).
Hunk #6 succeeded at 29800 (offset -8527 lines).
Hunk #7 succeeded at 29857 (offset -8560 lines).
Hunk #8 succeeded at 30456 (offset -9717 lines).
Hunk #9 succeeded at 30473 (offset -9717 lines).
3 out of 9 hunks FAILED -- saving rejects to file configure.rej

[nox@…]

$ sudo port -fv patch apr +universal
--->  Computing dependencies for apr.
--->  Fetching apr
--->  Verifying checksum(s) for apr
--->  Checksumming apr-1.3.9.tar.bz2
--->  Extracting apr
--->  Extracting apr-1.3.9.tar.bz2
--->  Applying patches to apr
--->  Applying /opt/local/var/macports/sources/rsync.macports.org/release/ports/devel/apr/files/patch-universal.diff
patching file include/apr.h.in
patching file include/arch/unix/apr_private.h.in
$ port cat apr | head -n 1
# $Id: Portfile 59152 2009-10-10 14:41:17Z nox@macports.org $

Why is it trying to patch configure? Did I forgot to commit something?

[ryandesign (Ryan Carsten Schmidt)]

Looks like my working copy was messed up. I had a different version of patch-universal.diff, and "svn up" and "svn st" didn't notice. Works fine after deleting the "files" directory and "svn up"ing it again. Sorry for the false alarm.

So, how about that apr-util? :)

[korpios@…]

I just ran into the same problem as kendallb: apr-util +universal did not actually build a universal library under Snow Leopard, and thus universal builds depending on it (e.g., serf +universal) failed. Using the apr-util-universal.diff patch attached to this ticket on the apr-util portfile resolved the issue.

[michaelsafyan@…]

Replying to korpios@…:

I just ran into the same problem as kendallb: apr-util +universal did not actually build a universal library under Snow Leopard, and thus universal builds depending on it (e.g., serf +universal) failed. Using the apr-util-universal.diff patch attached to this ticket on the apr-util portfile resolved the issue.

Hi, could you explain how to apply the diff? And could you post the solution in the following thread? Thanks.: #22610