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

depends: some base fixes/changes #7809

Merged
merged 4 commits into from Apr 15, 2016

Conversation

Projects
None yet
3 participants
@theuni
Member

theuni commented Apr 4, 2016

  1. Fix the annoying "unexpected operator" warning that shows up sometimes.
  2. Fix download fallbacks. Fixes #7627.
  3. Make sure to check all necessary checksums for cctools/qt.
  4. Ensure (make a good guess) that the host/build toolchains haven't changed since the previous build. If they have, invalidate the necessary packages.
  5. Add a salt option for adding extra info to 4. This would allow us to (for example) seed gitian packages with dpkg info, in order to invalidate the cache if certain conditions were met.

I'm pushing these all in one PR because most of these changes will cause full cache invalidation, so we may as well try to only do it once.

theuni added some commits Mar 3, 2016

depends: fix fallback downloads
In some cases, failed downloads wouldn't trigger a fallback download attempt.
Namely, checksum mismatches.
depends: create a hostid and buildid and add option for salts
These add very simple sanity checks to ensure that the build/host toolchains
have not changed since the last run. If they have, all ids will change and
packages will be rebuilt.

For more complicated usage (like parsing dpkg), HOST_ID_SALT/BUILD_ID_SALT may
be used to introduce arbitrary data to the ids.
depends: qt/cctools: fix checksum checksum tests
Checksums were being verified after download, but not again before extraction

@laanwj laanwj added the Build system label Apr 5, 2016

@MarcoFalke

This comment has been minimized.

Show comment
Hide comment
@MarcoFalke

MarcoFalke Apr 5, 2016

Member

So the patch to the fallback logic will fix checksum issues like https://travis-ci.org/bitcoin/bitcoin/jobs/112855470#L831 ?

