Skip to content
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

cmake: Fix "tidy" CI job #205

Merged
merged 2 commits into from
May 23, 2024
Merged

cmake: Fix "tidy" CI job #205

merged 2 commits into from
May 23, 2024

Conversation

hebasto
Copy link
Owner

@hebasto hebasto commented May 21, 2024

Fixes the first point from bitcoin#29790 (comment):

  1. Cannot open mapping file '/ci_container_base/ci/scratch/build-/contrib/devtools/iwyu/bitcoin.core.imp': No such file or directory.

Do not reference the `HOST` variable before defining it.
Fix `contrib/devtools/iwyu/bitcoin.core.imp` file location.
@hebasto
Copy link
Owner Author

hebasto commented May 22, 2024

See the recent push to bitcoin#29790 for the CI logs.

@@ -61,8 +61,6 @@ export CCACHE_COMPRESS=${CCACHE_COMPRESS:-1}
export CCACHE_DIR=${CCACHE_DIR:-$BASE_SCRATCH_DIR/.ccache}
# Folder where the build result is put (bin and lib).
export BASE_OUTDIR=${BASE_OUTDIR:-$BASE_SCRATCH_DIR/out}
# Folder where the build is done (dist and out-of-tree build).
export BASE_BUILD_DIR=${BASE_BUILD_DIR:-$BASE_SCRATCH_DIR/build-$HOST}
Copy link

Choose a reason for hiding this comment

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

Why isn't this a problem upstream?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Because that was suggested only in #142.

Copy link

Choose a reason for hiding this comment

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

HOST should be set (for cross) when importing the CI config in this line:

echo "Setting specific values in env"

So an alternative would be (in this context) to fall back to build-native, if the HOST isn't set.

But this is fine, too.

@hebasto
Copy link
Owner Author

hebasto commented May 23, 2024

Friendly ping @maflcko :)

Copy link

@maflcko maflcko left a comment

Choose a reason for hiding this comment

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

lgtm

@@ -61,8 +61,6 @@ export CCACHE_COMPRESS=${CCACHE_COMPRESS:-1}
export CCACHE_DIR=${CCACHE_DIR:-$BASE_SCRATCH_DIR/.ccache}
# Folder where the build result is put (bin and lib).
export BASE_OUTDIR=${BASE_OUTDIR:-$BASE_SCRATCH_DIR/out}
# Folder where the build is done (dist and out-of-tree build).
export BASE_BUILD_DIR=${BASE_BUILD_DIR:-$BASE_SCRATCH_DIR/build-$HOST}
Copy link

Choose a reason for hiding this comment

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

HOST should be set (for cross) when importing the CI config in this line:

echo "Setting specific values in env"

So an alternative would be (in this context) to fall back to build-native, if the HOST isn't set.

But this is fine, too.

@hebasto hebasto merged commit f5f951d into cmake-staging May 23, 2024
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Tests, benchmarks, fuzzing, CI etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants