[0/3] run-command.h: rename "env_array" to "env"

Message

Ævar Arnfjörð Bjarmason April 6, 2022, 10:42 a.m. UTC

On Thu, Dec 16 2021, Jeff King wrote:

> On Thu, Dec 16, 2021 at 12:51:55PM +0100, Ævar Arnfjörð Bjarmason wrote:
>
>> Or maybe you're not interested in either case, which is also fine. It's
>> really not needed for anything other than to resolve the minor oddity
>> that we have two "struct strvec"'s, "args" and "env_array". It's just a
>> bit odd to have one with "_array" in the name, which is only because the
>> now-removed "env" was taken at the time.
>
> Yeah, FWIW I absolutely hated "env_array" when I added it, but didn't
> want to do the transition all at once, so needed another name. So I
> would be happy to see it become "env" now. ;)

Perhaps this follow-up to 832ec72c3e1 (Merge branch 'ab/run-command',
2021-12-15) will warm Jeff's heart if/when he returns from his git
sabbatical :)

Junio: We're in the RC period, but perhaps this change if we're ever
going to make it would be good to make around that time of patch
slowdown. I.e. for the first batch after release.

I've been holding onto this since around December, but when I checked
there was always 1-2 series that was adding new use of "env_array",
resolving that conflict didn't seem worth the hassle.

Right now this doesn't conflict with anything in "seen". The
scary-at-first diffstat is almost entirely in 2/3, and everything
except a two-line change in that commit is made by the coccinelle rule
added in 1/3.

So this series for reviewing is really:

 * A trivial coccinelle rule in 1/3
 * The trivial 2 line change in run-command.h in 2/3 that isn't made
   by that rule
 * Subsequent trivial cleanup in 3/3 changing s/env_array/env/g in
   comments & variable names

Ævar Arnfjörð Bjarmason (3):
  cocci: add a rename of "struct child_process"'s "env_array" to "env"
  run-command API: rename "env_array" to "env"
  run-command API users: use "env" not "env_array" in comments & names

 add-patch.c                          |  2 +-
 branch.c                             |  2 +-
 builtin/difftool.c                   |  6 ++--
 builtin/receive-pack.c               | 32 ++++++++++----------
 builtin/stash.c                      | 16 +++++-----
 builtin/submodule--helper.c          | 44 ++++++++++++++--------------
 builtin/worktree.c                   |  8 ++---
 connect.c                            |  9 +++---
 connected.c                          |  2 +-
 contrib/coccinelle/run_command.cocci | 11 +++++++
 daemon.c                             | 14 ++++-----
 editor.c                             |  2 +-
 hook.c                               |  2 +-
 http-backend.c                       |  4 +--
 object-file.c                        |  2 +-
 pager.c                              |  4 +--
 promisor-remote.c                    |  2 +-
 run-command.c                        | 19 ++++++------
 run-command.h                        | 14 ++++-----
 sequencer.c                          | 18 ++++++------
 submodule.c                          | 38 ++++++++++++------------
 submodule.h                          |  4 +--
 t/helper/test-run-command.c          |  2 +-
 trailer.c                            |  2 +-
 transport-helper.c                   |  2 +-
 wt-status.c                          |  2 +-
 26 files changed, 138 insertions(+), 125 deletions(-)
 create mode 100644 contrib/coccinelle/run_command.cocci

Comments

Johannes Schindelin April 7, 2022, 12:09 p.m. UTC | #1

Hi Ævar,

On Wed, 6 Apr 2022, Ævar Arnfjörð Bjarmason wrote:

> Junio: We're in the RC period, but perhaps this change if we're ever
> going to make it would be good to make around that time of patch
> slowdown. I.e. for the first batch after release.

Good thinking about the first batch after release. However, please do send
such cleanups really only after the release. The reason for the patch
slowdown is that we want to be able to focus on making a rock-solid
v2.36.0. This here patch series is very much a distraction at this point.

Thank you,
Johannes

Junio C Hamano April 7, 2022, 4:48 p.m. UTC | #2

Johannes Schindelin <[e-mail]> writes:

> On Wed, 6 Apr 2022, Ævar Arnfjörð Bjarmason wrote:
>
>> Junio: We're in the RC period, but perhaps this change if we're ever
>> going to make it would be good to make around that time of patch
>> slowdown. I.e. for the first batch after release.
>
> Good thinking about the first batch after release. However, please do send
> such cleanups really only after the release. The reason for the patch
> slowdown is that we want to be able to focus on making a rock-solid
> v2.36.0. This here patch series is very much a distraction at this point.

OK.

FWIW, I have it near the tip of 'seen', but if any fix that is more
urgent than these changes need to be added to 'next' and down to
'master' makes the version queued no longer apply, I will drop it
immediately, instead of spending cycles to figure out if it is worth
resolving the conflict and requeuing it in 'seen' during the -rc
period ;-)

Junio C Hamano April 7, 2022, 5:52 p.m. UTC | #3

Junio C Hamano <[e-mail]> writes:

>> Good thinking about the first batch after release. However, please do send
>> such cleanups really only after the release. The reason for the patch
>> slowdown is that we want to be able to focus on making a rock-solid
>> v2.36.0. This here patch series is very much a distraction at this point.
>
> OK.
>
> FWIW, I have it near the tip of 'seen', but if any fix that is more
> urgent than these changes need to be added to 'next' and down to
> 'master' makes the version queued no longer apply, I will drop it
> immediately, instead of spending cycles to figure out if it is worth
> resolving the conflict and requeuing it in 'seen' during the -rc
> period ;-)

To be fair, I actually do appreciate an early heads-up to warn
against other contributors to think twice before adding more uses of
the env-array and consider if a different approach without using the
env-array would result in better code.  It would have been better to
stress on such an early warning aspect of the series by marking the
series explicitly "not for application during -rc".

The true -rc freeze where nothing goes into 'next' (unless it is a
regression fix for this cycle) starts tomorrow at -rc1; I think I've
picked up everything relevant that have been posted in the past few
days.

Thanks.