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.
next prev parent 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).