All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.