Re: [PATCH] CodingGuidelines: remove suggestion to write commands in Perl/SH

From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: "brian m. carlson" <sandals@crustytoothpaste.net>
Cc: Bagas Sanjaya <bagasdotme@gmail.com>,
	Junio C Hamano <gitster@pobox.com>,
	Git Users <git@vger.kernel.org>
Subject: Re: [PATCH] CodingGuidelines: remove suggestion to write commands in Perl/SH
Date: Sat, 17 Apr 2021 23:37:57 +0200	[thread overview]
Message-ID: <87k0p01tje.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <YHtFBeWxE2cFlShY@camp.crustytoothpaste.net>

On Sat, Apr 17 2021, brian m. carlson wrote:

> On 2021-04-17 at 12:36:00, Ævar Arnfjörð Bjarmason wrote:
>> 
>> On Sat, Apr 17 2021, Bagas Sanjaya wrote:
>> 
>> > On 17/04/21 15.43, Ævar Arnfjörð Bjarmason wrote:
>> >> Since then the consensus changed to having no new such commands unless
>> >> necessary, and existing ones have been actively migrated to C.
>> >
>> > What I implied that when we need to implement new commands, it must
>> > be directly written in C (steeper learning curve and more tedious
>> > than implemented in shell script), so I'm against this proposal.
>> 
>> I updated the v2 of this to note that I'm not really proposing anything
>> new, but just bringing the document in line with reality. For a long
>> time now we've rejected any new non-C things being imported into the
>> tree, unless those that fall under the "such as an importer to convert
>> random-scm-X" language that's still retained in the CodingGuidelines.
>> 
>> I think that even if you or someone else wanted to write a new thing in
>> Perl or SH we'd want a new way of doing that now anyway,
>> e.g. git-send-email.perl should really be a helper for a C program
>> rather than a stand-alone thing.
>
> I'm also kind of opposed to this change.  For example, I plan on adding
> a utility to fill in SHA-1 compatibility things for SHA-256 repos, and
> that will be written in shell.  The performance benefit of C here is
> going to be minimal, especially considering the fact that people will be
> running it literally at most once per repo, so I don't see a reason to
> spend a lot of time writing C code.

"This change" as in the patch or my informal summary of what I think the
current status quo is?

The change being proposed here isn't to say that you can never write a
new thing in shell, but advice that actively encourages that for
prototyping.

It was written at a time when the top-level glob of *.sh in the project was:

	git-am.sh git-bisect.sh git-checkout.sh git-clean.sh
	git-clone.sh git-commit.sh git-fetch.sh git-filter-branch.sh
	git-instaweb.sh git-lost-found.sh git-ls-remote.sh
	git-merge-octopus.sh git-merge-one-file.sh git-merge-ours.sh
	git-merge-resolve.sh git-merge.sh git-merge-stupid.sh
	git-mergetool.sh git-parse-remote.sh git-pull.sh
	git-quiltimport.sh git-rebase--interactive.sh git-rebase.sh
	git-repack.sh git-request-pull.sh git-reset.sh git-sh-setup.sh
	git-stash.sh git-submodule.sh

I.e. the structure of the codebase was entirely different than it is
today, and it really made sense to encourage prototyping in *.sh first.

I thought it would be uncontroversial to remove advice actively
encouraging writing new built-ins in non-C first, which is not the same
as adding a policy to say that such a thing should never be done.

To have a policy like that is pretty close to my personal opinion on the
subject, but that != the proposed patch.

> I'm not of the opinion that we should never have shell or Perl code in
> our project, nor does it intrinsically make sense to migrate everything
> to C.  Typically we've done that because it performs better, especially
> on Windows, but there are many situations in which those are not major
> considerations and shell or Perl can be a desirable approach.

... but since we're sharing our own opinions :)

As someone with >100 commits in perl.git, I don't think I can be thought
to be uncomfortable with the language.

But FWIW having patched git-send-email.perl the other day, I find myself
wishing that it and similar code was in C.

Because it's got nothing to do with performance anymore, but that >=95%
of our non-trivial code is in C, so doing almost anything in either *.sh
or *.perl runs up against a wall of not having the same libraries you
take for granted, plumbing being subtly lacking or different than the
core C APIs etc.

In an ideal world we'd fix some of our plumbing, but in practice
e.g. getting something as basic as git config values is some combination
of incorrect and/or atrociously slow via Git.pm[1].

And for shellscript in particular last I looked we were *this* close to
e.g. being able to git rm sh-i18n--envsubst.c, git-sh-i18n.sh etc. I
wrote that code, but I very much wish it were entirely gone. So as one
example I've put off some general improvement in our i18n infra
(e.g. writing some minimal lib so we don't need to depend on libintl),
but have always given up on the point of dreading having to support such
a thing in shell and Perl too.

So it sucks for the individual author, but at this point the trade-off
of whipping something new up in e.g. *.sh isn't just that the thing
doesn't need to be performant, but e.g. in the case of the gettext
integration means we'll be stuck with the fixed costs of extending
certain core APIs to shell-land forever, whereas currently it's looking
like we might be able to "git rm" much/most of that stuff sooner than
later.

1. I eventually got past that and other things and have local patches to
   reduce the time to invoke git-send-email.perl's by 80%, which sped up
   t9001 from ~30s to ~10s, but it wasn't pretty..