feat: set and shopts directives #929

pd93 · 2022-11-10T14:30:07Z

Summary

This PR adds 2 new directives:

set - []string - Equivalent of the POSIX set builtin
shopts - []string - Equivalent of the Bash shopt builtin

Both of these directives are available at global, task and command level. See example Taskfile below:

version: '3'

# Global-level - Set for all commands in all tasks
set: [e] # Shorthand flags work for set too
shopts: [globstar]

tasks:
  default:
    # Task-level - Set for all command in this task
    set: [pipefail]
    shopts: [globstar]
    cmds:
      # Command-level - Set for just this command
      - cmd: shopt
        shopts: [globstar]
      - cmd: set -o
        set: [allexport]

Considerations

mvdan/sh allows us to set POSIX shell options via interp.Params(). However, from what I can tell, there is no equivalent for Bash options. This means that this PR is currently prepending the shopt builtin before each command where shopts are specified. I'm not a huge fan of this and have been looking at contributing a change to mvdan/sh to add a new interp.Shopts() function or similar. I'd love some input from @mvdan on whether this is a good approach.
Originally, I implemented both directives as a map[string]bool so that you could enable/disable opts at different levels. However, I came to the conclusion that this level of complexity wasn't really necessary as all the currently supported shell options are disabled by default anyway. Implementing as a []string still allows us to enable at each level and I think this covers most use cases. It also keeps the Taskfile looking clean as we can inline the list with [].
I did also consider merging the two directives as none of the option names conflict. However, this seems confusing and would require us to maintain a list of both inside Task so that we know how to pass them into the interpreter. I think its easier just to keep them separate.
The names set and shopts are definitely up for debate. I wasn't sure about the plurality for example...

Currently supported shell options

POSIX (`set`)

If you have gosh installed, you can find these by running gosh -c "set -o".

Option Name	Short Name	Default
`allexport`	`a`	off
`errexit`	`e`	off
`noexec`	`n`	off
`noglob`	`f`	off
`nounset`	`u`	off
`xtrace`	`x`	off
`pipefail`		off

Bash (`shopt`)

If you have gosh installed, you can find these by running gosh -c "shopt".

Option Name	Default
`expand_aliases`	off
`globstar`	off
`nullglob`	off

Don't have `gosh`?

If you don't have gosh installed, you can use the following Taskfile to get these lists:

version: '3'

tasks:
  default:
    cmds:
      - set -o
      - shopt

andreynering

First look.

I liked the direction here, but adding interp.BashOpts to mvdan/sh first would be great IMO.

andreynering · 2022-11-12T15:03:50Z

internal/execext/exec.go

+ // Manually adding bash opts before each command as there is no interp.Shopts
+ if len(opts.BashOpts) > 0 {
+ s = append(s, fmt.Sprintf("shopt -s %s", strings.Join(opts.BashOpts, " ")))
+ }


