-
Notifications
You must be signed in to change notification settings - Fork 38.7k
gitian: build on ubuntu 14.04 #6900
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Does this change the minimum compatible system glibc/libstdc++ version of the Linux binary? If the new glibc or libstdc++ added new symbols @theuni may need to add more wrappers to the compat library. |
It shouldn't. That's what the symbols check ( |
|
Well that was easy :) For Windows, patching seems like it'd be the easiest, but I'd worry about breaking checksums. I suppose if Tor did it that way, either it's not an issue or updating the checksums is doable as well. Will look into it. @sipa mentioned on IRC that depends are currently broken for him on his dev machine, and the fix is the change you made here to qt.mk. Rather than removing the hackish sed, let's make a patch which properly ifdefs, so that it'll work on old/new mingw. Also, I'd be more comfortable with this if we could wait for Trusty on Travis. I've pinged them to try to get an idea of a timeline. Obviously if it's going to drag on too long, we can just bump the compiler versions we use in our current env in order to simulate a bump to Trusty. |
Tor uses 'strip' to recompute the checksum. I'm not sure what they used to find the index of the bytes, as it is not a fixed offset into any section in the final file. Maybe they used the linker map like me. We could ask them.
Right - Qt did an attempt at this, but they apparantly got the version threshold wrong. Though it is not impossible that Ubuntu complicated this by backporting things into older releases. From Or maybe it is correct! this kind of armoring is missing completely for the use of
I'd like so, too. Depends on the time frame - IMO it would be nice to build 0.12 using the new compiler, as it appears to be pretty much a hassle free upgrade. |
Using sed for binary patching is not anything I've seen before, or would necessarily encourage (fragility as the surrounding data may move). They're patching binutils: https://gitweb.torproject.org/builders/tor-browser-bundle.git/commit/gitian/descriptors/windows/gitian-firefox.yml?id=fd14de1870b38987c25eba1d3f8ca87b22f3bb8b |
|
Wrote a script that overwrites the bytes given an executable and linker map, then calls strip to recompute the checksum. It was somewhat more involved than I thought as it has to parse the PE sections to find the file offset: https://gist.github.com/laanwj/01dc996c4755d19e134e But it works for 32/64 bit, tested using #!/bin/bash
$1 mtest.c -o mtest_1 -Wl,-Map=mtest.map
$1 mtest.c -o mtest_2 -Wl,-Map=mtest.map
$1 mtest.c -o mtest_3 -Wl,-Map=mtest.map
sha256sum mtest_1 mtest_2 mtest_3
~/projects/bitcoin/bitcoin/contrib/devtools/bitstomp.py mtest_1 mtest.map
~/projects/bitcoin/bitcoin/contrib/devtools/bitstomp.py mtest_2 mtest.map
~/projects/bitcoin/bitcoin/contrib/devtools/bitstomp.py mtest_3 mtest.map
sha256sum mtest_1 mtest_2 mtest_3 |
|
Now uses another approach (suggested by @theuni) to make the windows build deterministic, until binutils can be updated. |
47a9d6e to
4eb6d24
Compare
|
@theuni I replaced the qt PIDLIST_ABSOLUTE patching with a patch that should work for both old and new mingw. Should be ready for merge. |
|
Anyone testing this? |
|
Yes. Will post back shortly. On Thursday, November 5, 2015, Wladimir J. van der Laan <
|
|
I'll try this within the next couple of hours. |
|
@jonasschnelli , @fanquake any luck? |
|
Anyone? I'd like to move forward with this, don't think there is anything really holding this back anymore. |
|
I really think this is useful. $bin/make-base-vm --arch amd64 --suite trusty
-------------- snip -------------------
2015-11-16 13:47:09,510 INFO : Creating file systems
2015-11-16 13:47:09,666 INFO : mke2fs 1.42.12 (29-Aug-2014)
2015-11-16 13:47:09,666 INFO : The file /dev/mapper/loop0p1 does not exist and no size was specified.
2015-11-16 13:47:09,667 INFO : Cleaning up
2015-11-16 13:47:12,684 ERROR : Process (['mkfs.ext4', '-F', '/dev/mapper/loop0p1']) returned 1. stdout: , stderr: mke2fs 1.42.12 (29-Aug-2014)
The file /dev/mapper/loop0p1 does not exist and no size was specified.
Traceback (most recent call last):
File "/usr/local/bin/vmbuilder", line 24, in <module>
cli.main()
File "/usr/local/lib/python2.7/dist-packages/VMBuilder/contrib/cli.py", line 228, in main |
|
Have you tried the latest Python vmbuilder? That solved that issue for me, I haven't had a chance to continue testing after getting that initial issue On Monday, November 16, 2015, Jonas Schnelli [email protected]
|
|
@fanquake: Thanks! Damit. I forgot that Debian does not have the python vmbuilder tools over apt. Manually updated and it works now... Testing gitian building this PR now... |
|
On my debian 8 machine I got the following errors: Linux:
MinGW:OSX: |
4eb6d24 to
0340ffb
Compare
Forgot to add these. Also add a short description for each required library.
Remove sed-based qt PIDLIST_ABSOLUTE workaround, replace by a patch that works for both old (such as used by Travis and Ubuntu Precise) and new mingw (Ubuntu Trusty).
|
Rebased this to master - going to retry my builds |
|
Re-Kicked the builder... log-links above are invalid now. |
|
Getting this now on the windows build, since rebase: This is because of: The workaround to set |
|
Pushed a new commit - all builds including windows should work now. (when testing, if you used gitian on 0.12 before, make sure that you remove your caches for 0.12, as they will have been built using previous c++ compiler, causing non-determinism and compile/link errors) |
|
All three platforms are working here: https://bitcoin.jonasschnelli.ch/pulls/6900/ |
|
@jonasschnelli Are you building a release, or this pull request on top of master? My signatures don't match with yours currently. |
|
Building commit b082935 with the descriptors of commit b082935 should give: |
|
@fanquake: right. My build script always pulls a PR on top of master. A match would only be possible if you do the same (check the git HEAD~5 log): https://bitcoin.jonasschnelli.ch/pulls/6900/ But i'll try now to build b082935 (to compare with @laanwj 's hashes) |
|
Linux / OSX: match ~~Windows: no match (https://bitcoin.jonasschnelli.ch/pulls/6900/bitcoin-win-0.12-build.assert) ~~ Any ideas? |
|
|
|
Correction: window build matched. (Compared against the wrong file). Nice! |
b082935 to
2cecb24
Compare
2cecb24 doc: change suite to trusty in gitian-building.md (Wladimir J. van der Laan) 957c0fd gitian: make windows build deterministic (Wladimir J. van der Laan) 2e31d74 gitian: use trusty for building (Wladimir J. van der Laan) 0b416c6 depends: qt PIDLIST_ABSOLUTE patch (Wladimir J. van der Laan) 9f251b7 devtools: add libraries for bitcoin-qt to symbol check (Wladimir J. van der Laan)
|
ACK On Thursday, November 19, 2015, Wladimir J. van der Laan <
|
|
Thanks a lot for testing @fanquake @jonasschnelli |
|
@laanwj Sorry for missing this while traveling. For the cache issue, sounds like depends needs to take the output as |
Upstream gitian updates This PR pulls in all gitian-related PRs that have been merged upstream since 0.11.2. The only ones I left out were documentation-only PRs, because we removed `doc/gitian-building.md` at some point. Here are the commits applied here, in the order shown in `git log` (ie. last to first): - bitcoin/bitcoin#7283 - fa42a67 - fa58c76 - bitcoin/bitcoin#8175 - 74c1347 - bitcoin/bitcoin#8167 - 7e7eb27 - ad38204 - b676f38 - bitcoin/bitcoin#7776 - f063863 - bitcoin/bitcoin#7424 - a81c87f ~ we already partly applied - a8ce872 - f3d3eaf ~ we already partly applied - 475813b - ~~cd27bf5~~ X we already applied - bitcoin/bitcoin#7060 - 3b468a0 ~ we removed doc/gitian-building.md - ~~99fda26~~ X we removed doc/gitian-building.md - bitcoin/bitcoin#7251 - fa09562 - bitcoin/bitcoin#6900 - ~~2cecb24~~ X we removed doc/gitian-building.md - 957c0fd - 2e31d74 - ~~0b416c6~~ X we removed QT - 9f251b7 - bitcoin/bitcoin#6854 - 579b863 ~ we already partly applied Part of #540
This workaround was added as part of the switch to gitian building using Ubuntu 14.04 (bitcoin#6900). However, it should no longer be required, as we have switched to Bionic (bitcoin#13171), and that has a far newer version of binutils. binutils patch: https://sourceware.org/bugzilla/show_bug.cgi?id=16192
…escriptor bd3f5a9 build: remove mingw linker workaround from win gitian descriptor (fanquake) Pull request description: This workaround was added as part of the switch to gitian building using Ubuntu 14.04 (#6900). However, it should no longer be required, as we have switched to Bionic (#13171) and that has a far newer version of binutils. Original discussion: #6900 binutils patch: https://sourceware.org/bugzilla/show_bug.cgi?id=16192 ACKs for top commit: MarcoFalke: ACK bd3f5a9 theuni: ACK bd3f5a9 laanwj: ACK bd3f5a9 Tree-SHA512: 01a5789994decf8cdedf7aaa0a449d2100a77e2e6b422d6b9dd5a4ac3e2e0b538c3d43aae4a1c3713614782f3c6b09d8d8bb21c20e86ce3c1734183dedd02d0c
…itian descriptor bd3f5a9 build: remove mingw linker workaround from win gitian descriptor (fanquake) Pull request description: This workaround was added as part of the switch to gitian building using Ubuntu 14.04 (bitcoin#6900). However, it should no longer be required, as we have switched to Bionic (bitcoin#13171) and that has a far newer version of binutils. Original discussion: bitcoin#6900 binutils patch: https://sourceware.org/bugzilla/show_bug.cgi?id=16192 ACKs for top commit: MarcoFalke: ACK bd3f5a9 theuni: ACK bd3f5a9 laanwj: ACK bd3f5a9 Tree-SHA512: 01a5789994decf8cdedf7aaa0a449d2100a77e2e6b422d6b9dd5a4ac3e2e0b538c3d43aae4a1c3713614782f3c6b09d8d8bb21c20e86ce3c1734183dedd02d0c
This workaround was added as part of the switch to gitian building using Ubuntu 14.04 (dashpay#6900). However, it should no longer be required, as we have switched to Bionic (bitcoin#13171), and that has a far newer version of binutils. binutils patch: https://sourceware.org/bugzilla/show_bug.cgi?id=16192
This changes the VM environment for gitian building to use Ubuntu 14.04 (Trusty).
clock_gettime, but this is the same for old descriptors thus unrelated)Known problems:
Windows build is not deterministicSOLVED
Every link, a few bytes in the executable near the end of
.rodataare different. These are not a timestamp but apparently random.I spent some time tracking this down, eventually used a linker map to find that the bytes come from a ephermal object
ertr000001.o. This helped trace this problem to the functionpe_create_runtime_relocator_referenceinbinutils/ld/pe-dll.cwhich leaks a few bytes of heap to the executable by writing uninitialized data (!).This sounds serious, however apparently this was already solved by the Erinn Clark of the Tor project in 2013: https://sourceware.org/bugzilla/show_bug.cgi?id=16192 . Awesome.
Unfortunately Ubuntu hasn't included the new binutils in 14.04... @theuni any idea how to handle this? Tor, for a while, manually patched the bytes (well not manually I suppose - in a postprocessing step). Another option would be to use our own binutils, or use an even newer image (but that wouldn't be long term supported).