Skip to content

ci: Add and document BASE_BUILD_DIR #18735

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.

Sign up for GitHub

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

Jump to bottom
Merged
maflcko merged 1 commit into from
Apr 28, 2020
Merged

ci: Add and document BASE_BUILD_DIR #18735

maflcko merged 1 commit into from
Apr 28, 2020

Conversation

@maflcko
Copy link
Member

@maflcko maflcko commented Apr 22, 2020 •
edited
Loading

Also fixes #18768

@practicalswift
Copy link
Contributor

practicalswift commented Apr 22, 2020

Concept ACK

@hebasto
Copy link
Member

hebasto commented Apr 22, 2020

Concept ACK.

@maflcko maflcko force-pushed the 2004-ciBaseBuildDir branch from aaaa2dd to fa14a8b Compare April 22, 2020 19:54
@hebasto
Copy link
Member

hebasto commented Apr 25, 2020

Wasn't BASE_BUILD_DIR removed in #17549?

@maflcko
Copy link
Member Author

maflcko commented Apr 25, 2020

Back then it was an alias for BASE_ROOT_DIR (the root of the git folder) and removed because it was redundant.

@maflcko maflcko force-pushed the 2004-ciBaseBuildDir branch from fa14a8b to fa6630d Compare April 26, 2020 15:06
@maflcko maflcko mentioned this pull request Apr 26, 2020
Closed
@hebasto
Copy link
Member

hebasto commented Apr 26, 2020

Also fixes #18768

Is this message expected:

it looks like wine32 is missing, you should install it.
multiarch needs to be enabled first.  as root, please
execute "dpkg --add-architecture i386 && apt-get update &&
apt-get install wine32"

@maflcko
Copy link
Member Author

maflcko commented Apr 26, 2020

no idea. This was removed in 59e9688

Seems unrelated to this pull.

hebasto
hebasto reviewed Apr 27, 2020
View reviewed changes
@hebasto
Copy link
Member

hebasto commented Apr 27, 2020

It is absolutely unrelated, but would you mind adding a typo fix in https://github.com/bitcoin/bitcoin/blob/fa6630d11a5a30f73e2e76d0804f518b9cba094d/ci/test/00_setup_env.sh#L62 s/:/:-/ ?

hebasto
hebasto approved these changes Apr 27, 2020
View reviewed changes
Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

ACK 1937d1ea445e9d10408941d3eca95bbca0ad6ac5

@maflcko maflcko force-pushed the 2004-ciBaseBuildDir branch from 1937d1e to fae1a37 Compare April 27, 2020 15:58
@maflcko
Copy link
Member Author

maflcko commented Apr 27, 2020

Done

hebasto
hebasto approved these changes Apr 27, 2020
View reviewed changes
Copy link
Member

@hebasto hebasto left a comment •
edited
Loading

Choose a reason for hiding this comment

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

re-ACK fae1a37bc55fc85cd179cdfb35f0b8b92dd69008, only a typo fixed since the previous review.

@hebasto
ci: Add and document BASE_BUILD_DIR
fae49f6 
Co-Authored-By: Hennadii Stepanov <[email protected]>
@maflcko maflcko force-pushed the 2004-ciBaseBuildDir branch from fae1a37 to fae49f6 Compare April 27, 2020 16:27
@maflcko
Copy link
Member Author

maflcko commented Apr 27, 2020

The "typo fix" doesn't pass, so I went ahead and reverted it. https://cirrus-ci.com/task/4815052404424704?command=ci#L20

I don't understand bash well enough to debug this, but this might be related: https://wiki.bash-hackers.org/syntax/pe#use_a_default_value

@hebasto
Copy link
Member

hebasto commented Apr 27, 2020

The "typo fix" doesn't pass, so I went ahead and reverted it. https://cirrus-ci.com/task/4815052404424704?command=ci#L20

I don't understand bash well enough to debug this, but this might be related: https://wiki.bash-hackers.org/syntax/pe#use_a_default_value

I'll look into it.
Also https://www.gnu.org/software/bash/manual/html_node/Shell-Parameter-Expansion.html

@hebasto hebasto mentioned this pull request Apr 28, 2020
Merged
@maflcko maflcko closed this Apr 28, 2020
@maflcko maflcko reopened this Apr 28, 2020
hebasto
hebasto approved these changes Apr 28, 2020
View reviewed changes
Copy link
Member

@hebasto hebasto left a comment •
edited
Loading

Choose a reason for hiding this comment

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

re-ACK fae49f6, which is essentially the same as the previously reviewed changes.

@maflcko
Copy link
Member Author

maflcko commented Apr 28, 2020

@maflcko maflcko merged commit a483ffd into bitcoin:master Apr 28, 2020
@maflcko maflcko deleted the 2004-ciBaseBuildDir branch April 28, 2020 15:51
maflcko pushed a commit that referenced this pull request Apr 29, 2020
Merge #18798: ci: Fix default retry script usage
ab91a0e 
45615de ci: Fix default retry script usage (Hennadii Stepanov)

Pull request description:

  On master (5352d14) `CI_RETRY_EXE=${CI_RETRY_EXE:retry}` works as a [Substring Expansion](https://www.gnu.org/software/bash/manual/html_node/Shell-Parameter-Expansion.html), and that is wrong.
  If `CI_RETRY_EXE` variable was unset initially, its new value becomes an empty string, but not "retry" as one could expect. Consequently, the `${CI_RETRY_EXE} ...` command does _not_ use `ci/retry/retry` script.

  This PR makes for `CI_RETRY_EXE` variable a usual parameter expansion, i.e., `${parameter:-word}`.

  Reference: #18735 (comment)

Top commit has no ACKs.

Tree-SHA512: 108173f6b2677979b9ddf2f9b9df4a6c56f5efa81c36543a1816bb3b984e42984bf3c83fe413ea3a5ca1e2317c4efb02fea7180a6b44863af7cfe6202e9cf94d
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Apr 29, 2020
Merge bitcoin#18798: ci: Fix default retry script usage
07691a2 
45615de ci: Fix default retry script usage (Hennadii Stepanov)

Pull request description:

  On master (5352d14) `CI_RETRY_EXE=${CI_RETRY_EXE:retry}` works as a [Substring Expansion](https://www.gnu.org/software/bash/manual/html_node/Shell-Parameter-Expansion.html), and that is wrong.
  If `CI_RETRY_EXE` variable was unset initially, its new value becomes an empty string, but not "retry" as one could expect. Consequently, the `${CI_RETRY_EXE} ...` command does _not_ use `ci/retry/retry` script.

  This PR makes for `CI_RETRY_EXE` variable a usual parameter expansion, i.e., `${parameter:-word}`.

  Reference: bitcoin#18735 (comment)

Top commit has no ACKs.

Tree-SHA512: 108173f6b2677979b9ddf2f9b9df4a6c56f5efa81c36543a1816bb3b984e42984bf3c83fe413ea3a5ca1e2317c4efb02fea7180a6b44863af7cfe6202e9cf94d
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@hebasto hebasto hebasto approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

ci: RUN_UNIT_TESTS_SEQUENTIAL is broken on Windows

3 participants

@maflcko @practicalswift @hebasto