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

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

[Ævar Arnfjörð Bjarmason]

Remove a suggestion to write new commands in Perl or Shell to
experiment. This advice was added in 6d0618a820a (Add
Documentation/CodingGuidelines, 2007-11-08).

Since then the consensus changed to having no new such commands unless
necessary, and existing ones have been actively migrated to C.

Signed-off-by: 

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

[Torsten Bögershausen]

On Sat, Apr 17, 2021 at 10:43:54AM +0200, Ævar Arnfjörð Bjarmason wrote:
> Remove a suggestion to write new commands in Perl or Shell to
> experiment. This advice was added in 6d0618a820a (Add
> Documentation/CodingGuidelines, 2007-11-08).
>
> Since then the consensus changed to having no new such commands unless
> necessary, and existing ones have been actively migrated to C.
>
> Signed-off-by: ??var Arnfj??r?? Bjarmason <avarab@gmail.com>

The patch looks good - but the Umlauts are garbled:
ISO-2022-JP is used ?

> ---
>  Documentation/CodingGuidelines | 5 -----
>  1 file changed, 5 deletions(-)
>
> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
> index 45465bc0c98..b9cd55db6a8 100644
> --- a/Documentation/CodingGuidelines
> +++ b/Documentation/CodingGuidelines
> @@ -416,11 +416,6 @@ For C programs:
>     that are made available to it by including one of the header files
>     it must include by the previous rule.
>
> - - If you are planning a new command, consider writing it in shell
> -   or perl first, so that changes in semantics can be easily
> -   changed and discussed.  Many Git commands started out like
> -   that, and a few are still scripts.
> -
>   - Avoid introducing a new dependency into Git. This means you
>     usually should stay away from scripting languages not already
>     used in the Git core command set (unless your command is clearly
> --
> 2.31.1.687.g1d87aeed692
>

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

[Bagas Sanjaya]

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.

-- 
An old man doll... just what I always wanted! - Clara

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

[Ævar Arnfjörð Bjarmason]

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.e. even if the main logic is still in the *.perl that code would
really benefit from first doing the command-line parsing etc. in C, so
that it would behave the same as everything else in that regard, ditto
for its config parsing.

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

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1973 bytes --]

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.

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.
-- 
brian m. carlson (he/him or they/them)
Houston, Texas, US

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

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

[Ævar Arnfjörð Bjarmason]

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..

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

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 2950 bytes --]

On 2021-04-17 at 21:37:57, Ævar Arnfjörð Bjarmason wrote:
> 
> On Sat, Apr 17 2021, brian m. carlson wrote:
> > 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 patch.

> 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.

I think in many cases it is valuable to use it for prototyping still.

> > 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.

I am duly aware of this fact, having worked on a mainly Perl codebase
full-time for 6 years.

> 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.

I think it's worth keeping the shell script stuff around because I think
it adds a lot less friction to building new tooling.  I'll be frank: I'm
not massively in love with C, despite having known it since I was 10,
nor is our C particularly elegant (memory leaks, tons of global
variables, etc.).  I think there can be an argument made that in a lot
of cases, shell or Perl is the better option when performance isn't
essential, and so I think those should continue to be first-class
languages in our codebase, even if that means we need to put a little
more effort into maintaining them.

Moreover, we still have a decent number of scripts using the shell
tooling, so I'm not sure they're going away quite as quickly as you'd
hoped.  I agree they're not as numerous as they used to be, but they're
still not entirely gone, nor do I think they're going to all disappear
anytime soon.
-- 
brian m. carlson (he/him or they/them)
Houston, Texas, US

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

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

[Ævar Arnfjörð Bjarmason]

Remove a suggestion to write new commands in Perl or Shell to
experiment. This advice was added in 6d0618a820a (Add
Documentation/CodingGuidelines, 2007-11-08).

Since then the consensus changed to having no new such commands unless
necessary, and existing ones have been actively migrated to C.

So this isn't a new proposal or a suggestion to change the coding
style, but bringing this stale part of the CodingGuidelines in line
with reality.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---

V1 of this was misencoded. I sent it as UTF-8 with
charset=ISO-2022-JP.

I'd been hacking the test-lib.sh, and at one point it escaped its
quarantine and started setting config in my main git.git, and I'd
managed to miss that i18n.logOutputEncoding encoding variable when cleaning it up.

Range-diff against v1:
1:  83266f30b67 ! 1:  98b1f938f7d CodingGuidelines: remove suggestion to write commands in Perl/SH
    @@ Commit message
         Since then the consensus changed to having no new such commands unless
         necessary, and existing ones have been actively migrated to C.
     
    +    So this isn't a new proposal or a suggestion to change the coding
    +    style, but bringing this stale part of the CodingGuidelines in line
    +    with reality.
    +
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## Documentation/CodingGuidelines ##

 Documentation/CodingGuidelines | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 45465bc0c98..b9cd55db6a8 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -416,11 +416,6 @@ For C programs:
    that are made available to it by including one of the header files
    it must include by the previous rule.
 
- - If you are planning a new command, consider writing it in shell
-   or perl first, so that changes in semantics can be easily
-   changed and discussed.  Many Git commands started out like
-   that, and a few are still scripts.
-
  - Avoid introducing a new dependency into Git. This means you
    usually should stay away from scripting languages not already
    used in the Git core command set (unless your command is clearly
-- 
2.31.1.722.g788886f50a2

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

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Remove a suggestion to write new commands in Perl or Shell to
> experiment. This advice was added in 6d0618a820a (Add
> Documentation/CodingGuidelines, 2007-11-08).

I think this needs to be rephrased to clarify that it is mainly to
allow easier prototyping.

> Since then the consensus changed to having no new such commands unless
> necessary, and existing ones have been actively migrated to C.

Well, "unless necessary" is quite an interesting word here.  There
are trade-offs, and some things are not worth spending cycles to
write in C than leaving quick hackability for developers.

Also existing ones getting migrated to C does nothing to support the
implicit claim the above sentence makes, which is "writing the first
version in C is recommended".

    If you are planning a new command, it may be easier to prototype
    by implementing it as a script, until you and others on the list
    can figure out what the semantics and end-user experience should
    be, instead of starting the initial version in C code.  Many Git
    commands started out as scripts and then later rewritten in C.

More importantly, we probably should stress that we've been very bad
at maintaining plumbing to allow such quick experimentation.  We
should encourage (or even require) developers to make the feature
they directly hacked into the Porcelain command available also to
the plumbing layer, so that those who script with plumbing commands
can also use it.

> - - If you are planning a new command, consider writing it in shell
> -   or perl first, so that changes in semantics can be easily
> -   changed and discussed.  Many Git commands started out like
> -   that, and a few are still scripts.
> -
>   - Avoid introducing a new dependency into Git. This means you
>     usually should stay away from scripting languages not already
>     used in the Git core command set (unless your command is clearly