Opened 7 weeks ago

Last modified 7 days ago

#31550 needs_revision enhancement

Fix shellcheck (and related) issues in start-tor-browser

Reported by: nickm Owned by: nickm
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-tbm, TorBrowserTeam201910
Cc: boklm Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

I got a warning when I was running the script, so I ran shellcheck on it and got a bunch of warnings.

Child Tickets

Change History (12)

Changed 7 weeks ago by nickm

comment:1 Changed 7 weeks ago by nickm

Status: assignedneeds_review

I've attached the patches. The first patch should be uncontroversial; you may want to consider the others more carefully. All are against the master branch of the tor-browser-build repository, which I hope is right. I haven't tested the output, since I can't get rbm working.

comment:2 Changed 7 weeks ago by gk

Keywords: TorBrowserTeam201908R added

#24755 might be relevant here as well.

comment:3 Changed 7 weeks ago by gk

Cc: boklm added
Keywords: tbb-tbm added

comment:4 Changed 6 weeks ago by gk

Keywords: TorBrowserTeam201909R added; TorBrowserTeam201908R removed

No August anymore.

comment:5 Changed 13 days ago by pili

Keywords: TorBrowserTeam201910R added; TorBrowserTeam201909R removed

We're now in October, moving September outstanding reviews to October

comment:6 Changed 7 days ago by boklm

Keywords: TorBrowserTeam201910 added; TorBrowserTeam201910R removed
Status: needs_reviewneeds_revision

In the first patch:

@@ -204,13 +203,13 @@ if [ -L "$myname" ]; then
        # make that function accessible to shell scripts.
 
        # If realpath is available, use it; it Does The Right Thing.
-       possibly_my_real_name="`realpath "$myname" 2>/dev/null`"
-       if [ "$?" -eq 0 ]; then
+       if myname="$(realpath "$myname" 2>/dev/null)"; then
                myname="$possibly_my_real_name"
        else
                # realpath is not available; hopefully readlink -f works.
-               myname="`readlink -f "$myname" 2>/dev/null`"
-               if [ "$?" -ne 0 ]; then
+               if myname="$(readlink -f "$myname" 2>/dev/null)"; then
+                       :
+               else
                        # Ugh.
                        complain "start-tor-browser cannot be run using a symlink on this operating system."
                fi

We are removing the line creating the variable possibly_my_real_name, but we are still using it with myname="$possibly_my_real_name" inside the if, so it does not look correct.

comment:7 Changed 7 days ago by boklm

Patch 3 (Stop using a heredoc in start-tor-browser) and 5 (Put curly quotes inside single quotes, so shellcheck believes them) look good to me, so I merged them to master as commits c5f30eb31a39a61fe300389fff1ff7adecb8b163 and c14d73615ba8d9e12ae29f0168babc0ecbe55052. I also updated the subject lines to include this bug number.

Patches 2 and 4 also look good to me, but they are changing the same areas as patch 1, so I will merge them after patch 1.

Note: See TracTickets for help on using tickets.