Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature request: POSIX shell compliance #31

Closed
the-real-neil opened this issue Dec 6, 2017 · 23 comments
Closed

feature request: POSIX shell compliance #31

the-real-neil opened this issue Dec 6, 2017 · 23 comments
Assignees
Milestone

Comments

@the-real-neil
Copy link

Would an option to support the generation of POSIX-compliant shell code be too much? Genuinely asking because I'm curious and trying to gauge interest.

@matejak
Copy link
Owner

matejak commented Dec 6, 2017

AFAIK, when your script accepts only positional arguments, the generated code is POSIX-compliant. Try and correct me if I am wrong!
Regarding positional arguments, it is very difficult to handle them without bash arrays.

@rugk
Copy link

rugk commented Dec 8, 2017

Sorry, but no, not at all. You use many bash features like local variables there and such stuff. Also, it should of course work for all types of arguments.

So pasting your example code (for pos. arg. only, and /bin/bash replaced with /bin/sh) into shellcheck shows the following POSIX-related errors:

Line 25:
        local _ret=$2
        ^-- SC2039: In POSIX sh, 'local' is undefined.
 
Line 37:
        local first_option all_short_options
        ^-- SC2039: In POSIX sh, 'local' is undefined.
 
Line 39:
        first_option="${1:0:1}"
                      ^-- SC2039: In POSIX sh, string indexing is undefined.
 
Line 40:
        test "$all_short_options" = "${all_short_options/$first_option/}" && return 1 || return 0
                                     ^-- SC2039: In POSIX sh, string replacement is undefined.
 
Line 48:
_positionals=()
             ^-- SC2039: In POSIX sh, arrays are undefined.
Line 87:
                                _positionals+=("$1")
                                ^-- SC2039: In POSIX sh, += is undefined.
                                              ^-- SC2039: In POSIX sh, arrays are undefined.
 
Line 111:
        _positional_names=('_arg_first' '_arg_second' '_arg_third' )
                          ^-- SC2039: In POSIX sh, arrays are undefined.
 
Line 113:
        for (( ii = 0; ii < ${#_positionals[@]}; ii++))
        ^-- SC2039: In POSIX sh, arithmetic for loops are undefined.
                                                   ^-- SC2039: In POSIX sh, ++ is undefined.
 
Line 115:
                eval "${_positional_names[ii]}=\${_positionals[ii]}" || die "Error during argument parsing, possibly an Argbash bug." 1
                      ^-- SC2039: In POSIX sh, array references are undefined.
 
Line 130:
echo "Value of first argument: $_arg_first"
                               ^-- SC2154: _arg_first is referenced but not assigned.
 
Line 131:
echo "Value of second argument: $_arg_second"
                                ^-- SC2154: _arg_second is referenced but not assigned.

@matejak
Copy link
Owner

matejak commented Dec 9, 2017

The "referenced, but not assigned" errors can be mitigated by using the ARG_DEFAULTS_POS() macro. I have mentioned that if you don't use positional arguments, there are no arrays, so the script is POSIX compliant except the local keyword that crept in recently.
I can feel some demand for POSIX compliant output, and I am interested in your use case. Specifically:

  • Do you require output of Argbash to be POSIX-compliant, or you also want the Argbash itself to be free of bashisms?
  • What is your use case of scripts generated by Argbash that have to be POSIX-compliant?

@rugk
Copy link

rugk commented Dec 9, 2017

Only the output basically, what the script itself uses does hardly matter IMHO. And why one needs the output to be compatible should be clear: if your own script needs to be compatible or you just want it to be compatible for whatever strange reason (😉), this small output should be compatible, too.

The best case would be to just have a command line switch --posix, which makes the output POSIX-compatible (would also imply the macro ARG_DEFAULTS_POS then).

@the-real-neil
Copy link
Author

I'm a habitual #!/bin/sh user, but I don't mind replacing the generated shebang with the one I require. @rugk is correct: even eschewing positional args isn't enough; the bashisms don't work with, say, dash for example.

@the-real-neil
Copy link
Author

@matejak, @rugk ; before I start on my own patch, do either of you have something in the works? I don't want to start something duplicative.

@matejak
Copy link
Owner

matejak commented Dec 10, 2017

I plan to slightly refactor Argbash, so it is easier to add other outputs, such as POSIX output scripts, bash completion and manpage templates. I am grateful that you consider contributing to Argbash, and I recommend you to postpone your contribution for a week or two, when the refactoring is completed.

@the-real-neil
Copy link
Author

@matejak absolutely, and thanks very much for even considering this.

@the-real-neil
Copy link
Author

@matejak, how's the refactor coming?

@matejak
Copy link
Owner

matejak commented Mar 12, 2018

The refactoring is not perfect, but it allows to writing of another outputs with relative ease.

As the POSIX output seems to be important for quite a few people, I will also dedicate some time and energy to accomplish the goal.

I propose to identify what Argbash is able to accomplish now and how to achieve as much of it as possible while restricting to POSIX compliant shell code only. I think that even some of the current code could be more POSIX-friendly. Then, depending on the involvement you want to have, we can craft more concrete steps forward.

If you want, you can proceed with the first part right here in this issue.

@the-real-neil
Copy link
Author

Wow, @matejak, this is harder than it looks. Do I understand correctly that the bin/argbash executable script feeds itself into what will become the output script? Where and how in the source is this accomplished? Would this mechanism, together with the expectation of POSIX compliance, preclude the use of Bashisms within some (or all) of the bin/argbash source?

@rugk
Copy link

rugk commented Mar 18, 2018

I am also a bit surprised. Yet again let me say it is really enough, if the output is POSIX.

@the-real-neil
Copy link
Author

@rugk , I think the current mechanism would force the removal of all Bashisms within bin/argbash starting at the beginning of the script and ending here:

https://github.com/matejak/argbash/blob/master/bin/argbash#L231

And probably any references thereto.

@matejak
Copy link
Owner

matejak commented Mar 18, 2018

I am not sure if I understand you. I think that it is not worth the effort to make the argbash script POSIX-compliant. However, scripts generated by argbash can be POSIX-compliant with some effort. Before we get into implementation details, you can examine some generated parsing code and propose changes that would make it POSIX-compliant. Then, we can discuss how complex could that be and how to implement that behavior.

@the-real-neil
Copy link
Author

Ah, certainly. Okay, let's use the minimal example and put a fine point how the generated script is non-compliant. To clarify, I'm testing within an argbash git checkout directory with the following commands:

$ ./bin/argbash -o ./resources/examples/minimal.sh ./resources/examples/minimal.m4
$ /bin/sh ./resources/examples/minimal.sh foobar

Then, I iterate though the errors, "fixing" or commenting them out as I progress. What follow are the (dash-specific) errors I encountered before my fixes started interfering with the actual execution:

  • resources/examples/minimal.sh: 37: resources/examples/minimal.sh: Syntax error: "(" unexpected
  • resources/examples/minimal.sh: 92: resources/examples/minimal.sh: Syntax error: word unexpected (expecting ")")
  • resources/examples/minimal.sh: 111: resources/examples/minimal.sh: Syntax error: Bad for loop variable
  • resources/examples/minimal.sh: 104: resources/examples/minimal.sh: Bad substitution

Some thoughts regarding the treatment of positional arguments...

For the positional argument counting within handle_passed_args_count: the Bash array could be replaced with a string containing whitespace-delimited positional arguments and counted via for-loop.

For the positional argument name definitions within assign_positional_args: instead of a pair of Bash arrays for the keys and values, use a pair of temporary files, paste -d=, and eval the assignment.

@matejak
Copy link
Owner

matejak commented Mar 19, 2018

@Rubicks Your output got somehow mangled.
Anyway, I have figured out how to get rid of at least some arrays. I haven't caught your paste suggestion though. Could you provide a more complete example? I am not familiar with that utility.

@the-real-neil
Copy link
Author

the-real-neil commented Mar 19, 2018

@matejak , assume you have the following positional argument variable names in a temporary file like this:

vars=$(mktemp --suffix=.vars)
tee ${vars} <<'EOF'
foo
bar
qux
EOF

Let us further assume you have the following positional argument values in a temporary file like this:

vals=$(mktemp --suffix=.vals)
tee ${vals} <<'EOF'
561
1105
1729
EOF

To (pairwise) assign the latter values to the former variable names, you could do something like this:

for var_eq_val in $(paste -d= ${vars} ${vals}); do
    eval ${var_eq_val}
done

@matejak
Copy link
Owner

matejak commented May 11, 2018

So from the looks of things the POSIX output is quite close. It looks like that it will use getopts and won't allow mixing optional and positional arguments. I am not sure about long/short options support, lack of some bash variable substitution features would make support of long options really painful.

@the-real-neil
Copy link
Author

@matejak, what's the "correct" thing to do when a user requests posix-compliant output and long options? Error out with a message like "I can't do that"?

@matejak
Copy link
Owner

matejak commented May 11, 2018

Strict POSIX-compliance means that no long options are allowed in the first place - they are a GNU extension. Therefore, I think that I will just make sure that every optional argument has a short option, and the help message will tell the rest of the story.
I am not completely ruling out producing scripts that could be executed by POSIX shells with a feature set that would supersede one of POSIX scripts, but that would be the next step.

@matejak matejak added this to the 2.6.0 milestone May 19, 2018
@matejak
Copy link
Owner

matejak commented May 19, 2018

The posix_output branch should now support the strict POSIX-compliant parsing code. There are likely bugs which will be ironed out gradually, but the biggest work is probably already over!

@matejak
Copy link
Owner

matejak commented Jun 29, 2018

Resolved by #48

@matejak matejak closed this as completed Jun 29, 2018
@nothingnesses
Copy link

It would be nice if argbash is able to produce portable POSIX-compliant scripts that can also parse long options. For what it's worth, it does seem to be possible. I've only tried running it on dash in Linux though and so I'm not sure if it works elsewhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants