This is an archive of the discontinued LLVM Phabricator instance.

Switch test-release.sh to build with cmake by default (PR21561)
ClosedPublic

Authored by hans on Jun 24 2015, 3:01 PM.

Details

Summary

Here is a stab at switch the release script to build with cmake instead of autoconf. It retains an option to build with autoconf for platforms where that's necessary, such as Darwin.

Here is a diff between the resulting tarballs of building 3.6.1-rc1 on Linux with the old script vs. this new version:

.

I removed the -build-triple option. That seems to be intended for cross-compiling and won't work with cmake. Does anyone know if that's actually used for any of our release binaries?

I'm also considering removing the "do_64bit" thing, as I'm not entirely sure how that's supposed to work. For me, it seems to always build two 64-bit packages. Let me know what you think.

(I will hold off committing this in any case until 3.6.2 is out the door to avoid that getting built with the wrong version script.)

Diff Detail

Event Timeline

hans updated this revision to Diff 28409.Jun 24 2015, 3:01 PM
hans retitled this revision from to Switch test-release.sh to build with cmake by default (PR21561).
hans updated this object.
hans edited the test plan for this revision. (Show Details)
hans added subscribers: Unknown Object (MLST), hansw.
dsanders edited edge metadata.Jun 25 2015, 1:59 AM

I removed the -build-triple option. That seems to be intended for cross-compiling and won't work with cmake. Does > anyone know if that's actually used for any of our release binaries?

The Mips releases use it to workaround to a long-standing target triple bug. The problem is that config.guess reports mips64-linux-gnu/mips64el-linux-gnu for all of my test machines. This is fine for GCC because there's no real difference between those triples and mips-linux-gnu/mipsel-linux-gnu (both _usually_ emit 32-bit code by default). Unfortunately, the triples currently have strict meanings in LLVM and mips64/mips64el triples are unable to emit the 32-bit code that is used by the rest of the installed Debian. As a result, I have to tell ./configure that the build system is really mips-linux-gnu/mipsel-linux-gnu so that it does the right thing. The work I'm doing on target triples at the moment will make it possible to fix it but the actual fix is unlikely to be ready for LLVM 3.7.

For CMake, I have to set LLVM_TARGET_ARCH, and LLVM_HOST_TRIPLE instead.

I'm also considering removing the "do_64bit" thing, as I'm not entirely sure how that's supposed to work.

Me neither.

hans updated this revision to Diff 28484.Jun 25 2015, 11:33 AM
hans edited edge metadata.
  • Dropping 'do_64bit'
  • Adding '-configure-flags' options to allow e.g. setting LLVM_TARGET_ARCH, and LLVM_HOST_TRIPLE or --build to cmake or configure.
hans updated this revision to Diff 28513.Jun 25 2015, 4:20 PM

Tweak the file comparison step to upgrade 'Phase2' strings to 'Phase3' to ignore file path diffs in the debug info.

rengolin edited edge metadata.Jul 14 2015, 3:34 AM

About cross compiling, it does work with CMake, but maybe not with this script. Though, people running cross-builds could still use autoconf. Maybe the MIPS release is done in that way? I'm building native on both ARM and AArch64, so that's not a problem for me.

utils/release/test-release.sh
94

I actually use this option on ARM.

The problem here is that this script assumes x86_64, and that it builds 32 and 64 binaries. With this option, the script skips the 64-bit version. On ARM, if I don't use this, it will build two identical versions but call them 32-bit and 64-bit, which is just wrong and wastes time.

I see that you also removes the "Release-64" down there, so I believe that would be fine with me, too.

Maybe the MIPS release is done in that way? I'm building native on both ARM and AArch64, so that's not a problem for me.

I build native too. It's just that mips64-linux-gnu (from config.guess) should be synonymous with mips-linux-gnu (the triple used by the OS) on this OS but clang (currently) treats them as having different meanings.

Right, ok. Still, feels a bit wrong to disable cross-compiling on autoconf just because the default is CMake.

If it's possible to set the triple in the new flags option, should be ok. If not, maybe we should just keep it for a while, at least until we can cross-compile on CMake.

