git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Elijah Newren <newren@gmail.com>,
	Christian Couder <christian.couder@gmail.com>,
	Git Mailing List <git@vger.kernel.org>,
	Christian Couder <chriscool@tuxfamily.org>,
	Taylor Blau <me@ttaylorr.com>
Subject: Re: [RFC PATCH 0/2] Introduce new merge-tree-ort command
Date: Thu, 13 Jan 2022 10:22:05 +0100	[thread overview]
Message-ID: <220113.86fspsvu6v.gmgdl@evledraar.gmail.com> (raw)
In-Reply-To: <xmqqy23kx2k5.fsf@gitster.g>


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

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

  reply	other threads:[~2022-01-13  9:52 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-05 16:33 [RFC PATCH 0/2] Introduce new merge-tree-ort command Christian Couder
2022-01-05 16:33 ` [RFC PATCH 1/2] merge-ort: add " Christian Couder
2022-01-05 17:08   ` Elijah Newren
2022-01-05 16:33 ` [RFC PATCH 2/2] merge-ort: add t/t4310-merge-tree-ort.sh Christian Couder
2022-01-05 17:29   ` Elijah Newren
2022-01-05 16:53 ` [RFC PATCH 0/2] Introduce new merge-tree-ort command Elijah Newren
2022-01-05 17:32   ` Elijah Newren
2022-01-07 17:58   ` Christian Couder
2022-01-07 19:06     ` Elijah Newren
2022-01-10 13:49       ` Johannes Schindelin
2022-01-10 17:56         ` Junio C Hamano
2022-01-11 13:47           ` Johannes Schindelin
2022-01-11 17:00             ` Ævar Arnfjörð Bjarmason
2022-01-11 22:25               ` Elijah Newren
2022-01-12 18:06                 ` Junio C Hamano
2022-01-12 20:06                   ` Elijah Newren
2022-01-13  6:08                     ` Junio C Hamano
2022-01-13  8:01                       ` Elijah Newren
2022-01-13  9:26                     ` Ævar Arnfjörð Bjarmason
2022-01-12 17:54               ` Junio C Hamano
2022-01-13  9:22                 ` Ævar Arnfjörð Bjarmason [this message]
2022-01-10 17:59         ` Elijah Newren
2022-01-11 21:15           ` Elijah Newren
2022-02-22 13:08             ` Johannes Schindelin
2022-01-11 22:30           ` Johannes Schindelin
2022-01-12  0:41             ` Elijah Newren
2022-02-22 12:44               ` Johannes Schindelin
2022-01-07 19:54     ` Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=220113.86fspsvu6v.gmgdl@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=chriscool@tuxfamily.org \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=me@ttaylorr.com \
    --cc=newren@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).