Skip to content

Commit 1db37d1

Browse files
committed
Don't generate cargo metadata eagerly in justfile
- Have the `justfile` wait to run `cargo metadata` until it is running a recipe that actually needs the information from it. (Currently, that information is always the `target` directory location, in whose `debug` subdirectory some binaries are found.) This avoids a delay if `cargo metadata` has not run. The length of the delay varied but was often noticeable on Windows. Because running it again (in the absence of a clean or relevant change) is cheap, it is not a performance problem that this runs the command multiple times instead of once. This also avoids error messages from `cargo metadata` if it can't complete, unless it is actually being used. Those messages didn't prevent other recipes (besides those that used the metadata) from running. But they created the appearance of failure, and also were misleading when they didn't come from the recipe being run. - Handle errors from `cargo metadata` more robustly, by causing recipes that actually need the result of `cargo metadata` to fail if it fail -- and to fail immediately before attempting any other operations -- rather than attempting to use empty data. The metadata are piped to `jq -r .target_directory`, which actually succeeds even if it doesn't receive any data, giving empty output. Before, an attempt was made to use the empty output in building paths meant to go to debug builds of some binaries. Because the command is now running only when the output is actually needed, it is fine to make it hard error when it fails. The natural way to do this would be to `set -o pipefail`. But while it is in the newest POSIX standard, there remain otherwise largely POSIX-compatible `sh` implementations in use that don't support it (koalaman/shellcheck#2555). Our test suite requires `bash`, so the next obvious choice would be to use a shebang recipe or script recipe for `bash`. The problem is that shebang and script recipes don't usually work on Windows, because `just` passes a Windows path with `\` separators to the shell unquoted. This often results in the shell trying to run a file whose name is transformed by treating them specially, then causing them to dropped as if by quote removal. For example, on a recipe named `hello` with a `#!/usr/bin/env bash` shebang: /bin/bash: C:UsersekAppDataLocalTempjust-BVbCgphello: No such file or directory (One way this happens is by argument processing in `cygwin1.dll` or `msys-2.0.dll`, which seek to bridge the gap between the Unix expectation that the caller is responsible for all expansions and the Windows expectation that the callee is responsible for some. See rust-lang/rust#82227. Even when only globbing is attempted, `\` can be treated specially if it seems to escape a wildcard, or to escape another `\` that seems to escape a wildcard, etc.) Fortunately, in this case we can just check if the result of trying to parse out the `target` directory path gave a nonempty result, and treat the failure to do that as a hard error. - Remove the `check-mode` recipe's need for cargo metadata, by having it run the `internal-tools` binary via `cargo run` rather than looking up where the `target` directory is and finding `it` in `debug` under it. That approach is needed for the paths that are passed to the journey test runner, but `check-mode` can just use `cargo`. The preceding build command could be removed, since `cargo run` builds unless the binary is up to date. But keeping them as separate commands may make the output more readable.
1 parent 892cd32 commit 1db37d1

File tree

1 file changed

+19
-15
lines changed

1 file changed

+19
-15
lines changed

justfile

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@
22
# ^ A shebang isn't required, but allows a justfile to be executed
33
# like a script, with `./justfile test`, for example.
44

5+
j := quote(just_executable())
6+
57
default:
6-
{{ quote(just_executable()) }} --list
8+
{{ j }} --list
79

810
alias t := test
911
alias c := check
@@ -191,35 +193,37 @@ unit-tests:
191193
unit-tests-flaky:
192194
cargo test -p gix --features async-network-client-async-std
193195

194-
target_dir := `cargo metadata --format-version 1 | jq -r .target_directory`
195-
ein := quote(target_dir / 'debug/ein')
196-
gix := quote(target_dir / 'debug/gix')
197-
jtt := quote(target_dir / 'debug/jtt')
198-
it := quote(target_dir / 'debug/it')
196+
# depend on this to pre-generate metadata, and/or use it inside a recipe as `"$({{ j }} dbg)"`
197+
[private]
198+
dbg:
199+
set -eux; \
200+
target_dir="$(cargo metadata --format-version 1 | jq -r .target_directory)"; \
201+
test -n "$target_dir"; \
202+
echo "$target_dir/debug"
199203

200204
# run journey tests (max)
201-
journey-tests:
205+
journey-tests: dbg
202206
cargo build --features http-client-curl-rustls
203207
cargo build -p gix-testtools --bin jtt
204-
tests/journey.sh {{ ein }} {{ gix }} {{ jtt }} max
208+
dbg="$({{ j }} dbg)" && tests/journey.sh "$dbg/ein" "$dbg/gix" "$dbg/jtt" max
205209

206210
# run journey tests (max-pure)
207-
journey-tests-pure:
211+
journey-tests-pure: dbg
208212
cargo build --no-default-features --features max-pure
209213
cargo build -p gix-testtools --bin jtt
210-
tests/journey.sh {{ ein }} {{ gix }} {{ jtt }} max-pure
214+
dbg="$({{ j }} dbg)" && tests/journey.sh "$dbg/ein" "$dbg/gix" "$dbg/jtt" max-pure
211215

212216
# run journey tests (small)
213-
journey-tests-small:
217+
journey-tests-small: dbg
214218
cargo build --no-default-features --features small
215219
cargo build -p gix-testtools
216-
tests/journey.sh {{ ein }} {{ gix }} {{ jtt }} small
220+
dbg="$({{ j }} dbg)" && tests/journey.sh "$dbg/ein" "$dbg/gix" "$dbg/jtt" small
217221

218222
# run journey tests (lean-async)
219-
journey-tests-async:
223+
journey-tests-async: dbg
220224
cargo build --no-default-features --features lean-async
221225
cargo build -p gix-testtools
222-
tests/journey.sh {{ ein }} {{ gix }} {{ jtt }} async
226+
dbg="$({{ j }} dbg)" && tests/journey.sh "$dbg/ein" "$dbg/gix" "$dbg/jtt" async
223227

224228
# Run cargo-diet on all crates to see that they are still in bound
225229
check-size:
@@ -260,7 +264,7 @@ find-yanked:
260264
# Find shell scripts whose +x/-x bits and magic bytes (e.g. `#!`) disagree
261265
check-mode:
262266
cargo build -p internal-tools
263-
{{ it }} check-mode
267+
cargo run -p internal-tools -- check-mode
264268

265269
# Delete gix-packetline-blocking/src and regenerate from gix-packetline/src
266270
copy-packetline:

0 commit comments

Comments
 (0)