hans added a comment.Jul 14 2015, 12:50 PM

Right, ok. Still, feels a bit wrong to disable cross-compiling on autoconf just because the default is CMake.

If it's possible to set the triple in the new flags option, should be ok. If not, maybe we should just keep it for a while, at least until we can cross-compile on CMake.

I figured since everyone I know of is doing native builds, the "-build-triple" flag isn't getting much use anyway. If someone does want to pass the "--build=" flag to configure, they can do it with the "-configure-flags" option.

hans added inline comments.Jul 14 2015, 1:02 PM
utils/release/test-release.sh
94

On ARM, if I don't use this, it will build two identical versions but call them 32-bit and 64-bit, which is just wrong and wastes time.

It did the same for me on x86: two 64-bit versions. The whole set-up seemed broken, so I removed the "Release-64" flavour below and then there's no need for the '-no-64bit' flag anymore.

hans updated this revision to Diff 29709.Jul 14 2015, 1:49 PM
hans edited edge metadata.

Rebase.

rengolin added inline comments.Jul 15 2015, 3:25 AM
utils/release/test-release.sh
268

What I normally do in these cases is to set the command line to a variable, then just:

echo $cmd
$cmd

or something to that effect, controlling the output / errors as you want, later.

dsanders added inline comments.Jul 15 2015, 3:33 AM
utils/release/test-release.sh
268

Doesn't that cause problems with whitespace and quotes being reparsed?

hans added inline comments.Jul 15 2015, 9:04 AM
utils/release/test-release.sh
268

I worry about that too. I'd like to not change this at this moment.

hans added a comment.Jul 15 2015, 2:06 PM

I'm going to commit this now so we can tag 3.7.0-rc1 and start testing it. Please let me know if you have more concerns.

This revision was automatically updated to reflect the committed changes.

I'm going to commit this now so we can tag 3.7.0-rc1 and start testing it. Please let me know if you have more concerns.

No problem. I was about to say LGTM. My trial build with this patch and D6563 applied finished this morning. I have a few problems but none are directly related to the new build method except in so far as we now build projects we didn't in 3.6.2.

For reference the issues are in 'make check-all' and all except one are for projects that weren't enabled in LLVM 3.6.2.
Failing Tests (7):

AddressSanitizer-mips-linux :: TestCases/Linux/kernel-area.cc
AddressSanitizer-mips-linux :: TestCases/Posix/coverage-direct-large.cc
Clang :: Driver/cuda-options.cu
UBSan-ASan-mips :: TestCases/Float/cast-overflow.cpp
UBSan-Standalone-mips :: TestCases/Float/cast-overflow.cpp
libc++abi :: backtrace_test.pass.cpp
libc++abi :: test_demangle.pass.cpp

I had to kill an excessively long 'llc -march=r600 -mcpu=cypress' process (>500 minutes) to make the tests finish. I assume it's one of those failures.

hans added a comment.Jul 16 2015, 7:56 AM

I'm going to commit this now so we can tag 3.7.0-rc1 and start testing it. Please let me know if you have more concerns.

No problem. I was about to say LGTM. My trial build with this patch and D6563 applied finished this morning. I have a few problems but none are directly related to the new build method except in so far as we now build projects we didn't in 3.6.2.

For reference the issues are in 'make check-all' and all except one are for projects that weren't enabled in LLVM 3.6.2.
Failing Tests (7):

AddressSanitizer-mips-linux :: TestCases/Linux/kernel-area.cc
AddressSanitizer-mips-linux :: TestCases/Posix/coverage-direct-large.cc
Clang :: Driver/cuda-options.cu
UBSan-ASan-mips :: TestCases/Float/cast-overflow.cpp
UBSan-Standalone-mips :: TestCases/Float/cast-overflow.cpp
libc++abi :: backtrace_test.pass.cpp
libc++abi :: test_demangle.pass.cpp

I had to kill an excessively long 'llc -march=r600 -mcpu=cypress' process (>500 minutes) to make the tests finish. I assume it's one of those failures.

Great, thanks for testing! And please file bugs for the failing tests :)