Run set -x in a subshell

I apologise in advance if this is the wrong forum because I could not easily find a better way to start a discussion with the community.

This is a request for a new check and depends on whether the Bash programming community agrees with the style to enforce.

Running cd in a subshell

The Bash community and Shell have long agreed that cd should run in a subshell if the intent is to later return to current dir. Thus this:

(cd my_dir || exit "$?"
 do_something)

Is considered preferable to

cd my_dir || exit "$?"
do_something
cd -

Running set -x in a subshell

For the same reason I always turn set -x on in a subshell, thus instead of:

set -x
do_something
set +x

It is preferrable to do:

(set -x
 do_something)

Further discussion

Many programmers may not spend time worrying about how they do debugging, however use of set -x has production application as a simple and neat way to generate logging.

Consider for example this snippet from a tool I have written:

# Create the change set to import resources.
#
# Input Globals:
#   initial_template: The initial CloudFormation template used during import.
#   template_bucket_name: The template bucket.
#   stack_name: The stack name.
#
_create_change_set() {
  local resources_to_import base_name template_url

  resources_to_import="$(resources_to_import | jq -c .)"
  base_name="$(basename "$initial_template")"
  template_url="https://$template_bucket_name.s3.ap-southeast-2.amazonaws.com/$base_name"

  (set -x ; aws cloudformation create-change-set --stack-name "$stack_name" \
     --change-set-name "ImportChangeSet" --change-set-type "IMPORT" \
     --resources-to-import "$resources_to_import" --template-url "$template_url" \
     --capabilities "CAPABILITY_NAMED_IAM"

   aws cloudformation wait change-set-create-complete --stack-name "$stack_name" \
     --change-set-name "ImportChangeSet")

  # shellcheck disable=SC2181
  [[ "$?" -ne 0 ]] && _change_set_failure
}

# Execute the change set to complete importing the resources.
#
# Input Globals:
#   stack_name: The stack name.
#
_execute_change_set() {
  (set -x
   aws cloudformation execute-change-set --change-set-name "ImportChangeSet" --stack-name "$stack_name"
   aws cloudformation wait stack-import-complete --stack-name "$stack_name")
}

This pattern results in these benefits:

Clean, readable log output, where the caller can see both the outputs for commands in real time, along with the commands generating that output indicated clearly with a leading + in a way that will always make sense.
More concise source code without noisy calls to set +x
Use of a subshell here is (as far as I know) the only way to avoid the very ugly appearance of + set +x in your logs, which often will not make sense to the reader. Noting that the initial set -x already does not appear in the log output.

I am not aware of any negatives to this approach or any scenario where it is preferrable not to use the subshell.

Therefore, I am proposing to enforce this in the linter, and welcome further discussion!

Please note I would be happy to implement this myself if there is agreement that it is a good feature.

Speaking for myself, I probably wouldn't use such a ShellCheck test.

My use case of xtrace is for debugging. Whenever there's some section of a script that I'm working on, I'll enable xtrace a few lines prior and insert an exit "${LINENO}" or a : "{Halt:?}" a few lines after. Then, when the debugging is completed I'll remove or comment any debugging code. A longer script (>1000 lines) would have numerous debugging blocks between sections. Inserting additional subshell code would mean more code to keep track of, when parentheses could be separated by 100's of lines. It seems like that could get a little confusing.

In recent versions of Bash, xtrace, or any other feature of set, can also be temporarily enabled within the confines of any given function by using local -.

foo(){ local -; set -euxo pipefail; echo something; }

Functions can also be defined using forced subshells:

bar() ( set -x echo quux )

A separate use case is when I'll occasionally use Thompson-style comments (: $'foo:\t' "${foo}"), which are visible in xtrace, in place of the ubiquitous Bourne-style (#). I find that helps if any xtrace output is especially opaque. Also, with Thompson comments, debugging code at times can remain in the script.

Regarding cd, I prefer to avoid it, actually. The idea being that the filesystem is beyond my control and can change at any time. TOCTOU's are the main concern. I suppose I've just gotten used to it; but then I do use per-execution private temporary directories pretty often.

Cheers,

Wiley

On Fri, Oct 25, 2024, 5:05 PM Alex Harvey @.***> wrote:

Please note I would be happy to implement this myself if there is agreement that it is a good feature.

— Reply to this email directly, view it on GitHub https://github.com/koalaman/shellcheck/issues/3072#issuecomment-2439061922, or unsubscribe https://github.com/notifications/unsubscribe-auth/AUF2F2Z6A2PW6EMGZHGN5NLZ5LMDPAVCNFSM6AAAAABQUGX7Q2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMZZGA3DCOJSGI . You are receiving this because you are subscribed to this thread.Message ID: @.***>

An additional side to the subshell question is that any alterations of parameters which occur within subshells can be invisible to the "parent" process.

~ $ xx=aa

~ $ fn()( set -x; xx=bb )

~ $ fn

xx=bb

~ $ declare -p xx

declare -- xx="aa"

If an xtrace subshell was removed after debugging, that could alter the state of parameter values within the final script. Wiley

On Fri, Oct 25, 2024, 7:59 PM Wiley Young @.***> wrote:

Speaking for myself, I probably wouldn't use such a ShellCheck test.

My use case of xtrace is for debugging. Whenever there's some section of a script that I'm working on, I'll enable xtrace a few lines prior and insert an exit "${LINENO}" or a : "{Halt:?}" a few lines after. Then, when the debugging is completed I'll remove or comment any debugging code. A longer script (>1000 lines) would have numerous debugging blocks between sections. Inserting additional subshell code would mean more code to keep track of, when parentheses could be separated by 100's of lines. It seems like that could get a little confusing.

In recent versions of Bash, xtrace, or any other feature of set, can also be temporarily enabled within the confines of any given function by using local -.

foo(){ local -; set -euxo pipefail; echo something; }

Functions can also be defined using forced subshells:

bar() ( set -x echo quux )

A separate use case is when I'll occasionally use Thompson-style comments (: $'foo:\t' "${foo}"), which are visible in xtrace, in place of the ubiquitous Bourne-style (#). I find that helps if any xtrace output is especially opaque. Also, with Thompson comments, debugging code at times can remain in the script.

Regarding cd, I prefer to avoid it, actually. The idea being that the filesystem is beyond my control and can change at any time. TOCTOU's are the main concern. I suppose I've just gotten used to it; but then I do use per-execution private temporary directories pretty often.

Cheers,

Wiley

On Fri, Oct 25, 2024, 5:05 PM Alex Harvey @.***> wrote:

Please note I would be happy to implement this myself if there is agreement that it is a good feature.

— Reply to this email directly, view it on GitHub https://github.com/koalaman/shellcheck/issues/3072#issuecomment-2439061922, or unsubscribe https://github.com/notifications/unsubscribe-auth/AUF2F2Z6A2PW6EMGZHGN5NLZ5LMDPAVCNFSM6AAAAABQUGX7Q2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMZZGA3DCOJSGI . You are receiving this because you are subscribed to this thread.Message ID: @.***>

koalaman / shellcheck