-
Notifications
You must be signed in to change notification settings - Fork 37.7k
ci: Move travis workarounds to .travis.yml #19276
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
6e84e89
to
6fbe3b2
Compare
6fbe3b2
to
fa71667
Compare
Has anyone an explanation? |
@@ -33,9 +33,7 @@ if [ -z "$NO_DEPENDS" ]; then | |||
else | |||
SHELL_OPTS="CONFIG_SHELL=" | |||
fi | |||
# Temporary workaround for https://github.com/bitcoin/bitcoin/issues/16368 | |||
python3 -c 'import time; [print(".") or time.sleep(500) for _ in range(4)]' & |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the issue was that this print
did nothing, because by default flush=False
(see https://docs.python.org/3.8/library/functions.html#print ).
Testing locally and on travis x86, it did always happen to print, just not on travis arm.
bash -c "${CI_WAIT}" & # Print dots in case the tests take a long time to run | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This deletion is also necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI_WAIT
no longer exists and this should be a no-op
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, CI_WAIT
was dropped from the .travis.yml
in #18929.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK fa71667, I have reviewed the code and it looks OK, I agree it can be merged.
It seems odd to have travis related workarounds in the general ci config files. Fix that oddity by moving the travis related workarounds to the travis yaml file.
For unexplained reasons, this should also work around and thus close #19171