Re: [RFC PATCH 0/2] Introduce new merge-tree-ort command

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Jan 13, 2022, 09:22 UTC
Message-ID: <220113.86fspsvu6v.gmgdl@evledraar.gmail.com>
In-Reply-To: <xmqqy23kx2k5.fsf@gitster.g>

On Wed, Jan 12 2022, Junio C Hamano wrote:

Show 13 quoted lines

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> Maybe that was the right thing to do, maybe not, but it went out with
>> v2.30.0 and the lack of complaints since then would seem to suggest that
>> I was right that removing it wouldn't be a big deal.
>>
>> Of course it may have broken someone's script somewhere.
>>
>> But an important distinction is that they can get it working again by
>> just copy/pasting that ~100 line shell library into their own script, or
>> calling the underlying commands it was invoking themselves.
>
> Was parse-remote a part of what promised our end-users? [...]

It was listed under "LOW-LEVEL COMMANDS (PLUMBING)" => "Syncing repositories", which has git-daemon, git-{upload,receive}-pack, git-shell etc. now.

So, pedantically we removed a removed a plumbing utility without much if any warning.

But as I argued when it was removed I think that realistically some of our plumbing tools were made for internal-only use, and as the *.sh->C rewriting of built-ins progressed they became orphaned.

They only had public-facing manpages due to plans that never came to fruition, or just in copy/pasting an existing template at the time.

I don't think that would matter for git-parse-remote if it had subsequently gotted widespread use in the wild (even if it were undocumented), but when I investigated that it really seemed to be used by approximately nobody.

But a while after it was removed you pushed back on some further similar changes to git-sh-setup. I asked some follow-up questions in https://lore.kernel.org/git/87tuiwjfvi.fsf@evledraar.gmail.com/ about how we should consider these that you didn't reply to.

The greater context being that I was removing git-parse-remote.sh so
that I could eventually get rid of the bridge of extending
libintl/gettext to *.sh-land. The current state of that on "master" in
that regard being:
        
    $ git grep '\b(eval_)?gettext(ln)?\b' -- ':!t/' ':!ci/' ':!git-gui' ':!git-sh-i18n.sh' '*.sh'
    git-merge-octopus.sh:    gettextln "Error: Your local changes to the following files would be overwritten by merge"
    git-merge-octopus.sh:           gettextln "Automated merge did not work."
    git-merge-octopus.sh:           gettextln "Should not be doing an octopus."
    git-merge-octopus.sh:           die "$(eval_gettext "Unable to find common commit with \$pretty_name")"
    git-merge-octopus.sh:           eval_gettextln "Already up to date with \$pretty_name"
    git-merge-octopus.sh:           eval_gettextln "Fast-forwarding to: \$pretty_name"
    git-merge-octopus.sh:   eval_gettextln "Trying simple merge with \$pretty_name"
    git-merge-octopus.sh:           gettextln "Simple merge did not work, trying automatic merge."
    git-sh-setup.sh:# Source git-sh-i18n for gettext support.
    git-sh-setup.sh:                die "$(eval_gettext "usage: \$dashless \$USAGE")"
    git-sh-setup.sh:                LONG_USAGE="$(eval_gettext "usage: \$dashless \$USAGE")"
    git-sh-setup.sh:                LONG_USAGE="$(eval_gettext "usage: \$dashless \$USAGE
    git-sh-setup.sh:                gettextln "Cannot chdir to \$cdup, the toplevel of the working tree" >&2
    git-sh-setup.sh:                die "$(eval_gettext "fatal: \$program_name cannot be used without a working tree.")"
    git-sh-setup.sh:                die "$(eval_gettext "fatal: \$program_name cannot be used without a working tree.")"
    git-sh-setup.sh:                        gettextln "Cannot rewrite branches: You have unstaged changes." >&2
    git-sh-setup.sh:                        eval_gettextln "Cannot \$action: You have unstaged changes." >&2
    git-sh-setup.sh:                        eval_gettextln "Cannot \$action: Your index contains uncommitted changes." >&2
    git-sh-setup.sh:                    gettextln "Additionally, your index contains uncommitted changes." >&2
    git-sh-setup.sh:                        gettextln "You need to run this command from the toplevel of the working tree." >&2
    git-sh-setup.sh:                gettextln "Unable to determine absolute path of git directory" >&2
    git-submodule.sh:                       die "fatal: $(eval_gettext "Unable to find current revision in submodule path '\$displaypath'")"
    git-submodule.sh:                               die "fatal: $(eval_gettext "Unable to fetch in submodule path '\$sm_path'")"
    git-submodule.sh:                       die "fatal: $(eval_gettext "Unable to find current \${remote_name}/\${branch} revision in submodule path '\$sm_path'")"
    git-submodule.sh:                               die_msg="fatal: $(eval_gettext "Failed to recurse into submodule path '\$displaypath'")"

I.e. if we were able to get rid of that we could remove sh-i18n--envsubst.c and git-sh-i18n.sh itself.

Some of that is dead code, others have pending *.sh->C rewrites. For the rest we could expose a trivial git-i18n.c helper to emit the <10 messages that remained pending further rewrites, which would be much simpler than extending the generic libintl functionality to *.sh.

But since git-sh-i18n.sh latter is publicly documented as plumbing (which I'm responsible for, merely by copy/pasting an existing template) I stalled on that. Since you seemed to suggest in the linked-to thread that removing any such publicly documented shellscripting functions was a no-go, even if we'd previously removed git-parse-remote.sh.