search

rust-bitcoin / rust-bitcoin-maintainer-tools

Scripts, test vectors, and other things used by or across multiple repositories in the rust-bitcoin ecosystem.
Creative Commons Zero v1.0 Universal
5 stars 4 forks source link

Enable usage of machine set CRATES var #13

Closed tcharding closed 2 weeks ago

tcharding commented 3 weeks ago

Repos using the run_task script would like to set the CRATES env var by using

REPO_DIR=$(git rev-parse --show-toplevel)
CRATES="$(cargo metadata --no-deps --format-version 1 | jq -j -r '.packages | map(.manifest_path | rtrimstr("/Cargo.toml") | ltrimstr("'"$REPO_DIR"'/")) | join(" ")')"

I don't know exactly why but this results in some sort of square braces data type that seems to only work in a loop using for crate in $CRATES instead of the for crate in ${CRATES[@]} like we currently have.

apoelstra commented 3 weeks ago

Using $PWD like that will mean that the script doesn't work if it's run from the wrong directory. I think we have an env var set somewhere already which uses git rev-parse --show-toplevel or something to orient our scripts around a fixed directory.

tcharding commented 3 weeks ago

I'll come back to this after #11 merges.

tcharding commented 2 weeks ago

The commit hash from this branch is now used in https://github.com/rust-bitcoin/rust-bitcoin/pull/3201 to verify this works using CI.

tcharding commented 2 weeks ago

Shall we give @Kixunil a chance to take a look, especially since I copied his code to get the looping to work?

apoelstra commented 2 weeks ago

However I wonder why a global variable is used and why isn't the command to obtain the list of crates ran by this script.

It would be nice if it would default to using the command-generated list of crates (and same for a feature matrix) but we'll definitely want a way to override the command for some repos, at least temporarily.

apoelstra commented 2 weeks ago

Also Kix ought to be a maintainer on this repo.