Opened 14 months ago

Last modified 4 months 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-rbm, TorBrowserTeam202006, gitlab-tb-tor-browser-build
Cc: boklm Actual Points:
Parent ID: Points: 1
Reviewer: Sponsor:


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 (19)

Changed 14 months ago by nickm

comment:1 Changed 14 months 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 14 months ago by gk

Keywords: TorBrowserTeam201908R added

#24755 might be relevant here as well.

comment:3 Changed 14 months ago by gk

Cc: boklm added
Keywords: tbb-tbm added

comment:4 Changed 14 months ago by gk

Keywords: TorBrowserTeam201909R added; TorBrowserTeam201908R removed

No August anymore.

comment:5 Changed 13 months ago by pili

Keywords: TorBrowserTeam201910R added; TorBrowserTeam201909R removed

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

comment:6 Changed 13 months 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
                # 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."

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 13 months 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.

comment:8 Changed 12 months ago by pili

Keywords: TorBrowserTeam201911 added; TorBrowserTeam201910 removed

Moving tickets to November 2019

comment:9 Changed 12 months ago by pili

Points: 1

comment:10 Changed 11 months ago by pili

Keywords: TorBrowserTeam202001 added; TorBrowserTeam201911 removed

Moving tickets to January

comment:11 Changed 9 months ago by pili

Keywords: TorBrowserTeam202002 added; TorBrowserTeam202001 removed

Moving tickets to February

comment:12 Changed 8 months ago by pili

Keywords: TorBrowserTeam202003 added; TorBrowserTeam202002 removed

We are no longer in February, moving tickets

comment:13 Changed 7 months ago by sysrqb

Keywords: TorBrowserTeam202006 added; TorBrowserTeam202003 removed

Move tickets to 2020 June

comment:14 Changed 4 months ago by acat

Keywords: tbb-rbm gitlab-tb-tor-browser-build added; tbb-tbm removed
Note: See TracTickets for help on using tickets.