I agree on contributing to mvdan/sh adding a interp.BashOpts option. I think it may be even worth waiting for it to be released before merging this here (but if it takes too long, it's fine to merge as is and adapt later).

internal/execext/exec.go

pd93 · 2023-01-07T19:56:36Z

@andreynering I've fixed the issues you raised, but haven't had the time to look into the codebase over at mvdan/sh yet. I've opened an issue (mvdan/sh#962) over there instead. Hopefully I (or someone else) will have the time to work on this soon. In the meantime, if you're happy to merge this as-is, I can open another issue to track the improvements once (if) the functionality is added upstream.

- Rename `shopts` to `shopt` - Move `UniqueJoin` to a more appropriate package - Write documentation - Add entry to README

andreynering

Yes, let's get this merged and we can improve once mvdan/sh#962 gets implemented.

I've renamed shopts to shopt for consistency with the proper builtin. Also added documentation and README entry.

Thanks again!

See go-task/task#929 for details

marco-m · 2023-01-16T08:19:43Z

I love

tasks:
  default:
    # Task-level - Set for all command in this task
    set: [pipefail]

thanks!!!

pd93 · 2023-01-16T11:49:08Z

Yes, let's get this merged and we can improve once mvdan/sh#962 gets implemented.

I've renamed shopts to shopt for consistency with the proper builtin. Also added documentation and README entry.

@andreynering Thanks for tidying this up. I forgot to add the docs/schema etc. when marking as ready. I've opened #984 to track the improvements as discussed.

marco-m-pix4d · 2023-01-20T11:05:35Z

docs/docs/usage.md

+builtins. This can be added at global, task or command level.
+
+```yaml
+version: '2'


@pd93 I think that this is an error and should be instead:

version: '3'

?

@marco-m-pix4d good spot! yes, this should be version: '3'. I'll get this updated.

#929 (comment)

pd93 linked an issue Nov 10, 2022 that may be closed by this pull request

Add support for working with shell options #908

Closed

andreynering reviewed Nov 12, 2022

View reviewed changes

andreynering added the state: reviewed label Nov 12, 2022

cdfa mentioned this pull request Jan 7, 2023

Add support for working with shell options #908

Closed

feat: set and shopts directives

08f9c33

pd93 force-pushed the set-and-shopts branch from 166fe43 to 08f9c33 Compare January 7, 2023 19:42

pd93 mentioned this pull request Jan 7, 2023

interp: add BashOpts for shopt options, just like Params for set options mvdan/sh#962

Open

pd93 requested a review from andreynering January 7, 2023 19:57

pd93 marked this pull request as ready for review January 7, 2023 20:01

set and shopt improvements

0a3defe

- Rename `shopts` to `shopt` - Move `UniqueJoin` to a more appropriate package - Write documentation - Add entry to README

andreynering approved these changes Jan 14, 2023

View reviewed changes

andreynering merged commit 1c1be68 into master Jan 14, 2023

andreynering deleted the set-and-shopts branch January 14, 2023 19:41

pinpox added a commit to pinpox/lollypops that referenced this pull request Jan 15, 2023

Set shell options globally

9c1832e

See go-task/task#929 for details

pinpox mentioned this pull request Jan 15, 2023

Set shell options globally pinpox/lollypops#15

Merged

pd93 mentioned this pull request Jan 16, 2023

Improve shopt integration with mvdan/sh #984

Open

marco-m-pix4d reviewed Jan 20, 2023

View reviewed changes

andreynering added a commit that referenced this pull request Jan 29, 2023

Documentation: version: '2' -> version: '3'

1091a91

#929 (comment)

pd93 mentioned this pull request Jul 19, 2023

prettier pd93/task#1

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: set and shopts directives #929

feat: set and shopts directives #929

pd93 commented Nov 10, 2022 •

edited

andreynering left a comment

andreynering Nov 12, 2022

pd93 commented Jan 7, 2023 •

edited

andreynering left a comment

marco-m commented Jan 16, 2023

pd93 commented Jan 16, 2023

marco-m-pix4d Jan 20, 2023

pd93 Jan 20, 2023

feat: set and shopts directives #929

feat: set and shopts directives #929

Conversation

pd93 commented Nov 10, 2022 • edited

Summary

Considerations

Currently supported shell options

POSIX (set)

Bash (shopt)

Don't have gosh?

andreynering left a comment

Choose a reason for hiding this comment

andreynering Nov 12, 2022

Choose a reason for hiding this comment

pd93 commented Jan 7, 2023 • edited

andreynering left a comment

Choose a reason for hiding this comment

marco-m commented Jan 16, 2023

pd93 commented Jan 16, 2023

marco-m-pix4d Jan 20, 2023

Choose a reason for hiding this comment

pd93 Jan 20, 2023

Choose a reason for hiding this comment

pd93 commented Nov 10, 2022 •

edited

POSIX (`set`)

Bash (`shopt`)

Don't have `gosh`?

pd93 commented Jan 7, 2023 •

edited