Skip to content

Commit aebc335

Browse files
committed
Change if ...; then false; else true; fi to ! ...
`! ...` is simpler, as well as easier to read in `just` output. The `if ...; then false; else true; fi` syntax seems to have been introduced in 72cf940, in a command that, at that time, was in the `Makefile`. It's unclear if there was a reason to prefer it at the time it was introduced. Both a `Makefile` and a `justfile` work fine with `!` as it is used here, in both cases it is preserved literally and passed to the shell, and in both cases the shell is by default `sh` on all systems. It looks like `!` may have been avoided to accommodate behavior of `bsdmake` (which was at one time the default `make` on macOS, and which remains the default `make` on various other systems), where at least in some versions an optimization to run commands without a shell is triggered even in the presence of a leading `!` and even though `!` is a shell keyword. This does happen today if `bsdmake` is installed on macOS via `brew`, but only with *simple* commands, as in a rule like this (if one fixes the indentation, to use tabs): hello: ! false Then: $ bsdmake hello ! false !:No such file or directory *** Error code 1 Whereas, with... hello: if false; then false; else true; fi ...the shell is used and it succeeds: $ bsdmake hello if false; then false; else true; fi However, this does not establish that the practice was needed even when the command was in a `Makefile`, because with... hello: ! false >/dev/null ...the presence of the redirection operator `>` is sufficient to cause a shell to be used, and it succeeds: $ bsdmake hello ! false >/dev/null All commands negated via `if ...; then false; else true; fi` in the history of `gitoxide` in the `Makefile` (as well as after they were moved to the `justfile`) seem to have used `>` redirection, because the point is to verify that a build command that should fail does fail, and it is distracting to show the full compiler output of the intended failure. So it seems `!` negation could have worked even with versions of `bsdmake` where it does not always work, and even when the commands were in the `Makefile`. (Though it may have been avoided in order to make immediately clear to human readers that no `!` misinterpretation would occur.) In any case, this is not in a `Makefile` at all anymore. Using `!` works locally and on CI, is easier to read, and the output is easier to understand, both in general and specifically in how it lines up with the explanatory comment. The output now looks like: # assure compile error occurs ! cargo check --features lean-async 2>/dev/null ! cargo check -p gitoxide-core --all-features 2>/dev/null ! cargo check -p gix-packetline --all-features 2>/dev/null ! cargo check -p gix-transport --all-features 2>/dev/null ! cargo check -p gix-protocol --all-features 2>/dev/null
1 parent 7a33e2a commit aebc335

File tree

1 file changed

+6
-6
lines changed

1 file changed

+6
-6
lines changed

justfile

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,16 +45,16 @@ check:
4545
cargo check --workspace
4646
cargo check --no-default-features --features small
4747
# assure compile error occurs
48-
if cargo check --features lean-async 2>/dev/null; then false; else true; fi
49-
if cargo check -p gitoxide-core --all-features 2>/dev/null; then false; else true; fi
50-
if cargo check -p gix-packetline --all-features 2>/dev/null; then false; else true; fi
51-
if cargo check -p gix-transport --all-features 2>/dev/null; then false; else true; fi
52-
if cargo check -p gix-protocol --all-features 2>/dev/null; then false; else true; fi
48+
! cargo check --features lean-async 2>/dev/null
49+
! cargo check -p gitoxide-core --all-features 2>/dev/null
50+
! cargo check -p gix-packetline --all-features 2>/dev/null
51+
! cargo check -p gix-transport --all-features 2>/dev/null
52+
! cargo check -p gix-protocol --all-features 2>/dev/null
5353
cargo tree -p gix --no-default-features -e normal -i imara-diff 2>&1 | grep warning # warning happens if nothing found, no exit code :/
5454
cargo tree -p gix --no-default-features -e normal -i gix-submodule 2>&1 | grep warning
5555
cargo tree -p gix --no-default-features -e normal -i gix-pathspec 2>&1 | grep warning
5656
cargo tree -p gix --no-default-features -e normal -i gix-filter 2>&1 | grep warning
57-
if cargo tree -p gix --no-default-features -i gix-credentials 2>/dev/null; then false; else true; fi
57+
! cargo tree -p gix --no-default-features -i gix-credentials 2>/dev/null
5858
cargo check --no-default-features --features lean
5959
cargo check --no-default-features --features lean-async
6060
cargo check --no-default-features --features max

0 commit comments

Comments
 (0)