travis: upload binaries #13855

ken2812221 commented at 5:39 PM on August 2, 2018: contributor

Print out sha256 hash and upload the binaries to transfer.sh

This is the easiest way to test and verify what PR do.

Download it from travis-ci console
Verify the sha256 hash
Start testing the binaries

ken2812221 force-pushed on Aug 2, 2018

DrahtBot commented at 6:42 PM on August 2, 2018: member

Note to reviewers: This pull request conflicts with the following ones:

#13863 (travis: move script sections to files in .travis/ subject to shellcheck by scravy)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

fanquake added the label Tests on Aug 2, 2018

fanquake requested review from laanwj on Aug 2, 2018

scravy commented at 5:10 AM on August 3, 2018: contributor

This will upload the binaries for every travis job that runs. Maybe limit it to pull requests on the bitcoin repository?

practicalswift commented at 8:04 AM on August 3, 2018: contributor

@ken2812221 Can the binaries be downloaded from Travis CI console currently? Please update the PR description with scenarios that could be achieved post merge that cannot be achieved pre merge :-)

ken2812221 commented at 8:15 AM on August 3, 2018: contributor

This will upload the binaries for every travis job that runs. Maybe limit it to pull requests on the bitcoin repository?

People may want to test the master branch.

Can the binaries be downloaded from Travis CI console currently?

Yes, it prints out the URL at the end of each build.

Please update the PR description with scenarios that could be achieved post merge that cannot be achieved pre merge :-)

What do you mean?

practicalswift commented at 8:29 AM on August 3, 2018: contributor

@ken2812221 Oh, sorry I think I misunderstood you. The binaries are not available from download directly from Travis CI, but by uploading them to transfer.sh they are made available? If so it makes sense. I think the text "Download it from travis-ci console" fooled me into thinking the binaries were downloadable directly from Travis CI :-)

in .travis.yml:133 in cf4d9831de outdated

 129 | @@ -130,6 +130,7 @@ script:
 130 |      - if [ "$TRAVIS_EVENT_TYPE" = "cron" ]; then extended="--extended --exclude feature_pruning,feature_dbcrash"; fi
 131 |      - if [ "$RUN_TESTS" = "true" ]; then BEGIN_FOLD functional-tests; DOCKER_EXEC test/functional/test_runner.py --combinedlogslen=4000 --coverage --quiet --failfast ${extended}; END_FOLD; fi
 132 |  after_script:
 133 | +    - test ! -z $OUTDIR && for file in $(find $OUTDIR/bin -executable -type f); do curl --upload-file $file https://transfer.sh/$(sha256sum $file | cut -d' ' -f1)-$(basename $file); echo; done

practicalswift commented at 8:38 AM on August 3, 2018:

Please make it so that we don't fail in case of unavailability of transfer.sh. Handle both the timeout and downtime case.

ken2812221 commented at 12:25 PM on August 3, 2018:

Done

ken2812221 force-pushed on Aug 3, 2018

scravy commented at 1:00 PM on August 3, 2018: contributor

People may want to test the master branch. @ken2812221 fair enough, but it will also run for every fork (other peopls repositories), every branch, every push that anyone does, right?

scravy commented at 5:15 PM on August 3, 2018: contributor

I think the curl version in the travis image is quite an old one (the one shipped with ubuntu 14.04 trusty probably).

I see in the logs:

0.38s$ test ! -z $OUTDIR && for file in $(find $OUTDIR/bin -executable -type f); do curl --retry 3 --retry-delay 0 -m 60 --connect-timeout=10 --upload-file $file https://transfer.sh/$(sha256sum $file | cut -d' ' -f1)-$(basename $file); echo; done
curl: option --connect-timeout=10: is unknown
curl: try 'curl --help' or 'curl --manual' for more information
curl: option --connect-timeout=10: is unknown
curl: try 'curl --help' or 'curl --manual' for more information
curl: option --connect-timeout=10: is unknown
curl: try 'curl --help' or 'curl --manual' for more information
curl: option --connect-timeout=10: is unknown
curl: try 'curl --help' or 'curl --manual' for more information
curl: option --connect-timeout=10: is unknown
curl: try 'curl --help' or 'curl --manual' for more information

But I think after_script sections don't fail the build (?) hence all checks passed.

ken2812221 force-pushed on Aug 3, 2018

ken2812221 commented at 5:25 PM on August 3, 2018: contributor

@scravy I should not use =. That's my mistake.

ken2812221 commented at 5:32 PM on August 3, 2018: contributor

but it will also run for every fork (other peopls repositories), every branch, every push that anyone does, right?

I can't see any downside of doing this.

MarcoFalke commented at 5:57 PM on August 3, 2018: member

Hmm, we could really limit this to pull requests on our repo. Otherwise, the hoster might exclude travis ips for spamming, no?

Note that for the master branch, we already have nightly builds: https://bitcoin.jonasschnelli.ch/#nighly And for release branches, we have our usual gitian builds: https://bitcoincore.org/en/download/

ken2812221 force-pushed on Aug 3, 2018

ken2812221 commented at 6:43 PM on August 3, 2018: contributor

we could really limit this to pull requests on our repo. Otherwise, the hoster might exclude travis ips for spamming.

You really convinced me. I don't want to get any ban by any host.

promag commented at 9:13 PM on August 3, 2018: member

This is the easiest way to test and verify what PR do.

I feel that the correct way to do that is to checkout and build the PR branch. It's common to tweak the code while testing, especially the corresponding tests.

At the moment, from the reviewer/tester point of view, I don't see advantages of this and for that reason NACK.

travis: Upload binaries 48c354583f

ken2812221 force-pushed on Aug 5, 2018

ken2812221 commented at 2:04 AM on August 5, 2018: contributor

@promag It's easy to do testing and tweak the code for Linux, but it's complicate for Windows and cross compile things. No body want to do cross compiling, I think this PR is good for that.

laanwj commented at 2:12 PM on August 6, 2018: member

This has been discussed before, and I don't think it's a good idea. Uploading binaries for pull requests automatically gives e.g. malware authors a way to make and host semi-genuine-looking executables easily (and gives incentive to use our repository for this!).

Furthermore, the Travis executables aren't really intended to be distributed. They might, at least theoretically, be specially adapted for testing in a specific environment. It's not a gitian build.

I think @jonasschnelli / @marcofalke 's solution is better - have a real gitian build be triggered by a tag or otherwise, by maintainers.

ken2812221 closed this on Aug 6, 2018

ken2812221 deleted the branch on Aug 6, 2018

ken2812221 commented at 2:40 PM on August 6, 2018: contributor

@laanwj Thanks for your comment. Although this PR was rejected, I can still provide my own gitian build result in my PR, right?

laanwj commented at 2:46 PM on August 6, 2018: member

but I can still provide my own gitian-build result in my PR, right?

Sure, you can do what you want in your own PRs. My argument is only against doing it automatically for PRs in a "centralized" way.

MarcoFalke commented at 4:24 PM on August 6, 2018: member

It is still good to have to code around. If someone really wants to debug the travis binaries, they can lookup this pull and cherry-pick this patch into their travis yml.

DrahtBot locked this on Sep 8, 2021