But does it also fix the issues where the fallback logic itself appeared to fail? (c.f. https://travis-ci.org/bitcoin/bitcoin/jobs/112846086#L4033 )

Member

MarcoFalke commented Apr 5, 2016

So the patch to the fallback logic will fix checksum issues like https://travis-ci.org/bitcoin/bitcoin/jobs/112855470#L831 ?

But does it also fix the issues where the fallback logic itself appeared to fail? (c.f. https://travis-ci.org/bitcoin/bitcoin/jobs/112846086#L4033 )

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Apr 5, 2016

Member

Concept ACK, nice!

Member

laanwj commented Apr 5, 2016

Concept ACK, nice!

@theuni

This comment has been minimized.

Show comment
Hide comment
@theuni

theuni Apr 5, 2016

Member

Yes, it should fix both. It broadens the retry logic, so that a "try" becomes fetch+validate. If the first try fails, it's retried with the fallback url. In the first example, the download appears to succeed but the checksum doesn't match. Since the download was "successful", it doesn't retry. Now it retries the whole thing with the fallback url. A hacked test (i purposely broke the checksum):

cory@cory-i7:~/dev/bitcoin/depends(depends-updates)$ make download
Checksum missing or mismatched for qrencode source. Forcing re-download.
Fetching qrencode-3.4.4.tar.bz2 from https://fukuchi.org/works/qrencode/
2016-04-05 13:20:14 URL:https://fukuchi.org/works/qrencode//qrencode-3.4.4.tar.bz2 [369136/369136] -> "/home/cory/dev/bitcoin/depends/work/download/qrencode-3.4.4/qrencode-3.4.4.tar.bz2.temp" [1]
/home/cory/dev/bitcoin/depends/work/download/qrencode-3.4.4/qrencode-3.4.4.tar.bz2.temp: FAILED
sha256sum: WARNING: 1 computed checksum did NOT match
Fetching qrencode-3.4.4.tar.bz2 from https://bitcoincore.org/depends-sources
2016-04-05 13:20:15 URL:http://dev.bitcoincore.org/depends-sources/qrencode-3.4.4.tar.bz2 [369136/369136] -> "/home/cory/dev/bitcoin/depends/work/download/qrencode-3.4.4/qrencode-3.4.4.tar.bz2.temp" [1]
/home/cory/dev/bitcoin/depends/work/download/qrencode-3.4.4/qrencode-3.4.4.tar.bz2.temp: FAILED
sha256sum: WARNING: 1 computed checksum did NOT match
make[1]: *** [/home/cory/dev/bitcoin/depends/sources/download-stamps/.stamp_fetched-qrencode-qrencode-3.4.4.tar.bz2.hash] Error 1
make: *** [download-osx] Error 2

For the second, I'm not sure why that was a problem before. As-is, it looks like it should try to re-download. But as a test, I've forced a similar failure, and it works as intended.

cory@cory-i7:~/dev/bitcoin/depends(depends-updates)$ make download
Checksum missing or mismatched for qrencode source. Forcing re-download.
Fetching qrencode-3.4.4.tar.bz2 from https://fukuchi.org/works/qrencode/
OpenSSL: error:14094410:SSL routines:SSL3_READ_BYTES:sslv3 alert handshake failure
Unable to establish SSL connection.
Fetching qrencode-3.4.4.tar.bz2 from https://bitcoincore.org/depends-sources
OpenSSL: error:14094410:SSL routines:SSL3_READ_BYTES:sslv3 alert handshake failure
Unable to establish SSL connection.
make[1]: *** [/home/cory/dev/bitcoin/depends/sources/download-stamps/.stamp_fetched-qrencode-qrencode-3.4.4.tar.bz2.hash] Error 4
make: *** [download-osx] Error 2
Member

theuni commented Apr 5, 2016

Yes, it should fix both. It broadens the retry logic, so that a "try" becomes fetch+validate. If the first try fails, it's retried with the fallback url. In the first example, the download appears to succeed but the checksum doesn't match. Since the download was "successful", it doesn't retry. Now it retries the whole thing with the fallback url. A hacked test (i purposely broke the checksum):

cory@cory-i7:~/dev/bitcoin/depends(depends-updates)$ make download
Checksum missing or mismatched for qrencode source. Forcing re-download.
Fetching qrencode-3.4.4.tar.bz2 from https://fukuchi.org/works/qrencode/
2016-04-05 13:20:14 URL:https://fukuchi.org/works/qrencode//qrencode-3.4.4.tar.bz2 [369136/369136] -> "/home/cory/dev/bitcoin/depends/work/download/qrencode-3.4.4/qrencode-3.4.4.tar.bz2.temp" [1]
/home/cory/dev/bitcoin/depends/work/download/qrencode-3.4.4/qrencode-3.4.4.tar.bz2.temp: FAILED
sha256sum: WARNING: 1 computed checksum did NOT match
Fetching qrencode-3.4.4.tar.bz2 from https://bitcoincore.org/depends-sources
2016-04-05 13:20:15 URL:http://dev.bitcoincore.org/depends-sources/qrencode-3.4.4.tar.bz2 [369136/369136] -> "/home/cory/dev/bitcoin/depends/work/download/qrencode-3.4.4/qrencode-3.4.4.tar.bz2.temp" [1]
/home/cory/dev/bitcoin/depends/work/download/qrencode-3.4.4/qrencode-3.4.4.tar.bz2.temp: FAILED
sha256sum: WARNING: 1 computed checksum did NOT match
make[1]: *** [/home/cory/dev/bitcoin/depends/sources/download-stamps/.stamp_fetched-qrencode-qrencode-3.4.4.tar.bz2.hash] Error 1
make: *** [download-osx] Error 2

For the second, I'm not sure why that was a problem before. As-is, it looks like it should try to re-download. But as a test, I've forced a similar failure, and it works as intended.

cory@cory-i7:~/dev/bitcoin/depends(depends-updates)$ make download
Checksum missing or mismatched for qrencode source. Forcing re-download.
Fetching qrencode-3.4.4.tar.bz2 from https://fukuchi.org/works/qrencode/
OpenSSL: error:14094410:SSL routines:SSL3_READ_BYTES:sslv3 alert handshake failure
Unable to establish SSL connection.
Fetching qrencode-3.4.4.tar.bz2 from https://bitcoincore.org/depends-sources
OpenSSL: error:14094410:SSL routines:SSL3_READ_BYTES:sslv3 alert handshake failure
Unable to establish SSL connection.
make[1]: *** [/home/cory/dev/bitcoin/depends/sources/download-stamps/.stamp_fetched-qrencode-qrencode-3.4.4.tar.bz2.hash] Error 4
make: *** [download-osx] Error 2
@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Apr 6, 2016

Member

Tested ACK dc4ec6d with my crosstool-ng ARM build

Member

laanwj commented Apr 6, 2016

Tested ACK dc4ec6d with my crosstool-ng ARM build

build_id_string+=$(shell $(build_AR) --version 2>/dev/null)
build_id_string+=$(shell $(build_CXX) --version 2>/dev/null)
build_id_string+=$(shell $(build_RANLIB) --version 2>/dev/null)
build_id_string+=$(shell $(build_STRIP) --version 2>/dev/null)

This comment has been minimized.

@laanwj

laanwj Apr 6, 2016

Member

Do we also want to get the linker info?

@laanwj

laanwj Apr 6, 2016

Member

Do we also want to get the linker info?

This comment has been minimized.

@theuni

theuni Apr 8, 2016

Member

That's tricky because we don't use the linker directly. It's not specified in depends, because there's no generic way to handle it. I think in 99% of the cases, if the linker changed, one of the above would've changed as well.

@theuni

theuni Apr 8, 2016

Member

That's tricky because we don't use the linker directly. It's not specified in depends, because there's no generic way to handle it. I think in 99% of the cases, if the linker changed, one of the above would've changed as well.

This comment has been minimized.

@laanwj

laanwj Apr 15, 2016

Member

True - we never use ld directly, and we shouldn't, for example it would be quite annoying to figure out what the linker is (and what plugin it is using - turns out it is loading /opt/clang39/lib/LLVMgold.so) in my lto setup testing the clang master branch:

   CC="${CLANGPATH}/bin/clang " 
   CXX="${CLANGPATH}/bin/clang++" 
   CFLAGS="-flto -O2 -g" CXXFLAGS="-flto -O2 -g" LDFLAGS="-flto -O2 -fuse-ld=gold"
   RANLIB="${CLANGPATH}/bin/llvm-ranlib"

So yes just wanted to ask to be sure.

@laanwj

laanwj Apr 15, 2016

Member

True - we never use ld directly, and we shouldn't, for example it would be quite annoying to figure out what the linker is (and what plugin it is using - turns out it is loading /opt/clang39/lib/LLVMgold.so) in my lto setup testing the clang master branch:

   CC="${CLANGPATH}/bin/clang " 
   CXX="${CLANGPATH}/bin/clang++" 
   CFLAGS="-flto -O2 -g" CXXFLAGS="-flto -O2 -g" LDFLAGS="-flto -O2 -fuse-ld=gold"
   RANLIB="${CLANGPATH}/bin/llvm-ranlib"

So yes just wanted to ask to be sure.

@laanwj laanwj merged commit 11d9f6b into bitcoin:master Apr 15, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Apr 15, 2016

Merge #7809: depends: some base fixes/changes
11d9f6b depends: qt/cctools: fix checksum checksum tests (Cory Fields)
bb717f4 depends: fix "unexpected operator" error during "make download" (Cory Fields)
fe740f1 depends: fix fallback downloads (Cory Fields)
dc4ec6d depends: create a hostid and buildid and add option for salts (Cory Fields)

codablock added a commit to codablock/dash that referenced this pull request Sep 16, 2017

Merge #7809: depends: some base fixes/changes
11d9f6b depends: qt/cctools: fix checksum checksum tests (Cory Fields)
bb717f4 depends: fix "unexpected operator" error during "make download" (Cory Fields)
fe740f1 depends: fix fallback downloads (Cory Fields)
dc4ec6d depends: create a hostid and buildid and add option for salts (Cory Fields)

codablock added a commit to codablock/dash that referenced this pull request Sep 19, 2017

Merge #7809: depends: some base fixes/changes
11d9f6b depends: qt/cctools: fix checksum checksum tests (Cory Fields)
bb717f4 depends: fix "unexpected operator" error during "make download" (Cory Fields)
fe740f1 depends: fix fallback downloads (Cory Fields)
dc4ec6d depends: create a hostid and buildid and add option for salts (Cory Fields)

@schinzelh schinzelh referenced this pull request Oct 19, 2017

Closed

[WIP] Update build system to Bitcoin 0.13.2 #1692

22 of 24 tasks complete

codablock added a commit to codablock/dash that referenced this pull request Dec 20, 2017

Merge #7809: depends: some base fixes/changes
11d9f6b depends: qt/cctools: fix checksum checksum tests (Cory Fields)
bb717f4 depends: fix "unexpected operator" error during "make download" (Cory Fields)
fe740f1 depends: fix fallback downloads (Cory Fields)
dc4ec6d depends: create a hostid and buildid and add option for salts (Cory Fields)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment