| Message ID | 4c59cab53a0d9bb7c9cccfaf5544ae5c904bb2ba.1617185147.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | trailer: pass arg as positional parameter | expand |
On Wed, Mar 31, 2021 at 12:05 PM ZheNing Hu via GitGitGadget <gitgitgadget@gmail.com> wrote: > > From: ZheNing Hu <adlternative@gmail.com> > > When we use subprocess to run a shell-script, if we have any Maybe: s/subprocess/a subprocess/ > args, git will add extra $@ to the end of the shell-script, > This can pass positional parameters correctly, But if we just > want to use some of these passed parameters, git will still > add an extra "$@", which contains all positional parameters we > passed. This does not meet our expectations. I am not sure explaining things using $@ is the best way to make this as clear as possible. I don't have a clear alternative right now though. > E.g. our shell-script is: > "echo \"\$1\"" > and pass $1 "abc", Maybe: s/pass $1 "abc"/we pass "abc" as $1/ > git will change our script to: > "echo \"\$1\" \"$@\"" Where will "abc" appear then? > The positional parameters we entered will be printed > repeatedly. If you take us passing "abc" in $1 as an example, then I think it's a good idea to show us the result of that. > So let add a new `shell_no_implicit_args` Maybe: s/`shell_no_implicit_args`/`shell_no_implicit_args` flag/ > to `struct child_process`, which can suppress the > joining of $@ if `shell_no_implicit_args` is set to 1, > this will allow us to use only few of positional args > in multi-parameter shell script, instead of using all > of them. I think our goal is more to have each argument we pass be passed just once.
Christian Couder <christian.couder@gmail.com> 于2021年4月1日周四 下午3:22写道： > > On Wed, Mar 31, 2021 at 12:05 PM ZheNing Hu via GitGitGadget > <gitgitgadget@gmail.com> wrote: > > > > From: ZheNing Hu <adlternative@gmail.com> > > > > When we use subprocess to run a shell-script, if we have any > > Maybe: s/subprocess/a subprocess/ > > > args, git will add extra $@ to the end of the shell-script, > > This can pass positional parameters correctly, But if we just > > want to use some of these passed parameters, git will still > > add an extra "$@", which contains all positional parameters we > > passed. This does not meet our expectations. > > I am not sure explaining things using $@ is the best way to make this > as clear as possible. I don't have a clear alternative right now > though. > > > E.g. our shell-script is: > > "echo \"\$1\"" > > and pass $1 "abc", > > Maybe: s/pass $1 "abc"/we pass "abc" as $1/ > > > git will change our script to: > > "echo \"\$1\" \"$@\"" > > Where will "abc" appear then? > > > The positional parameters we entered will be printed > > repeatedly. > > If you take us passing "abc" in $1 as an example, then I think it's a > good idea to show us the result of that. > > > So let add a new `shell_no_implicit_args` > > Maybe: s/`shell_no_implicit_args`/`shell_no_implicit_args` flag/ > Thanks for these grammar corrections. > > to `struct child_process`, which can suppress the > > joining of $@ if `shell_no_implicit_args` is set to 1, > > this will allow us to use only few of positional args > > in multi-parameter shell script, instead of using all > > of them. > > I think our goal is more to have each argument we pass be passed just once. More accurately, we only want those explicit positional parameters to be replaced. But Junio probably thinks it's OK to put on a layer of "sh -c" to "absorb" the "$@". I think it works, but it may cause some trouble for users. -- ZheNing Hu
diff --git a/run-command.c b/run-command.c index be6bc128cd9d..a2cf6177f522 100644 --- a/run-command.c +++ b/run-command.c @@ -264,7 +264,7 @@ int sane_execvp(const char *file, char * const argv[]) return -1; } -static const char **prepare_shell_cmd(struct strvec *out, const char **argv) +static const char **prepare_shell_cmd(struct strvec *out, const char **argv, int shell_no_implicit_args) { if (!argv[0]) BUG("shell command is empty"); @@ -281,7 +281,7 @@ static const char **prepare_shell_cmd(struct strvec *out, const char **argv) * If we have no extra arguments, we do not even need to * bother with the "$@" magic. */ - if (!argv[1]) + if (!argv[1] || shell_no_implicit_args) strvec_push(out, argv[0]); else strvec_pushf(out, "%s \"$@\"", argv[0]); @@ -416,7 +416,7 @@ static int prepare_cmd(struct strvec *out, const struct child_process *cmd) if (cmd->git_cmd) { prepare_git_cmd(out, cmd->argv); } else if (cmd->use_shell) { - prepare_shell_cmd(out, cmd->argv); + prepare_shell_cmd(out, cmd->argv, cmd->shell_no_implicit_args); } else { strvec_pushv(out, cmd->argv); } @@ -929,7 +929,7 @@ int start_command(struct child_process *cmd) if (cmd->git_cmd) cmd->argv = prepare_git_cmd(&nargv, cmd->argv); else if (cmd->use_shell) - cmd->argv = prepare_shell_cmd(&nargv, cmd->argv); + cmd->argv = prepare_shell_cmd(&nargv, cmd->argv, cmd->shell_no_implicit_args); cmd->pid = mingw_spawnvpe(cmd->argv[0], cmd->argv, (char**) cmd->env, cmd->dir, fhin, fhout, fherr); diff --git a/run-command.h b/run-command.h index d08414a92e73..9597c987c5bb 100644 --- a/run-command.h +++ b/run-command.h @@ -133,6 +133,7 @@ struct child_process { * argv[1], etc, do not need to be shell-quoted. */ unsigned use_shell:1; + unsigned shell_no_implicit_args:1; unsigned stdout_to_stderr:1; unsigned clean_on_exit:1; diff --git a/trailer.c b/trailer.c index be4e9726421c..35dd0f4c8512 100644 --- a/trailer.c +++ b/trailer.c @@ -231,6 +231,7 @@ static char *apply_command(const char *command, const char *arg) cp.env = local_repo_env; cp.no_stdin = 1; cp.use_shell = 1; + cp.shell_no_implicit_args = 1; if (capture_command(&cp, &buf, 1024)) { error(_("running trailer command '%s' failed"), cmd.buf);