Opened 9 months ago

Closed 7 months ago

#28012 closed defect (fixed)

shellcheck: updateRustDependencies.sh issues

Reported by: rl1987 Owned by:
Priority: Medium Milestone: Tor: 0.4.0.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: technical-debt
Cc: Actual Points:
Parent ID: Points:
Reviewer: asn Sponsor:

Description

Shellcheck (​​​​​​https://github.com/koalaman/shellcheck) finds the following issues:


In updateRustDependencies.sh line 23:
HERE=`dirname $(realpath $0)`
     ^-- SC2006: Use $(..) instead of legacy `..`.
              ^-- SC2046: Quote this to prevent word splitting.
                         ^-- SC2086: Double quote to prevent globbing and word splitting.


In updateRustDependencies.sh line 24:
TOPLEVEL=`dirname $(dirname $HERE)`
         ^-- SC2006: Use $(..) instead of legacy `..`.
                  ^-- SC2046: Quote this to prevent word splitting.
                            ^-- SC2086: Double quote to prevent globbing and word splitting.


In updateRustDependencies.sh line 27:
CARGO=`which cargo`
      ^-- SC2006: Use $(..) instead of legacy `..`.
       ^-- SC2230: which is non-standard. Use builtin 'command -v' instead.


In updateRustDependencies.sh line 30:
    printf "Error: Couldn't find workspace Cargo.toml in expected location: %s\n" "$TOML"
                                                                              ^-- SC1117: Backslash is literal in "\n". Prefer explicit escaping: "\\n".


In updateRustDependencies.sh line 34:
    printf "Error: Couldn't find directory for Rust dependencies! Expected location: %s\n" "$VENDORED"
                                                                                       ^-- SC1117: Backslash is literal in "\n". Prefer explicit escaping: "\\n".


In updateRustDependencies.sh line 38:
    printf "Error: cargo must be installed and in your \$PATH\n"
                                                             ^-- SC1117: Backslash is literal in "\n". Prefer explicit escaping: "\\n".


In updateRustDependencies.sh line 41:
if test -z `cargo --list | grep vendor` ; then
           ^-- SC2046: Quote this to prevent word splitting.
           ^-- SC2006: Use $(..) instead of legacy `..`.


In updateRustDependencies.sh line 42:
    printf "Error: cargo-vendor not installed\n"
                                             ^-- SC1117: Backslash is literal in "\n". Prefer explicit escaping: "\\n".


In updateRustDependencies.sh line 45:
$CARGO vendor -v --locked --explicit-version --no-delete --sync $TOML $VENDORED
                                                                ^-- SC2086: Double quote to prevent globbing and word splitting.
                                                                      ^-- SC2086: Double quote to prevent globbing and word splitting.

Child Tickets

Change History (6)

comment:1 Changed 8 months ago by teor

Keywords: technical-debt added
Milestone: Tor: 0.3.6.x-final

Since these tickets are about technical debt, I'm tentatively putting them in 0.3.6.

To avoid issues like this in future, we could run shellcheck as part of "make check" (#28058).

comment:2 Changed 8 months ago by nickm

Milestone: Tor: 0.3.6.x-finalTor: 0.4.0.x-final

Tor 0.3.6.x has been renamed to 0.4.0.x.

comment:3 Changed 7 months ago by rl1987

Status: newneeds_review

https://github.com/torproject/tor/pull/531

I don't have Rust installed, so I'm not sure I didn't break anything here.

comment:4 Changed 7 months ago by dgoulet

Reviewer: asn

comment:5 Changed 7 months ago by asn

Status: needs_reviewmerge_ready

LGTM!

comment:6 Changed 7 months ago by nickm

Resolution: fixed
Status: merge_readyclosed

merged to master

Note: See TracTickets for help on using tickets.