Adds a allow_universal_binary to homebrew.

[GuillaumeDIDIER]

Adds a allow_universal_binary flag to make superenv handle non-standard universal builds under superenv
See #17352

[GuillaumeDIDIER]

This is what still remains to do.
Has anyone got a clue of how to extract the arch ?

[GuillaumeDIDIER]

Sorted out

[mistydemeo]

I think we still want superenv to be selecting which universal archs to set; it filters out PPC arches on Intel, etc.

[GuillaumeDIDIER]

So add a set of valid arches. How could this be handle to end with a validarchset={"i386", "x86_64"} in cc ?
Also this could be used to initialize in a cleaner way -Xarch_ handling (that piece of code is still on me computer, coming soon.

[GuillaumeDIDIER]

Thinking about it, if we can transfer a set from homebrew to cc there would be no need of the extra ccfg flag, the set itself would flag that we are doing a universal binary.

[GuillaumeDIDIER]

In that case we should add code so that we always get the arch we are building for in the set and detect universal_binary_allow from that.
Formula would actually set the arches they are building for, and that could be even stored in the keg .json file.

[GuillaumeDIDIER]

How could that be one though ?

[mistydemeo]

In #21636, we're adding new helpers to determine these kinds of things, and an environment variable will be propagated into superenv with the appropriate architectures - likely as a HOMEBREW_ARCHFLAGS string that carries -arch i386, etc.

[GuillaumeDIDIER]

If the environment var is correctly set up, independently of 'u' in ccfg, which actually tells cc to add those flags, that would work, but what I would have used is rather HOMEBREW_VALID_ARCHS, with a plain list of arches we are allowed to build, that is, those that we won't remove.
Is #21636 mature enough for me to pull it in or should I wait before using it.

[GuillaumeDIDIER]

I am testing that with qt (whose universal support was broken) see #21534.

[GuillaumeDIDIER]

Actually I need to make some more important modifications to refurbish -Xarch_ arguments.

[GuillaumeDIDIER]

bad news this meant much copy paste.

[jacknagel]

The total duplication of the flag filtering is really a non-starter.

[jacknagel]

This is not what I had in mind. We need a separate flag that does not automatically add -arch flags, but does not filter them out, either.

[GuillaumeDIDIER]

This is a different flag.
But if we don't set it in universal binary, cc will remove the -arch flags added by universal binary (I supposed the CCFG flags were case sensitive.

[jacknagel]

Any flags that we are explicitly adding should not end up going through the arg filtering. I see that you changed this, but I don't think that's a good idea.

[GuillaumeDIDIER]

This allows for de-duping -arch flags, and the line I added ensures that those flag are not filtered.

[MikeMcQuaid]

I'm still not entirely sure I understand why us adding the arch flags doesn't work for Qt? Is it because the ./configure or qmake screw up?

[MikeMcQuaid]

I appreciate the work doing here but you are speaking a lot in abstracts about what formulae "can" do and not specific problems with specific formulae. Superenv (and our chosen optimisation flags) work well as-is and are heavily tested at this point. If the problem is the Qt formula then we should try and fix that specifically in the formula rather than adding a new feature for a single, slightly broken, buildsystem.

[MikeMcQuaid]

I'd like to see a solution that fixes the Qt configure tests so we can just use ENV.universal_binary for the rest of the build process.

[mistydemeo]

Is the problem that superenv is massaging the compiler flags during the build but not configure, misleading Qt as to what it can/can't do?

[GuillaumeDIDIER]

@mistydemeo actually it is the contrary, the flags we add aren't expected by configure so it messes the tests.
In the build, qt handles all the flags correctly and we just duplicate the arch flag.

This PR is an answer to #17352, which also explain why I tend to speak in abstracts.
Qt is a direct application of the new feature, handling non standard universal binaries, because it is a non standard universal build.
CMake would be another and the mentioned issue shows that other formulae also preset non-standard universal binary (llvm I think)
I also fixed another superenv bug, "Xarched args" were simply removed, which broke Qt, but could also break other formula relying on -Xarch.
And also, anything that modifies superenv needs to be considered as something that can be used by other formula.
You can see what happens to qt with ENV.universal_binary, using -verbose passed to configure.
(Some test fail where they work perfectly in normal qt build and with this new fix, xcodeversion, dwarf2 and -Xarch (before building qmake) if I remember well)
Debug and optimization creeped into the discussion, because of the previous hack which disabled some optimization and are no longer needed with the fixed -Xarch support, and because I wondered why we strip all debug info. That's should be discussed somewhere else since it isn't the purpose of this PR, do whatever you want as for -no-sse3 -no-3d-now (bottle, always, never)
I leave on holidays tomorrow, so feel free to play with the code yourselves.

[mistydemeo]

@mistydemeo actually it is the contrary, the flags we add aren't expected by configure so it messes the tests.

Superenv only massages compiler arguments when called from make. It shouldn't be active during the configure phase.

[GuillaumeDIDIER]

As for duplication I think I am going to redesign this, filter Xarch_ and then refurbishing all the args.

[GuillaumeDIDIER]

There seem to be a superenv bug with gcc and llvm-gcc, both complain about cc1plus -arch i386 unrecognized option On ML I'll check SL if I can

[jacknagel]

That's probably because you're passing it as a single argument ("-arch #{...}") instead of two arguments.

[jacknagel]

I suggest we take a step back and make a very clear outline of the problem this is attempting to solve; I bet there is a simpler way to do it that doesn't involve such major surgery.

[GuillaumeDIDIER]

Actually I simply taked the -Xarch arg apart treated the same way as the normal args but separately and put them back. It looks like major surgery because all var names have been replaced, so github sees it a major change.
The refurbishing main code hasn't changed.
It was split up in two part and the main part was wrapped in a block executed for normal args and Xarch args
Still at some point I had actually added much more code (duplicated actually, which is bad or more accurately dangerous), I actually deleted most of this code in the bad... commit (which unfortunately contained an infinite loop, so it has a fitting name ;)

[GuillaumeDIDIER]

gcc and llvm-gcc back on tracks, I split in the place were there were spaces.

[jacknagel]

Some general comments:

• Looping over all the arguments multiple times seems inefficient, we should loop over everything once, collect the arguments you wish to process further and then do so in a separate loop.
• The hashes with the empty string as a key make things very unclear, let's find a better way.
• Variables should be snake_case without type suffixes (i.e. foo_bar, not fooBarSet or fooBarDict)
• Spaces around all operators (assignment, => in hashes, etc)
• Use do/end for multiline blocks

[GuillaumeDIDIER]

I'll fix the style error after diner.Instead of Empty string I'll use "all"I'll try to see if from all those blocks I can make one and loop over it
GuillaumeDIDIER
Date: Mon, 5 Aug 2013 10:07:07 -0700
From: notifications@github.com
To: homebrew@noreply.github.com
CC: guillaume.didier95@hotmail.fr
Subject: Re: [homebrew] Adds a allow_universal_binary to homebrew. (#21664)

Some general comments:

Looping over all the arguments multiple times seems inefficient, we should loop over everything once, collect the arguments you wish to process further and then do so in a separate loop.
The hashes with the empty string as a key make things very unclear, let's find a better way.
Variables should be snake_case without type suffixes (i.e. foo_bar, not fooBarSet or fooBarDict)
Spaces around all operators (assignment, => in hashes, etc)
Use do/end for multiline blocks

—
Reply to this email directly or view it on GitHub.

[GuillaumeDIDIER]

But by the way it seems that we are filtring debug and optimization, which makes the --with-debug-and-release option of qt.rb useless.

Date: Mon, 5 Aug 2013 10:07:07 -0700
From: notifications@github.com
To: homebrew@noreply.github.com
CC: guillaume.didier95@hotmail.fr
Subject: Re: [homebrew] Adds a allow_universal_binary to homebrew. (#21664)

Some general comments:

Looping over all the arguments multiple times seems inefficient, we should loop over everything once, collect the arguments you wish to process further and then do so in a separate loop.
The hashes with the empty string as a key make things very unclear, let's find a better way.
Variables should be snake_case without type suffixes (i.e. foo_bar, not fooBarSet or fooBarDict)
Spaces around all operators (assignment, => in hashes, etc)
Use do/end for multiline blocks

—
Reply to this email directly or view it on GitHub.

[GuillaumeDIDIER]

Due to the fact some well behaved build sysytem like quake handle debug and optimization themselves, I think we should add a allow debug and optimization kind of flag to CCFG, which would then leave debug and optimization related file from the build system. letter 'D' ?

[MikeMcQuaid]

The --debug-and-release option is still useful as it doesn't strip the backtrace and keeps the source around. I don't think QMake is a particularly well behaved buildsystem, personally.

[GuillaumeDIDIER]

Still It has been years since qt was born, and the Trolltech, Nokia and now Digia probably new what optimization flags they needed to use, and we most probably delete those flags. (which are handled knowing the system + compiler used for compiling) QMake is not the cleverest build system, but at least it keeps a lot of information on each platform, so that it uses some decents flags.
macx-g++ trigers the include not less than 8 .conf files, which handle everything, including debug, release and dwarf-2.
Debug flags : -g -> removed -g-dwarf-2 -> removed
Release -O2 -> removed
And qt opensource isn't broken due to those flags.

[GuillaumeDIDIER]

Sometimes some not very clever ./configure scripts will put wrong -O or debug flags, but CMake, qmake and other have enough knowledge not to break build with that and use appropriate flags, which we remove.
If we don't keep debug information (-g) I don't think Qt***_debug will be that useful, with or without the sources.

[GuillaumeDIDIER]

Qt 32 bit support with clang remains broken. (It's an upstream issue).

[GuillaumeDIDIER]

Qt built, --with-debug-and-release --universal --developer, using gcc

[GuillaumeDIDIER]

Rebased everything, and fixes the test-bot failure I hope

[GuillaumeDIDIER]

Well what is weird is the bot failure is that Mountain_lion claims the OS X version Qt is building on isn't supported, so OS X 10.9 must end up to be transmitted to the compiler. And Lion failed while recording the test result.
Isn't the SDK something WE set from superenv ?

[MikeMcQuaid]

No, Qt decides on its own SDK I'm pretty sure. Given that most other packages are working this seems to be a Qt bug.

[GuillaumeDIDIER]

I tested it on my mac and I've got 10.8 as SDK, that's weird.
And it seems that Qt 5 completely bypass superenv.
(It calls directly clang in Xcode.app)

[GuillaumeDIDIER]

I investigate a bit Qt configuration files.
Qt gets the SDK path using /usr/bin/xcodebuild -sdk macosx -version Path if we don't specify one.
(XCode 5.0.1 returns : /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.**9**.sdk)
the acceptable values are given by xcodebuild -showsdks

OS X SDKs:
    OS X 10.8                       -sdk macosx10.8
    OS X 10.9                       -sdk macosx10.9

For some weird reason Xcode 5.0.0 did not return the same as 5.0.1 (it return 10.8)
Should we ask Qt to use the SDK corresponding to the OS version it is building on ?
(ie macosx10.8 on 10.8)
Also it takes care of asking xcrun and xcodebuild the path of the tools, which he then calls using theirs full paths, which means that we cannot filter out what gets passed on to the compiler.
Also its use of CLT explains why we have hacks for non CLT.

[GuillaumeDIDIER]

What should I do ?
Give a hint to qt giving it "-sdk" "macosx#{MACVERSION}" ?

[MikeMcQuaid]

If it takes an SDK argument, go for it. It'll be:

'-sdk', "macosx#{MacOS::version}"

[GuillaumeDIDIER]

I tested that.
It seems to work.
We still have warnings while bootstrapping qmake since it use -sdk macosx anyway for bootstrap, but configure doesn't fail and the rest works correctly with the right SDK.

[GuillaumeDIDIER]

Could you trigger the test build again, please.
Jenkins did not take into account the last commit but still says that it failed.

[MikeMcQuaid]

Having the bots rebuild qt and qt5 every time you want to test isn't brilliant, really. Are you testing this locally too?

[GuillaumeDIDIER]

Well I test locally, but the bot tend not to agree with me.
That's why I was bothered by this failure.
Obviously, the next time I'll have to change something they will start again, so lets let them do something else.
Also the installed Qt Assistant does not work, but I'll file a separate issue.

[MikeMcQuaid]

@BrewTestBot test this please

[GuillaumeDIDIER]

As for Qt Assstant t seems that we didn't fix completely #20020.
It seems that QtWebkit build system forgets to handle install_name_tool, and assistant depends on QtWebkit.

[GuillaumeDIDIER]

Thanks for the test.
This time only Maverick failed.
But you should have a look at the bots.
Both Lion ad Mountain Lion passed all the tests but failed with a Java exception while recording the tests results.

What should be done while Qt isn't updated for maverick ?

[MikeMcQuaid]

That's because the log exhausts all the memory in the Java process. We can just leave Qt failing on Mavericks until upstream fix it.

[MikeMcQuaid]

(See #21000).

[cliffrowley]

Hi, I've managed to get Qt building and installing on Mavericks using some patches from the Qt code review. Please see my comment here: #21000 (comment)

[GuillaumeDIDIER]

So what should we do use those patch in qt5.rb, knowing that we'll have to remove them once the new release is out, or redirect people to the modified formula until the new release is out, or leave it as is ?

[cliffrowley]

The best way to use a modified formula is to create a fork of Homebrew, then create a branch, then modify / overwrite the relevant formula, and then brew install <link to raw formula URL>

[MikeMcQuaid]

@GuillaumeDIDIER Think it's unrelated to this issue.

[GuillaumeDIDIER]

So that's settled.
What else is there to fix, change, discuss in the pull request ?

[MikeMcQuaid]

I'm sorry but, after discussion, thought and given this is a fix for a singular formula (which already works in another way) we're not going to accept this patch.

[scott-vsi]

Sorry for commenting on an old thread, but am I to assume that Homebrew does not support building a universal binary (i386 and x86_64) of qt (4.8.5) on Mountain Lion? This thread seems to suggest so (although it was somewhat difficult to follow) and the build failed when I tried it. If that is the case, should the --universal flag be removed from the qt.rb formula?

[GuillaumeDIDIER]

I don't know in which state is qt 4 universal build.
It seems that though my PR was closed, superenv know tries not to mess up with configure flags, it seems.
Also Qt universal support was at the time broken with clang.
You should try gcc.
I think it should work.