From: Junio C Hamano <gitster@pobox.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: git@vger.kernel.org, "Jeff King" <peff@peff.net>,
"René Scharfe" <l.s.r@web.de>
Subject: Re: [PATCH 01/10] run-command.c: refactor run_command_*_tr2() to internal helpers
Date: Fri, 14 Oct 2022 11:22:18 -0700 [thread overview]
Message-ID: <xmqq4jw69sf9.fsf@gitster.g> (raw)
In-Reply-To: <patch-01.10-c1f701af6e8-20221014T153426Z-avarab@gmail.com> ("Ævar Arnfjörð Bjarmason"'s message of "Fri, 14 Oct 2022 17:40:13 +0200")
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> +static void run_command_set_opts(struct child_process *cmd, int opt)
> +{
> + cmd->no_stdin = opt & RUN_COMMAND_NO_STDIN ? 1 : 0;
> + cmd->git_cmd = opt & RUN_GIT_CMD ? 1 : 0;
> + cmd->stdout_to_stderr = opt & RUN_COMMAND_STDOUT_TO_STDERR ? 1 : 0;
> + cmd->silent_exec_failure = opt & RUN_SILENT_EXEC_FAILURE ? 1 : 0;
> + cmd->use_shell = opt & RUN_USING_SHELL ? 1 : 0;
> + cmd->clean_on_exit = opt & RUN_CLEAN_ON_EXIT ? 1 : 0;
> + cmd->wait_after_clean = opt & RUN_WAIT_AFTER_CLEAN ? 1 : 0;
> + cmd->close_object_store = opt & RUN_CLOSE_OBJECT_STORE ? 1 : 0;
> +}
This, combined with the change in the caller that loses the
corresponding lines, does make sense. But
> @@ -1019,24 +1031,26 @@ int run_command_v_opt_cd_env(const char **argv, int opt, const char *dir, const
> return run_command_v_opt_cd_env_tr2(argv, opt, dir, env, NULL);
> }
>
> +static int run_command_v_opt_cd_env_tr2_1(struct child_process *cmd, int opt,
> + const char *dir,
> + const char *const *env,
> + const char *tr2_class)
> +{
> + run_command_set_opts(cmd, opt);
> + cmd->dir = dir;
> + if (env)
> + strvec_pushv(&cmd->env, (const char **)env);
> + cmd->trace2_child_class = tr2_class;
> + return run_command(cmd);
> +}
This?
The original caller took argv and copied it into cmd.args itself and
the updated caller still does so because this new helper doesn't do
so for it, but it no longer does the copying of env because this
helper does it.
Unless we will add several more callers of this helper in later
steps, this sounds a bit too specialized to the way the first single
caller used to do it.
But let's keep reading to see if it is worth adding it.
> int run_command_v_opt_cd_env_tr2(const char **argv, int opt, const char *dir,
> const char *const *env, const char *tr2_class)
> {
> struct child_process cmd = CHILD_PROCESS_INIT;
> +
> strvec_pushv(&cmd.args, argv);
> - cmd.no_stdin = opt & RUN_COMMAND_NO_STDIN ? 1 : 0;
> - cmd.git_cmd = opt & RUN_GIT_CMD ? 1 : 0;
> - cmd.stdout_to_stderr = opt & RUN_COMMAND_STDOUT_TO_STDERR ? 1 : 0;
> - cmd.silent_exec_failure = opt & RUN_SILENT_EXEC_FAILURE ? 1 : 0;
> - cmd.use_shell = opt & RUN_USING_SHELL ? 1 : 0;
> - cmd.clean_on_exit = opt & RUN_CLEAN_ON_EXIT ? 1 : 0;
> - cmd.wait_after_clean = opt & RUN_WAIT_AFTER_CLEAN ? 1 : 0;
> - cmd.close_object_store = opt & RUN_CLOSE_OBJECT_STORE ? 1 : 0;
> - cmd.dir = dir;
> - if (env)
> - strvec_pushv(&cmd.env, (const char **)env);
> - cmd.trace2_child_class = tr2_class;
> - return run_command(&cmd);
> + return run_command_v_opt_cd_env_tr2_1(&cmd, opt, dir, env, tr2_class);
> }
>
> #ifndef NO_PTHREADS
next prev parent reply other threads:[~2022-10-14 18:22 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-14 15:40 [PATCH 00/10] run-command API: add run_command_{l,sv}_opt() Ævar Arnfjörð Bjarmason
2022-10-14 15:40 ` [PATCH 01/10] run-command.c: refactor run_command_*_tr2() to internal helpers Ævar Arnfjörð Bjarmason
2022-10-14 18:22 ` Junio C Hamano [this message]
2022-10-14 15:40 ` [PATCH 02/10] merge: remove always-the-same "verbose" arguments Ævar Arnfjörð Bjarmason
2022-10-14 18:31 ` Junio C Hamano
2022-10-14 15:40 ` [PATCH 03/10] run-command API: add and use a run_command_l_opt() Ævar Arnfjörð Bjarmason
2022-10-14 18:40 ` Junio C Hamano
2022-10-14 20:03 ` Jeff King
2022-10-14 20:27 ` Junio C Hamano
2022-10-14 15:40 ` [PATCH 04/10] am: use run_command_l_opt() for show_patch() Ævar Arnfjörð Bjarmason
2022-10-14 18:43 ` Junio C Hamano
2022-10-14 15:40 ` [PATCH 05/10] run-command API docs: clarify & fleshen out run_command_v_opt*() docs Ævar Arnfjörð Bjarmason
2022-10-14 15:40 ` [PATCH 06/10] run-command API: remove RUN_COMMAND_STDOUT_TO_STDERR flag Ævar Arnfjörð Bjarmason
2022-10-14 15:40 ` [PATCH 07/10] run-command API & diff.c: remove run_command_v_opt_cd_env() Ævar Arnfjörð Bjarmason
2022-10-14 15:40 ` [PATCH 08/10] run-command API & users: remove run_command_v_opt_tr2() Ævar Arnfjörð Bjarmason
2022-10-14 15:40 ` [PATCH 09/10] gc: use strvec_pushf(), avoid redundant strbuf_detach() Ævar Arnfjörð Bjarmason
2022-10-14 15:40 ` [PATCH 10/10] run-command API: add and use a run_command_sv_opt() Ævar Arnfjörð Bjarmason
2022-10-14 19:21 ` René Scharfe
2022-10-17 17:49 ` [PATCH v2 00/10] run-command API: add run_command_{l,sv}_opt() Ævar Arnfjörð Bjarmason
2022-10-17 17:49 ` [PATCH v2 01/10] run-command.c: refactor run_command_*_tr2() to internal helpers Ævar Arnfjörð Bjarmason
2022-10-17 17:49 ` [PATCH v2 02/10] merge: remove always-the-same "verbose" arguments Ævar Arnfjörð Bjarmason
2022-10-17 17:49 ` [PATCH v2 03/10] run-command API: add and use a run_command_l_opt() Ævar Arnfjörð Bjarmason
2022-10-17 17:49 ` [PATCH v2 04/10] am: use run_command_l_opt() for show_patch() Ævar Arnfjörð Bjarmason
2022-10-17 17:49 ` [PATCH v2 05/10] run-command API docs: clarify & fleshen out run_command_v_opt*() docs Ævar Arnfjörð Bjarmason
2022-10-17 17:49 ` [PATCH v2 06/10] run-command API: remove RUN_COMMAND_STDOUT_TO_STDERR flag Ævar Arnfjörð Bjarmason
2022-10-17 17:49 ` [PATCH v2 07/10] run-command API & diff.c: remove run_command_v_opt_cd_env() Ævar Arnfjörð Bjarmason
2022-10-17 17:49 ` [PATCH v2 08/10] run-command API & users: remove run_command_v_opt_tr2() Ævar Arnfjörð Bjarmason
2022-10-17 17:49 ` [PATCH v2 09/10] gc: use strvec_pushf(), avoid redundant strbuf_detach() Ævar Arnfjörð Bjarmason
2022-10-17 17:49 ` [PATCH v2 10/10] run-command API: add and use a run_command_sv_opt() Ævar Arnfjörð Bjarmason
2022-10-18 9:11 ` [PATCH v2 00/10] run-command API: add run_command_{l,sv}_opt() Junio C Hamano
2022-10-18 13:28 ` Ævar Arnfjörð Bjarmason
2022-10-18 20:42 ` Jeff King
2022-10-19 15:43 ` Junio C Hamano
2022-10-19 17:06 ` Jeff King
2022-10-19 18:00 ` Junio C Hamano
2022-10-19 18:57 ` Jeff King
2022-10-19 19:41 ` Junio C Hamano
2022-10-20 18:34 ` René Scharfe
2022-10-20 23:50 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqq4jw69sf9.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=l.s.r@web.de \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).