[v2,0/2] avoid running "git-subcmd" in the dashed form

Message

Junio C Hamano Aug. 26, 2020, 7:46 p.m. UTC

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

> I think the cvsexportcommit and transport-helper changes are worth
> salvaging even if we don't remove builtin binaries, so I'll split
> them and whip them a bit more into a reasonable shape to be merged
> to 'next'.  The "break those who say 'git-cat-file'" can be left for
> future.

And here it is.

These two patches are clean-ups that are worth doing whether we
decide to remove on-disk binaries for the builtin commands.  

I droped the third one, that actually stops users from running
built-in commands using the dashed form, at least for now.  It can
be resurrected later if we really wants to propose removal to the
users, but I am not inclined to make such a proposal right now.

Junio C Hamano (2):
  transport-helper: do not run git-remote-ext etc. in dashed form
  cvsexportcommit: do not run git programs in dashed form

 git-cvsexportcommit.perl | 16 ++++++++--------
 transport-helper.c       |  4 ++--
 2 files changed, 10 insertions(+), 10 deletions(-)

Comments

Derrick Stolee Aug. 27, 2020, 12:57 a.m. UTC | #1

On 8/26/2020 3:46 PM, Junio C Hamano wrote:
> Junio C Hamano <[e-mail]> writes:
> 
>> I think the cvsexportcommit and transport-helper changes are worth
>> salvaging even if we don't remove builtin binaries, so I'll split
>> them and whip them a bit more into a reasonable shape to be merged
>> to 'next'.  The "break those who say 'git-cat-file'" can be left for
>> future.
> 
> And here it is.
> 
> These two patches are clean-ups that are worth doing whether we
> decide to remove on-disk binaries for the builtin commands.  

LGTM.

> I droped the third one, that actually stops users from running
> built-in commands using the dashed form, at least for now.  It can
> be resurrected later if we really wants to propose removal to the
> users, but I am not inclined to make such a proposal right now.

I think that's fine for now. A plan to fully deprecate the dashed
forms can come later.

Would an interesting in-between step include removing the dashed
forms for builtins that didn't exist in Git 2.0?

Thanks,
-Stolee

Junio C Hamano Aug. 27, 2020, 1:22 a.m. UTC | #2

Derrick Stolee <[e-mail]> writes:

> Would an interesting in-between step include removing the dashed
> forms for builtins that didn't exist in Git 2.0?

I am not sure if it makes much sense to treat newer and older
built-in commands differently.

Imagine that an old-timer wrote a script by somebody who trusted the
"futz with PATH and you can use git-foo" promise before Git 2.0 and
then the script was inherited by relatively new users.  Adhering to
the "when in doubt, mimic the surrounding code", which is usually a
good discipline to follow, these new users, who are now in charge of
maintaining the script, would add any new calls in "git-foo" form to
match the local convention in good faith.  And the resulting code
would have been working just fine.

Before such a "in-between step" is thrown at them, that is, at which
point it stops working if they were unlucky that they used a
relatively new built-ins.  Typically new end-users would not know
which ones are old built-ins and which ones are new, I suspect.

I do agree with Dscho that "we won't let you use builtin in dashed
form before you export an enviornment" I wrote was not a good way to
gauge the usage of on-disk builtins.  We should move the on-disk
builtins to a different directory and have them point at the
location with their $PATH as the escape hatch, as Dscho suggested,
if we were to do this for real, I'd think.

Thanks.