Skip to content

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Dec 20, 2016

@theuni This should be enough to get back the test, which was removed due to 142ffc7?

Edit: Also added an unrelated commit which removes a line in gitignore. (No longer needed, as the java comparision tool was removed)

@maflcko maflcko added the Tests label Dec 20, 2016
@maflcko maflcko added this to the 0.14.0 milestone Dec 20, 2016
@maflcko maflcko requested a review from theuni December 20, 2016 21:56
@fanquake
Copy link
Member

fanquake commented Dec 20, 2016

utACK fad896d, post #9376 merge.

Copy link
Member

@theuni theuni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fails in .17 sec, I assume it'll take < 1sec to pass. Looks good to me :)

@@ -60,6 +60,7 @@ script:
- mkdir build && cd build
- ../configure $BITCOIN_CONFIG_ALL $BITCOIN_CONFIG || ( cat config.log && false)
- make $MAKEJOBS $GOAL || ( echo "Build failure. Verbose build follows." && make $GOAL V=1 ; false )
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@theuni I'd rather have it build the distributed files, to catch errors like #9393

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though, this requires calling configure twice, I assume...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MarcoFalke Yea, I don't think there's any way around configuring twice for that. There is a built-in that uses the cache to save some time, though.

Just change the:

make $MAKEJOBS check VERBOSE=1

to

make $MAKEJOBS distcheck VERBOSE=1

I think that should actually work these days.

Copy link
Member

@laanwj laanwj Dec 21, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this takes extra time, we could have only one of the builds do this. The distdir is arch-independent, after all.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, if I change check to distcheck, it will compile twice, which is worse than configure twice?

@laanwj
Copy link
Member

laanwj commented Dec 21, 2016

Restarting travis now that #9393 and #9376 merged

@fanquake
Copy link
Member

Looks like it's taking just over 1 second on each build.

@laanwj
Copy link
Member

laanwj commented Dec 21, 2016

Looks like it's taking just over 1 second on each build.

Then it's not worth spending more time optimizing this, there's bigger fish to fry. Thanks for measuring.
ACK fad896d

@laanwj laanwj merged commit fad896d into bitcoin:master Dec 21, 2016
laanwj added a commit that referenced this pull request Dec 21, 2016
fad896d gitignore: Wipe line after java comp tool removal (MarcoFalke)
fad632e travis: make distdir (MarcoFalke)
@maflcko maflcko deleted the Mf1612-travisDistDir branch December 21, 2016 13:37
codablock pushed a commit to codablock/dash that referenced this pull request Jan 18, 2018
fad896d gitignore: Wipe line after java comp tool removal (MarcoFalke)
fad632e travis: make distdir (MarcoFalke)
andvgal pushed a commit to energicryptocurrency/gen2-energi that referenced this pull request Jan 6, 2019
fad896d gitignore: Wipe line after java comp tool removal (MarcoFalke)
fad632e travis: make distdir (MarcoFalke)
CryptoCentric pushed a commit to absolute-community/absolute that referenced this pull request Feb 26, 2019
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants