Skip to content

Commit

Permalink
installShellFiles: ensure shell completion has more than 256 bytes
Browse files Browse the repository at this point in the history
I found an error in the completion script of tmuxp which were outputting
an error message instead of a proper script so a sanity check in the
installShellCompletion script is helpful to avoid this to happen in the
future.

After looking at possible ways to detect this, the only valid check I
found was based on the generated file size as any completion script I
could find had more than 256 bytes.

Signed-off-by: Otavio Salvador <[email protected]>
  • Loading branch information
otavio committed Aug 5, 2023
1 parent 83383ab commit 684bc07
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 6 deletions.
19 changes: 18 additions & 1 deletion pkgs/build-support/setup-hooks/install-shell-files.sh
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ installManPage() {
#
# If any argument is `--` the remaining arguments will be treated as paths.
installShellCompletion() {
local shell='' name='' cmdname='' retval=0 parseArgs=1 arg
local shell='' name='' cmdname='' size_check=256 retval=0 parseArgs=1 arg
while { arg=$1; shift; }; do
# Parse arguments
if (( parseArgs )); then
Expand All @@ -122,6 +122,17 @@ installShellCompletion() {
return 1
}
continue;;
--size-check)
size_check=$1
shift || {
echo 'installShellCompletion: error: --size-check flag expected an argument' >&2
return 1
}
continue;;
--size-check=*)
# treat `--size-check=foo` the same as `--size-check foo`
size_check=${arg#--size-check=}
continue;;
--cmd=*)
# treat `--cmd=foo` the same as `--cmd foo`
cmdname=${arg#--cmd=}
Expand Down Expand Up @@ -219,6 +230,12 @@ installShellCompletion() {
else
install -Dm644 -T "$arg" "$outPath"
fi || return
# Sanity check file size to avoid assuming completion has succeed but it generate an invalid file.
local fileSize=$(stat -c%s "$outPath")
if (( fileSize < size_check )); then
echo "installShellCompletion: error: $outPath is too small ($fileSize only), use '--size-check $fileSize' to override" >&2
return 1
fi
# Clear the per-path flags
name=
done
Expand Down
11 changes: 6 additions & 5 deletions pkgs/test/install-shell-files/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ recurseIntoAttrs {
echo qux > qux.zsh
echo quux > quux
installShellCompletion --bash foo bar --zsh baz qux.zsh --fish quux
installShellCompletion --size-check 4 --bash foo bar --zsh baz qux.zsh --fish quux

This comment has been minimized.

Copy link
@lucasew

lucasew Aug 5, 2023

Contributor

I think that stuff like this should be moved to that terraform commit and then change the name of that commit to a treewide.

nixpkgs-review is the whole afternoon already doing nix log in the derivations lol

This comment has been minimized.

Copy link
@otavio

otavio Aug 6, 2023

Author Contributor

I think that stuff like this should be moved to that terraform commit and then change the name of that commit to a treewide.

This is fixing the tests of the function; other packages should be added to a treewide commit, indeed.

nixpkgs-review is the whole afternoon already doing nix log in the derivations lol

I couldn't finish it here. I'd love to have the list of packages needing fixing so I could address them.

cmp foo $out/share/bash-completion/completions/foo
cmp bar $out/share/bash-completion/completions/bar
Expand All @@ -68,7 +68,7 @@ recurseIntoAttrs {
} ''
echo foo > foo
installShellCompletion --bash foo
installShellCompletion --size-check 4 --bash foo
# assert it didn't go into $out
[[ ! -f $out/share/bash-completion/completions/foo ]]
Expand All @@ -82,7 +82,7 @@ recurseIntoAttrs {
echo bar > bar
echo baz > baz
installShellCompletion --bash --name foobar.bash foo --zsh --name _foobar bar --fish baz
installShellCompletion --size-check 4 --bash --name foobar.bash foo --zsh --name _foobar bar --fish baz
cmp foo $out/share/bash-completion/completions/foobar.bash
cmp bar $out/share/zsh/site-functions/_foobar
Expand All @@ -93,7 +93,7 @@ recurseIntoAttrs {
echo bar > bar.zsh
echo baz > baz.fish
installShellCompletion foo.bash bar.zsh baz.fish
installShellCompletion --size-check 4 foo.bash bar.zsh baz.fish
cmp foo.bash $out/share/bash-completion/completions/foo.bash
cmp bar.zsh $out/share/zsh/site-functions/_bar
Expand All @@ -105,7 +105,7 @@ recurseIntoAttrs {
echo baz > baz.fish
echo qux > qux.fish
installShellCompletion --cmd foobar --bash foo.bash --zsh bar.zsh --fish baz.fish --name qux qux.fish
installShellCompletion --size-check 4 --cmd foobar --bash foo.bash --zsh bar.zsh --fish baz.fish --name qux qux.fish
cmp foo.bash $out/share/bash-completion/completions/foobar.bash
cmp bar.zsh $out/share/zsh/site-functions/_foobar
Expand All @@ -114,6 +114,7 @@ recurseIntoAttrs {
'';
install-completion-fifo = runTest "install-completion-fifo" {} ''
installShellCompletion \
--size-check 4 \
--bash --name foo.bash <(echo foo) \
--zsh --name _foo <(echo bar) \
--fish --name foo.fish <(echo baz)
Expand Down

0 comments on commit 684bc07

Please sign in to comment.