* Re: [PATCH] merge: --ff-one-only to apply FF if commit is one
From: Taylor Blau @ 2023-10-30 20:01 UTC (permalink / raw)
To: Ruslan Yakauleu via GitGitGadget; +Cc: git, Ruslan Yakauleu
In-Reply-To: <pull.1599.git.git.1698224280816.gitgitgadget@gmail.com>
Hi Ruslan,
On Wed, Oct 25, 2023 at 08:58:00AM +0000, Ruslan Yakauleu via GitGitGadget wrote:
> From: Ruslan Yakauleu <ruslan.yakauleu@gmail.com>
>
> A new option --ff-one-only to control the merging strategy.
> For one commit option works like -ff to avoid extra merge commit.
> In other cases the option works like --no-ff to create merge commit for
> complex features.
This seems like a pretty niche feature to want to introduce a new option
for. I would imagine the alternative is something like:
ff="--no-ff"
if test 1 -eq $(git rev-list @{u}..)
then
ff="--ff"
fi
[on upstream @{u}]
git merge "$ff" "$branch"
I don't have a great sense of how many users might want or benefit from
something like this. My sense is that there aren't many, but I could
very easily be wrong here.
In any case, my sense is that this is probably too niche to introduce a
new command-line option just to implement this behavior when the above
implementation is pretty straightforward. Regardless, here's my review
of the patch...
> @@ -631,6 +633,8 @@ static int git_merge_config(const char *k, const char *v,
> fast_forward = boolval ? FF_ALLOW : FF_NO;
> } else if (v && !strcmp(v, "only")) {
> fast_forward = FF_ONLY;
> + } else if (v && !strcmp(v, "one-only")) {
> + fast_forward = FF_ONE_ONLY;
The configuration handling and documentation all look good.
> } /* do not barf on values from future versions of git */
> return 0;
> } else if (!strcmp(k, "merge.defaulttoupstream")) {
> @@ -1527,6 +1531,18 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
> free(list);
> }
>
> + if (fast_forward == FF_ONE_ONLY) {
> + fast_forward = FF_NO;
> +
> + /* check that we have one and only one commit to merge */
> + if (squash || ((!remoteheads->next &&
> + !common->next &&
> + oideq(&common->item->object.oid, &head_commit->object.oid)) &&
> + oideq(&remoteheads->item->parents->item->object.oid, &head_commit->object.oid))) {
> + fast_forward = FF_ALLOW;
> + }
And this rather long conditional looks right, too. This patch could
definitely benefit from some tests, though...
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH v2 00/12] show-ref: introduce mode to check for ref existence
From: Taylor Blau @ 2023-10-30 19:32 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Junio C Hamano, Eric Sunshine, Han-Wen Nienhuys
In-Reply-To: <cover.1698314128.git.ps@pks.im>
On Thu, Oct 26, 2023 at 11:56:16AM +0200, Patrick Steinhardt wrote:
> Hi,
>
> this is the second version of my patch series that introduces a new `git
> show-ref --exists` mode to check for reference existence.
All looks quite reasonable to me. I'm happy with this series as-is,
though I left a few minor comments and suggestions throughout.
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH v2 09/12] builtin/show-ref: ensure mutual exclusiveness of subcommands
From: Taylor Blau @ 2023-10-30 19:31 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Junio C Hamano, Eric Sunshine, Han-Wen Nienhuys
In-Reply-To: <5ba566723e8742e6df150b12f1d044089ff62b59.1698314128.git.ps@pks.im>
On Thu, Oct 26, 2023 at 11:56:57AM +0200, Patrick Steinhardt wrote:
> The git-show-ref(1) command has three different modes, of which one is
> implicit and the other two can be chosen explicitly by passing a flag.
> But while these modes are standalone and cause us to execute completely
> separate code paths, we gladly accept the case where a user asks for
> both `--exclude-existing` and `--verify` at the same time even though it
> is not obvious what will happen. Spoiler: we ignore `--verify` and
> execute the `--exclude-existing` mode.
>
> Let's explicitly detect this invalid usage and die in case both modes
> were requested.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> builtin/show-ref.c | 4 ++++
> t/t1403-show-ref.sh | 5 +++++
> 2 files changed, 9 insertions(+)
>
> diff --git a/builtin/show-ref.c b/builtin/show-ref.c
> index 87bc45d2d13..1768aef77b3 100644
> --- a/builtin/show-ref.c
> +++ b/builtin/show-ref.c
> @@ -271,6 +271,10 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)
> argc = parse_options(argc, argv, prefix, show_ref_options,
> show_ref_usage, 0);
>
> + if ((!!exclude_existing_opts.enabled + !!verify) > 1)
> + die(_("only one of '%s' or '%s' can be given"),
> + "--exclude-existing", "--verify");
> +
This is technically correct, but I was surprised to see it written this
way instead of
if (exclude_existing_opts.enabled && verify)
die(...);
I don't think it's a big deal either way, I was just curious why you
chose one over the other.
> +test_expect_success 'show-ref sub-modes are mutually exclusive' '
> + test_must_fail git show-ref --verify --exclude-existing 2>err &&
> + grep "only one of ${SQ}--exclude-existing${SQ} or ${SQ}--verify${SQ} can be given" err
> +'
grepping is fine here, but since you have the exact error message, it
may be worth switching to test_cmp.
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH v2 06/12] builtin/show-ref: stop using global variable to count matches
From: Taylor Blau @ 2023-10-30 19:14 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Junio C Hamano, Eric Sunshine, Han-Wen Nienhuys
In-Reply-To: <d52a5e8ced2adc5c9315edea9fc497d1ffa30125.1698314128.git.ps@pks.im>
On Thu, Oct 26, 2023 at 11:56:42AM +0200, Patrick Steinhardt wrote:
> When passing patterns to git-show-ref(1) we're checking whether any
> reference matches -- if none does, we indicate this condition via an
> unsuccessful exit code.
s/does/do, but not a big enough deal to reroll IMHO.
> We're using a global variable to count these matches, which is required
> because the counter is getting incremented in a callback function. But
> now that we have the `struct show_ref_data` in place, we can get rid of
> the global variable and put the counter in there instead.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> builtin/show-ref.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
Looks all good to me!
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH v2 05/12] builtin/show-ref: refactor `--exclude-existing` options
From: Taylor Blau @ 2023-10-30 18:55 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Junio C Hamano, Eric Sunshine, Han-Wen Nienhuys
In-Reply-To: <bed2a8a07696371e07c0b2d1282ed51c0b1b9fee.1698314128.git.ps@pks.im>
On Thu, Oct 26, 2023 at 11:56:37AM +0200, Patrick Steinhardt wrote:
> @@ -95,6 +94,11 @@ static int add_existing(const char *refname,
> return 0;
> }
>
> +struct exclude_existing_options {
> + int enabled;
> + const char *pattern;
> +};
> +
Thinking on my earlier suggestion more, I wondered if using the
OPT_SUBCOMMAND() function might make things easier to organize and
eliminate the need for things like .enabled or having to define structs
for each of the sub-commands.
But I don't think that this is (easily) possible to do, since
`--exclude-existing` is behind a command-line option, not a separate
mode (e.g. "commit-graph verify", not "commit-graph --verify"). So I
think you *could* make it work with some combination of OPT_SUBCOMMAND
and callbacks to set the function pointer yourself when given the
`--exclude-existing` option. But I think that's sufficiently gross as to
not be worth it.
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH v2 05/12] builtin/show-ref: refactor `--exclude-existing` options
From: Taylor Blau @ 2023-10-30 18:37 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Junio C Hamano, Eric Sunshine, Han-Wen Nienhuys
In-Reply-To: <bed2a8a07696371e07c0b2d1282ed51c0b1b9fee.1698314128.git.ps@pks.im>
On Thu, Oct 26, 2023 at 11:56:37AM +0200, Patrick Steinhardt wrote:
> It's not immediately obvious options which options are applicable to
> what subcommand in git-show-ref(1) because all options exist as global
> state. This can easily cause confusion for the reader.
>
> Refactor options for the `--exclude-existing` subcommand to be contained
> in a separate structure. This structure is stored on the stack and
> passed down as required. Consequently, it clearly delimits the scope of
> those options and requires the reader to worry less about global state.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
All makes sense, but...
> @@ -19,8 +19,7 @@ static const char * const show_ref_usage[] = {
> };
>
> static int deref_tags, show_head, tags_only, heads_only, found_match, verify,
> - quiet, hash_only, abbrev, exclude_arg;
> -static const char *exclude_existing_arg;
> + quiet, hash_only, abbrev;
>
> static void show_one(const char *refname, const struct object_id *oid)
> {
> @@ -95,6 +94,11 @@ static int add_existing(const char *refname,
> return 0;
> }
>
> +struct exclude_existing_options {
> + int enabled;
...do we need an .enabled here? I think checking whether or not .pattern
is NULL is sufficient, but perhaps there is another use of .enabled
later on in the series...
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH v2 04/12] builtin/show-ref: fix dead code when passing patterns
From: Taylor Blau @ 2023-10-30 18:24 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Junio C Hamano, Eric Sunshine, Han-Wen Nienhuys
In-Reply-To: <87afcee830caab2782ce693c1f961df6fea6e7b5.1698314128.git.ps@pks.im>
On Thu, Oct 26, 2023 at 11:56:33AM +0200, Patrick Steinhardt wrote:
> When passing patterns to `git show-ref` we have some code that will
> cause us to die if `verify && !quiet` is true. But because `verify`
> indicates a different subcommand of git-show-ref(1) that causes us to
> execute `cmd_show_ref__verify()` and not `cmd_show_ref__patterns()`, the
> condition cannot ever be true.
>
> Let's remove this dead code.
Makes sense. Let's read on...
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH v3 5/8] ci: unify setup of some environment variables
From: Dragan Simic @ 2023-10-30 18:22 UTC (permalink / raw)
To: phillip.wood; +Cc: Patrick Steinhardt, git, Junio C Hamano, Oswald Buddenhagen
In-Reply-To: <87430c6c-91c0-4be1-b89d-bf442b3f018b@gmail.com>
On 2023-10-30 16:09, Phillip Wood wrote:
> On 30/10/2023 12:15, Patrick Steinhardt wrote:
>> Both GitHub Actions and Azue Pipelines set up the environment
>> variables
>> GIT_TEST_OPTS, GIT_PROVE_OPTS and MAKEFLAGS. And while most values are
>> actually the same, the setup is completely duplicate. With the
>> upcoming
>> support for GitLab CI this duplication would only extend even further.
>>
>> Unify the setup of those environment variables so that only the
>> uncommon
>> parts are separated. While at it, we also perform some additional
>> small
>> improvements:
>>
>> - We use nproc instead of a hardcoded count of jobs for make and
>> prove. This ensures that the number of concurrent processes
>> adapts
>> to the host automatically.
>
> Sadly this makes the Windows and MacOS jobs fail on GitHub Actions as
> nproc is not installed[1]. Perhaps we could do
>
> --jobs="$(nproc || echo 2)"
It would be better to use the following, to also suppress any error
messages:
--jobs=$(nproc 2> /dev/null || echo 2)
Having the quotation marks is also pretty much redundant.
> instead. (Maybe 2 is a bit low but the current value of 10 seems
> pretty high for the number of cores on the runners that we use)
^ permalink raw reply
* Re: [PATCH v2 03/12] builtin/show-ref: fix leaking string buffer
From: Taylor Blau @ 2023-10-30 18:10 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Junio C Hamano, Eric Sunshine, Han-Wen Nienhuys
In-Reply-To: <bb0d656a0b40b79c90dc2505239976a93f18f432.1698314128.git.ps@pks.im>
On Thu, Oct 26, 2023 at 11:56:29AM +0200, Patrick Steinhardt wrote:
> Fix a leaking string buffer in `git show-ref --exclude-existing`. While
> the buffer is technically not leaking because its variable is declared
> as static, there is no inherent reason why it should be.
Well spotted and fixed. I ran the test suite in GIT_TEST_PASSING_SANITIZE_LEAK's
"check" mode and didn't find anything that was made leak-free by this
patch not already marked as such. So this (and the rest of the series up
to this point) LGTM.
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH] tests: handle "funny" exit code 127 produced by MSVC-compiled exes
From: Jeff King @ 2023-10-30 17:56 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget; +Cc: git, Johannes Schindelin
In-Reply-To: <pull.1604.git.1698680732691.gitgitgadget@gmail.com>
On Mon, Oct 30, 2023 at 03:45:32PM +0000, Johannes Schindelin via GitGitGadget wrote:
> Now, `jk/tree-name-and-depth-limit` introduces a pair of test cases that
> expect a command that produces a stack overflow to fail, which it
> typically does with exit code 139 (which means SIGSEGV).
I think you're misinterpreting the purpose of the tests from that
series; they're not intended to segfault. Quoting from t6700:
# We'll test against two depths here: a small one that will let us check the
# behavior of the config setting easily, and a large one that should be
# forbidden by default. Testing the default depth will let us know whether our
# default is enough to prevent segfaults on systems that run the tests.
So for the "big tree" tests in that file, we are looking for a
controlled failure rather than a segfault. And indeed, the end of that
series already lowered the default to accommodate the msys windows
build; see the discussion in 4d5693ba05 (lower core.maxTreeDepth default
to 2048, 2023-08-31).
So I think the test is working as designed here: it is showing us that
the default value is not sufficient to protect MSVC builds from running
out of stack space. There are a few options there:
1. We can lower the default everywhere.
2. We can lower it just for MSVC builds.
3. We can accept the situation and skip the tests for that build.
There's a bit more discussion in the commit I referenced above.
> Let's work around this by:
>
> 1) recording which C compiler was used, and
>
> 2) adding an MSVC-only exception to `test_must_fail` to treat 127 as a
> regular failure.
>
> There is a slight downside of this approach in that a real missing
> command could be mistaken for a failure. However, this would be caught
> on other platforms, and besides, we use `test_must_fail` only for `git`
> and `scalar` anymore, and we can be pretty certain that both are there.
I think there is another much worse downside to your patch: we will stop
noticing when MSVC builds segfault in the tests. The purpose of
test_must_fail is to allow controlled and expected failure returns from
the command, but still report on unexpected situations (signal death,
command not found, and so on).
-Peff
^ permalink raw reply
* Re: Repository cloned using SSH does not respect bare repository initial branch
From: Jeff King @ 2023-10-30 17:43 UTC (permalink / raw)
To: Sheik; +Cc: git
In-Reply-To: <b310e254-f6d3-4715-b042-341bf5a98bbc@gmail.com>
On Tue, Oct 31, 2023 at 02:24:46AM +1100, Sheik wrote:
> Server version is same as client (v2.42.0) as I ran these commands all on
> the same machine.
OK. The next thing I'd check is running both commands with:
GIT_TRACE_PACKET=1 git clone ...
to see the protocol trace, and how it differs between the two. What I
suspect you may see is that the local clone is using the "v2" protocol
(a capabilities report, followed by "ls-refs", which mentions the symref
value of HEAD), and the ssh one uses the older "v0" (it goes straight to
the ref advertisement).
Quoting from 59e1205d16 (ls-refs: report unborn targets of symrefs,
2021-02-05), the commit I mentioned before:
This change is only for protocol v2. A similar change for protocol
v0 would require independent protocol design (there being no
analogous position to signal support for "unborn") and client-side
plumbing of the data required, so the scope of this patch set is
limited to protocol v2.
So in v0 the server doesn't pass back sufficient information for the
client to know about the name of the unborn HEAD branch.
If that's the culprit, the next question of course is why we'd do v2
locally versus v0 overssh. And that probably has to do with how we
trigger the protocol upgrade. To see if the server supports v2, the
client passes extra information "out of band". For git-over-http, this
happens in an extra HTTP header. For local repositories, it happens in
an environment variable ($GIT_PROTOCOL). For git-over-ssh it happens in
that sameenvironment variable, which we instruct the ssh client to pass
using "-o SendEnv". But:
1. If your ssh client doesn't look like openssh, we don't know if it
supports "-o" and may skip it. See the discussion in ssh.variant in
"git help config".
2. Some servers need to be configured to allow the client to set
environment variables. In the case of openssh, you'd want a line
like this in your sshd_config file:
AcceptEnv GIT_PROTOCOL
Of the two, I'd guess that the second one is more likely to be your
problem (since you're running Linux, where openssh is the norm).
-Peff
^ permalink raw reply
* Re: [PATCH] sequencer: remove use of comment character
From: Elijah Newren @ 2023-10-30 17:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Tony Tung via GitGitGadget, git, Tony Tung
In-Reply-To: <xmqq7cn4g3nx.fsf@gitster.g>
On Sun, Oct 29, 2023 at 9:01 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Tony Tung via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Tony Tung <tonytung@merly.org>
> >
> > Instead of using the hardcoded `# `, use the
> > user-defined comment_line_char. Adds a test
> > to prevent regressions.
>
> Good spotting.
>
> Two observations.
>
> (1) There are a few more places that need similar treatment in the
> same file; you may want to fix them all while at it.
>
> (2) The second argument to strbuf_commented_addf() is always the
> comment_line_char global variable, not just inside this file
> but all callers across the codebase. We probably should drop
> it and have the strbuf_commented_addf() helper itself refer to
> the global. That way, if we ever want to change the global
> variable reference to something else (e.g. function call), we
> only have to touch a single place.
>
> The latter is meant as #leftoverbits and will be a lot wider
> clean-up that we may want to do long after this patch hits out
> codebase. The "other places" I spotted for the former are the
> following, but needs to be taken with a huge grain of salt, as it
> has not even been compile tested.
>
> Thanks.
>
> sequencer.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git c/sequencer.c w/sequencer.c
> index d584cac8ed..33208b1660 100644
> --- c/sequencer.c
> +++ w/sequencer.c
> @@ -1893,8 +1893,8 @@ static void update_squash_message_for_fixup(struct strbuf *msg)
> size_t orig_msg_len;
> int i = 1;
>
> - strbuf_addf(&buf1, "# %s\n", _(first_commit_msg_str));
> - strbuf_addf(&buf2, "# %s\n", _(skip_first_commit_msg_str));
> + strbuf_addf(&buf1, comment_line_char, "%s\n", _(first_commit_msg_str));
> + strbuf_addf(&buf2, comment_line_char, "%s\n", _(skip_first_commit_msg_str));
> s = start = orig_msg = strbuf_detach(msg, &orig_msg_len);
> while (s) {
> const char *next;
> @@ -2269,8 +2269,8 @@ static int do_pick_commit(struct repository *r,
> next = parent;
> next_label = msg.parent_label;
> if (opts->commit_use_reference) {
> - strbuf_addstr(&msgbuf,
> - "# *** SAY WHY WE ARE REVERTING ON THE TITLE LINE ***");
> + strbuf_commented_addf(&msgbuf, comment_line_char, "%s",
> + "*** SAY WHY WE ARE REVERTING ON THE TITLE LINE ***");
> } else if (skip_prefix(msg.subject, "Revert \"", &orig_subject) &&
> /*
> * We don't touch pre-existing repeated reverts, because
>
I thought the point of the comment_line_char was so that commit
messages could have lines starting with '#'. That rationale doesn't
apply to the TODO list generation or parsing, and I'm not sure if we
want to add the same complexity there. If we do want to add the same
complexity there, I'm worried that making these changes are
insufficent; there are some other hardcoded '#' references in the code
(as a quick greps for '".*#' and "'#'" will turn up). Since those
other references include parsing as well as generation, I think we
might actually be introducing bugs in the TODO list handling if we
only partially convert it, but someone would need to double check.
^ permalink raw reply
* Re: [PATCH] reflog: fix expire --single-worktree
From: Taylor Blau @ 2023-10-30 17:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: René Scharfe, Git List, John Cai
In-Reply-To: <xmqqa5s1hxhh.fsf@gitster.g>
On Mon, Oct 30, 2023 at 07:31:22AM +0900, Junio C Hamano wrote:
> René Scharfe <l.s.r@web.de> writes:
>
> > ... and added a non-printable short flag for it, presumably by
> > accident.
>
> Very well spotted.
>
> FWIW, with the following patch on top of this patch, all tests pass
> (and without your fix, of course this notices the "\001" and breaks
> numerous tests that use "git reflog"). So you seem to have found
> the only one broken instance (among those that are tested, anyway).
This makes sense to me, but obviously won't catch non-tested cases. I
thought that a new Cocinelle rule might be appropriate here, but it is
frustratingly difficult to specify a constraint like:
OPT_BOOL(e1, e2, e3, ...)
with
!(e1 == 0 || (33 <= e1 && e1 <= 127))
I'll think on it a little bit, but this seems low priority enough that I
don't feel compelled to urgently deal with adding a new Coccinelle rule.
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH v5 00/14] Introduce new `git replay` command
From: Elijah Newren @ 2023-10-30 17:18 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Christian Couder, git, Junio C Hamano, Patrick Steinhardt,
John Cai, Derrick Stolee, Phillip Wood, Calvin Wan, Toon Claes,
Dragan Simic, Linus Arver
In-Reply-To: <e04cfbdc-fd28-c645-8f5d-132f7ceec6be@gmx.de>
Hi Johannes,
On Sun, Oct 29, 2023 at 7:14 AM Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
>
> Hi Elijah,
>
> On Sat, 28 Oct 2023, Elijah Newren wrote:
>
> > On Tue, Oct 10, 2023 at 5:39 AM Christian Couder
> > <christian.couder@gmail.com> wrote:
> > > * Patch 12/15 (replay: disallow revision specific options and
> > > pathspecs) in version 4 has been removed, so there are now only 14
> > > patches instead of 15 in the series. This follows a suggestion by
> > > Dscho, and goes in the direction Elijah initially wanted before
> > > Derrick Stolee argued for disallowing revision specific options and
> > > pathspecs.
> >
> > [... snipping many parts that I agree with...]
> >
> > > Also instead of forcing reverse order we use the reverse order by
> > > default but allow it to be changed using `--reverse`. Thanks to
> > > Dscho.
> >
> > I can see why this might sometimes be useful for exclusively linear
> > history, but it seems to open a can of worms and possibly unfixable
> > corner cases for non-linear history. I'd rather not do this, or at
> > least pull it out of this series and let us discuss it in some follow
> > up series. There are some other alternatives that might handle such
> > usecases better.
>
> I find myself wishing for an easy way to reverse commits, if only to
> switch around the latest two commits while stopped during a rebase.
>
> So it would have been nice for me if there had been an easy, worktree-less
> way to make that happen.
Seems reasonable; we'll definitely want to keep this in mind.
> I guess this would be going in the direction of reordering commits,
> though, something we deliberately left for later?
Yes, I think that's a good framing for it.
Thanks,
Elijah
^ permalink raw reply
* Re: [PATCH v2 0/1] Object ID support for git merge-file
From: Elijah Newren @ 2023-10-30 17:15 UTC (permalink / raw)
To: brian m. carlson
Cc: git, Junio C Hamano, Phillip Wood, Eric Sunshine, Taylor Blau
In-Reply-To: <20231030162658.567523-1-sandals@crustytoothpaste.net>
On Mon, Oct 30, 2023 at 9:27 AM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
>
> This series introduces an --object-id option to git merge-file such
> that, instead of reading and writing from files on the system, it reads
> from and writes to the object store using blobs.
>
> Changes from v1:
> * Improve error handling
> * Re-add `-p` argument for documentation
>
> brian m. carlson (1):
> merge-file: add an option to process object IDs
>
> Documentation/git-merge-file.txt | 20 +++++++++++
> builtin/merge-file.c | 62 +++++++++++++++++++++++---------
> t/t6403-merge-file.sh | 58 ++++++++++++++++++++++++++++++
> 3 files changed, 124 insertions(+), 16 deletions(-)
Thanks, this version looks good to me.
^ permalink raw reply
* Re: [PATCH 0/1] Object ID support for git merge-file
From: Elijah Newren @ 2023-10-30 17:14 UTC (permalink / raw)
To: brian m. carlson, Taylor Blau, Elijah Newren, git, Junio C Hamano,
Phillip Wood
In-Reply-To: <ZT_YuF4g-8P9fc4t@tapette.crustytoothpaste.net>
On Mon, Oct 30, 2023 at 9:24 AM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
>
> On 2023-10-30 at 15:54:14, Taylor Blau wrote:
> > On Sat, Oct 28, 2023 at 11:24:06PM -0700, Elijah Newren wrote:
> > > But...wouldn't you already have the conflicts generated when doing the
> > > merge and learning that it fails? Why would you need to generate them
> > > again?
> >
> > brian would know better than I do, but I believe the reason is because
> > the "attempt this merge" RPC is handled separately from the "show me the
> > merge conflict(s) at xyz path". Those probably could be combined
> > (obviating the need for this patch), but doing so is probably rather
> > complicated.
>
> That's correct. They could in theory happen at different times, which
> is why they're not linked.
Maybe this is digging a little into "historical reasons" too much, but
this still seems a little funny. If they happen at different times,
you still need multiple pieces of information remembered from the
merge operation in order for git-merge-file to be able to regenerate
the conflict correctly in general. In particular, you need the OIDs
and the filenames. Trying to regenerate a conflict without
remembering those from the merge step would only work for common
cases, but would be problematic in the face of either renames being
involved or recursive merges or both. And if you need to remember
information from the merge step, then why not remember the actual
conflicts (or at least the tree OID generated by the merge operation,
which has the conflicts embedded within it)?
I know, I know, there's probably just historical cruft that needs
cleaning up, and I don't think any of this matters to the patch at
hand since it's independently useful. It just sounds like a system
has been set up that has some rough edge cases caused by a poor
splitting.
> --
> brian m. carlson (he/him or they/them)
> Toronto, Ontario, CA
^ permalink raw reply
* Re: [PATCH] chore: fix typo in .clang-format comment
From: Taylor Blau @ 2023-10-30 16:56 UTC (permalink / raw)
To: Aditya Neelamraju via GitGitGadget; +Cc: git, Aditya Neelamraju
In-Reply-To: <pull.1602.git.git.1698610987926.gitgitgadget@gmail.com>
On Sun, Oct 29, 2023 at 08:23:07PM +0000, Aditya Neelamraju via GitGitGadget wrote:
> From: Aditya Neelamraju <adityanv97@gmail.com>
We typically prefix commit messages with the subject area they're
working in, not with "chore", or "feat" like some Git workflows
recommend.
That said, the contents of this patch look obviously correct to me.
Thanks for noticing and fixing!
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH 1/1] merge-file: add an option to process object IDs
From: Elijah Newren @ 2023-10-30 16:39 UTC (permalink / raw)
To: brian m. carlson, Elijah Newren, git, Junio C Hamano,
Phillip Wood
In-Reply-To: <ZT5p0WUw7EeaY8vW@tapette.crustytoothpaste.net>
Hi,
On Sun, Oct 29, 2023 at 7:18 AM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
>
> On 2023-10-29 at 06:17:09, Elijah Newren wrote:
> > Hi,
> >
> > Overall, looks good. Just a couple questions...
> >
> > On Tue, Oct 24, 2023 at 12:58 PM brian m. carlson
> > <sandals@crustytoothpaste.net> wrote:
> > >
> > > From: "brian m. carlson" <bk2204@github.com>
> > >
> > [...]
> > > --- a/Documentation/git-merge-file.txt
> > > +++ b/Documentation/git-merge-file.txt
> > > @@ -12,6 +12,9 @@ SYNOPSIS
> > > 'git merge-file' [-L <current-name> [-L <base-name> [-L <other-name>]]]
> > > [--ours|--theirs|--union] [-p|--stdout] [-q|--quiet] [--marker-size=<n>]
> > > [--[no-]diff3] <current-file> <base-file> <other-file>
> > > +'git merge-file' --object-id [-L <current-name> [-L <base-name> [-L <other-name>]]]
> > > + [--ours|--theirs|--union] [-q|--quiet] [--marker-size=<n>]
> > > + [--[no-]diff3] <current-oid> <base-oid> <other-oid>
> >
> > Why was the `[-p|--stdout]` option removed in the second synopsis?
> > Elsewhere you explicitly call it out as a possibility to be used with
> > --object-id.
>
> Originally because it implied `-p`, but I changed that to write into the
> object store. I'll restore it.
>
> > Also, why the extra synopsis instead of just adding a `[--object-id]`
> > option to the previous one?
>
> Because there's a relevant difference: the former has <current-file>,
> <base-file>, and <other-file>, and the latter has the -oid versions.
Ah, I looked over it a couple times and just kept missing that
difference. Thanks for pointing it out.
>
> > Does "/dev/null" have any portability considerations? (I really don't
> > know; just curious.)
>
> We already use it elsewhere in the codebase, so I assume it works. We
> also have a test for that case and it worked in CI, so it's probably
> fine.
Seems reasonable to me; thanks.
^ permalink raw reply
* [PATCH v2 1/1] merge-file: add an option to process object IDs
From: brian m. carlson @ 2023-10-30 16:26 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Elijah Newren, Phillip Wood, Eric Sunshine,
Taylor Blau
In-Reply-To: <20231030162658.567523-1-sandals@crustytoothpaste.net>
From: "brian m. carlson" <bk2204@github.com>
git merge-file knows how to merge files on the file system already. It
would be helpful, however, to allow it to also merge single blobs.
Teach it an `--object-id` option which means that its arguments are
object IDs and not files to allow it to do so.
Since we obviously won't be writing the data to the first argument,
imply the -p option so we write to standard output.
We handle the empty blob specially since read_mmblob doesn't read it
directly and otherwise users cannot specify an empty ancestor.
Signed-off-by: brian m. carlson <bk2204@github.com>
---
Documentation/git-merge-file.txt | 20 +++++++++++
builtin/merge-file.c | 62 +++++++++++++++++++++++---------
t/t6403-merge-file.sh | 58 ++++++++++++++++++++++++++++++
3 files changed, 124 insertions(+), 16 deletions(-)
diff --git a/Documentation/git-merge-file.txt b/Documentation/git-merge-file.txt
index 7e9093fab6..a4de2bd297 100644
--- a/Documentation/git-merge-file.txt
+++ b/Documentation/git-merge-file.txt
@@ -12,6 +12,9 @@ SYNOPSIS
'git merge-file' [-L <current-name> [-L <base-name> [-L <other-name>]]]
[--ours|--theirs|--union] [-p|--stdout] [-q|--quiet] [--marker-size=<n>]
[--[no-]diff3] <current-file> <base-file> <other-file>
+'git merge-file' --object-id [-L <current-name> [-L <base-name> [-L <other-name>]]]
+ [--ours|--theirs|--union] [-p|--stdout] [-q|--quiet] [--marker-size=<n>]
+ [--[no-]diff3] <current-oid> <base-oid> <other-oid>
DESCRIPTION
@@ -40,6 +43,10 @@ however, these conflicts are resolved favouring lines from `<current-file>`,
lines from `<other-file>`, or lines from both respectively. The length of the
conflict markers can be given with the `--marker-size` option.
+If `--object-id` is specified, exactly the same behavior occurs, except that
+instead of specifying what to merge as files, it is specified as a list of
+object IDs referring to blobs.
+
The exit value of this program is negative on error, and the number of
conflicts otherwise (truncated to 127 if there are more than that many
conflicts). If the merge was clean, the exit value is 0.
@@ -52,6 +59,14 @@ linkgit:git[1].
OPTIONS
-------
+--object-id::
+ Specify the contents to merge as blobs in the current repository instead of
+ files. In this case, the operation must take place within a valid repository.
++
+If the `-p` option is specified, the merged file (including conflicts, if any)
+goes to standard output as normal; otherwise, the merged file is written to the
+object store and the object ID of its blob is written to standard output.
+
-L <label>::
This option may be given up to three times, and
specifies labels to be used in place of the
@@ -93,6 +108,11 @@ EXAMPLES
merges tmp/a123 and tmp/c345 with the base tmp/b234, but uses labels
`a` and `c` instead of `tmp/a123` and `tmp/c345`.
+`git merge-file -p --object-id abc1234 def567 890abcd`::
+
+ combines the changes of the blob abc1234 and 890abcd since def567,
+ tries to merge them and writes the result to standard output
+
GIT
---
Part of the linkgit:git[1] suite
diff --git a/builtin/merge-file.c b/builtin/merge-file.c
index d7eb4c6540..832c93d8d5 100644
--- a/builtin/merge-file.c
+++ b/builtin/merge-file.c
@@ -1,5 +1,8 @@
#include "builtin.h"
#include "abspath.h"
+#include "hex.h"
+#include "object-name.h"
+#include "object-store.h"
#include "config.h"
#include "gettext.h"
#include "setup.h"
@@ -31,10 +34,11 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
mmfile_t mmfs[3] = { 0 };
mmbuffer_t result = { 0 };
xmparam_t xmp = { 0 };
- int ret = 0, i = 0, to_stdout = 0;
+ int ret = 0, i = 0, to_stdout = 0, object_id = 0;
int quiet = 0;
struct option options[] = {
OPT_BOOL('p', "stdout", &to_stdout, N_("send results to standard output")),
+ OPT_BOOL(0, "object-id", &object_id, N_("use object IDs instead of filenames")),
OPT_SET_INT(0, "diff3", &xmp.style, N_("use a diff3 based merge"), XDL_MERGE_DIFF3),
OPT_SET_INT(0, "zdiff3", &xmp.style, N_("use a zealous diff3 based merge"),
XDL_MERGE_ZEALOUS_DIFF3),
@@ -71,8 +75,12 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
return error_errno("failed to redirect stderr to /dev/null");
}
+ if (object_id)
+ setup_git_directory();
+
for (i = 0; i < 3; i++) {
char *fname;
+ struct object_id oid;
mmfile_t *mmf = mmfs + i;
if (!names[i])
@@ -80,12 +88,22 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
fname = prefix_filename(prefix, argv[i]);
- if (read_mmfile(mmf, fname))
+ if (object_id) {
+ if (repo_get_oid(the_repository, argv[i], &oid))
+ ret = error(_("object '%s' does not exist"),
+ argv[i]);
+ else if (!oideq(&oid, the_hash_algo->empty_blob))
+ read_mmblob(mmf, &oid);
+ else
+ read_mmfile(mmf, "/dev/null");
+ } else if (read_mmfile(mmf, fname)) {
ret = -1;
- else if (mmf->size > MAX_XDIFF_SIZE ||
- buffer_is_binary(mmf->ptr, mmf->size))
+ }
+ if (ret != -1 && (mmf->size > MAX_XDIFF_SIZE ||
+ buffer_is_binary(mmf->ptr, mmf->size))) {
ret = error("Cannot merge binary files: %s",
argv[i]);
+ }
free(fname);
if (ret)
@@ -99,20 +117,32 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
ret = xdl_merge(mmfs + 1, mmfs + 0, mmfs + 2, &xmp, &result);
if (ret >= 0) {
- const char *filename = argv[0];
- char *fpath = prefix_filename(prefix, argv[0]);
- FILE *f = to_stdout ? stdout : fopen(fpath, "wb");
+ if (object_id && !to_stdout) {
+ struct object_id oid;
+ if (result.size) {
+ if (write_object_file(result.ptr, result.size, OBJ_BLOB, &oid) < 0)
+ ret = error(_("Could not write object file"));
+ } else {
+ oidcpy(&oid, the_hash_algo->empty_blob);
+ }
+ if (ret >= 0)
+ printf("%s\n", oid_to_hex(&oid));
+ } else {
+ const char *filename = argv[0];
+ char *fpath = prefix_filename(prefix, argv[0]);
+ FILE *f = to_stdout ? stdout : fopen(fpath, "wb");
- if (!f)
- ret = error_errno("Could not open %s for writing",
- filename);
- else if (result.size &&
- fwrite(result.ptr, result.size, 1, f) != 1)
- ret = error_errno("Could not write to %s", filename);
- else if (fclose(f))
- ret = error_errno("Could not close %s", filename);
+ if (!f)
+ ret = error_errno("Could not open %s for writing",
+ filename);
+ else if (result.size &&
+ fwrite(result.ptr, result.size, 1, f) != 1)
+ ret = error_errno("Could not write to %s", filename);
+ else if (fclose(f))
+ ret = error_errno("Could not close %s", filename);
+ free(fpath);
+ }
free(result.ptr);
- free(fpath);
}
if (ret > 127)
diff --git a/t/t6403-merge-file.sh b/t/t6403-merge-file.sh
index 1a7082323d..2c92209eca 100755
--- a/t/t6403-merge-file.sh
+++ b/t/t6403-merge-file.sh
@@ -65,11 +65,30 @@ test_expect_success 'merge with no changes' '
test_cmp test.txt orig.txt
'
+test_expect_success 'merge with no changes with --object-id' '
+ git add orig.txt &&
+ git merge-file -p --object-id :orig.txt :orig.txt :orig.txt >actual &&
+ test_cmp actual orig.txt
+'
+
test_expect_success "merge without conflict" '
cp new1.txt test.txt &&
git merge-file test.txt orig.txt new2.txt
'
+test_expect_success 'merge without conflict with --object-id' '
+ git add orig.txt new2.txt &&
+ git merge-file --object-id :orig.txt :orig.txt :new2.txt >actual &&
+ git rev-parse :new2.txt >expected &&
+ test_cmp actual expected
+'
+
+test_expect_success 'can accept object ID with --object-id' '
+ git merge-file --object-id $(test_oid empty_blob) $(test_oid empty_blob) :new2.txt >actual &&
+ git rev-parse :new2.txt >expected &&
+ test_cmp actual expected
+'
+
test_expect_success 'works in subdirectory' '
mkdir dir &&
cp new1.txt dir/a.txt &&
@@ -138,6 +157,31 @@ test_expect_success "expected conflict markers" '
test_cmp expect.txt test.txt
'
+test_expect_success "merge with conflicts with --object-id" '
+ git add backup.txt orig.txt new3.txt &&
+ test_must_fail git merge-file -p --object-id :backup.txt :orig.txt :new3.txt >actual &&
+ sed -e "s/<< test.txt/<< :backup.txt/" \
+ -e "s/>> new3.txt/>> :new3.txt/" \
+ expect.txt >expect &&
+ test_cmp expect actual &&
+ test_must_fail git merge-file --object-id :backup.txt :orig.txt :new3.txt >oid &&
+ git cat-file blob "$(cat oid)" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success "merge with conflicts with --object-id with labels" '
+ git add backup.txt orig.txt new3.txt &&
+ test_must_fail git merge-file -p --object-id \
+ -L test.txt -L orig.txt -L new3.txt \
+ :backup.txt :orig.txt :new3.txt >actual &&
+ test_cmp expect.txt actual &&
+ test_must_fail git merge-file --object-id \
+ -L test.txt -L orig.txt -L new3.txt \
+ :backup.txt :orig.txt :new3.txt >oid &&
+ git cat-file blob "$(cat oid)" >actual &&
+ test_cmp expect.txt actual
+'
+
test_expect_success "merge conflicting with --ours" '
cp backup.txt test.txt &&
@@ -256,6 +300,14 @@ test_expect_success 'binary files cannot be merged' '
grep "Cannot merge binary files" merge.err
'
+test_expect_success 'binary files cannot be merged with --object-id' '
+ cp "$TEST_DIRECTORY"/test-binary-1.png . &&
+ git add orig.txt new1.txt test-binary-1.png &&
+ test_must_fail git merge-file --object-id \
+ :orig.txt :test-binary-1.png :new1.txt 2> merge.err &&
+ grep "Cannot merge binary files" merge.err
+'
+
test_expect_success 'MERGE_ZEALOUS simplifies non-conflicts' '
sed -e "s/deerit.\$/deerit;/" -e "s/me;\$/me./" <new5.txt >new6.txt &&
sed -e "s/deerit.\$/deerit,/" -e "s/me;\$/me,/" <new5.txt >new7.txt &&
@@ -389,4 +441,10 @@ test_expect_success 'conflict sections match existing line endings' '
test $(tr "\015" Q <nolf.txt | grep "^[<=>].*Q$" | wc -l) = 0
'
+test_expect_success '--object-id fails without repository' '
+ empty="$(test_oid empty_blob)" &&
+ nongit test_must_fail git merge-file --object-id $empty $empty $empty 2>err &&
+ grep "not a git repository" err
+'
+
test_done
^ permalink raw reply related
* [PATCH v2 0/1] Object ID support for git merge-file
From: brian m. carlson @ 2023-10-30 16:26 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Elijah Newren, Phillip Wood, Eric Sunshine,
Taylor Blau
In-Reply-To: <20231024195655.2413191-1-sandals@crustytoothpaste.net>
This series introduces an --object-id option to git merge-file such
that, instead of reading and writing from files on the system, it reads
from and writes to the object store using blobs.
Changes from v1:
* Improve error handling
* Re-add `-p` argument for documentation
brian m. carlson (1):
merge-file: add an option to process object IDs
Documentation/git-merge-file.txt | 20 +++++++++++
builtin/merge-file.c | 62 +++++++++++++++++++++++---------
t/t6403-merge-file.sh | 58 ++++++++++++++++++++++++++++++
3 files changed, 124 insertions(+), 16 deletions(-)
^ permalink raw reply
* Re: [PATCH 0/1] Object ID support for git merge-file
From: brian m. carlson @ 2023-10-30 16:24 UTC (permalink / raw)
To: Taylor Blau; +Cc: Elijah Newren, git, Junio C Hamano, Phillip Wood
In-Reply-To: <ZT/RpqvfQyx+uzxa@nand.local>
[-- Attachment #1: Type: text/plain, Size: 756 bytes --]
On 2023-10-30 at 15:54:14, Taylor Blau wrote:
> On Sat, Oct 28, 2023 at 11:24:06PM -0700, Elijah Newren wrote:
> > But...wouldn't you already have the conflicts generated when doing the
> > merge and learning that it fails? Why would you need to generate them
> > again?
>
> brian would know better than I do, but I believe the reason is because
> the "attempt this merge" RPC is handled separately from the "show me the
> merge conflict(s) at xyz path". Those probably could be combined
> (obviating the need for this patch), but doing so is probably rather
> complicated.
That's correct. They could in theory happen at different times, which
is why they're not linked.
--
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply
* Re: [PATCH 1/1] merge-file: add an option to process object IDs
From: brian m. carlson @ 2023-10-30 16:14 UTC (permalink / raw)
To: phillip.wood; +Cc: Elijah Newren, git, Junio C Hamano
In-Reply-To: <fec21bbe-46da-4f1c-a9b8-6be44403d68f@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 468 bytes --]
On 2023-10-29 at 10:12:57, Phillip Wood wrote:
> It would be nice to print an error message here
>
> ret = error("object '%s' does not exist", argv[i]);
Good point. I'll include that in my reroll.
> none of the existing error messages are marked for translation so I've left
> this untranslated as well.
I've opted to make them translatable here because I think that's for the
best.
--
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply
* Re: [PATCH] reflog: fix expire --single-worktree
From: René Scharfe @ 2023-10-30 16:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git List, John Cai
In-Reply-To: <xmqqa5s1hxhh.fsf@gitster.g>
Am 29.10.23 um 23:31 schrieb Junio C Hamano:
> René Scharfe <l.s.r@web.de> writes:
>
>> ... and added a non-printable short flag for it, presumably by
>> accident.
>
> Very well spotted.
>
> FWIW, with the following patch on top of this patch, all tests pass
> (and without your fix, of course this notices the "\001" and breaks
> numerous tests that use "git reflog"). So you seem to have found
> the only one broken instance (among those that are tested, anyway).
>
> parse-options.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git i/parse-options.c w/parse-options.c
> index 093eaf2db8..be8bedba29 100644
> --- i/parse-options.c
> +++ w/parse-options.c
> @@ -469,7 +469,8 @@ static void parse_options_check(const struct option *opts)
> optbug(opts, "uses incompatible flags "
> "LASTARG_DEFAULT and OPTARG");
> if (opts->short_name) {
> - if (0x7F <= opts->short_name)
> + if (opts->short_name &&
> + (opts->short_name < 0x21 || 0x7F <= opts->short_name))
Good idea. This is equivalent to !isprint(opts->short_name), which I
find to be more readable here. Seeing why "char short_opts[128];" a
few lines up is big enough would become a bit harder, though.
> optbug(opts, "invalid short name");
> else if (short_opts[opts->short_name]++)
> optbug(opts, "short name already used");
^ permalink raw reply
* Re: [PATCH v3 7/8] ci: install test dependencies for linux-musl
From: Phillip Wood @ 2023-10-30 16:09 UTC (permalink / raw)
To: Patrick Steinhardt, phillip.wood; +Cc: git, Junio C Hamano, Oswald Buddenhagen
In-Reply-To: <ZT_KWjxkPEIXHEeH@tanuki>
On 30/10/2023 15:23, Patrick Steinhardt wrote:
> On Mon, Oct 30, 2023 at 03:13:35PM +0000, Phillip Wood wrote:
>> Hi Patrick
>>
>> On 30/10/2023 12:47, Patrick Steinhardt wrote:
>>> On Mon, Oct 30, 2023 at 01:15:10PM +0100, Patrick Steinhardt wrote:
>>> But once fixed, tests do indeed start to fail:
>>>
>>> t5540-http-push-webdav.sh (Wstat: 256 (exited 1) Tests: 20 Failed: 11)
>>> Failed tests: 5-11, 13, 15-16, 18
>>> Non-zero exit status: 1
>>>
>>> Seems like another thing to fix in a separate patch series.
>>
>> Yes, or we could just leave it - I had not realized before that it was only
>> the musl job that was not running the httpd tests (I thought
>> install-docker-dependencies.sh was missing the packages for ubuntu as well).
>> Given that, the status quo does not seem so bad.
>
> I of course couldn't let go. The following would fix this:
Great, that was fast! As you've got a fix I agree it makes sense to
include it in this series.
Best Wishes
Phillip
> diff --git a/ci/install-docker-dependencies.sh b/ci/install-docker-dependencies.sh
> index 5e28adf55b6..48cb2e735b5 100755
> --- a/ci/install-docker-dependencies.sh
> +++ b/ci/install-docker-dependencies.sh
> @@ -21,7 +21,8 @@ linux32)
> linux-musl)
> apk add --update shadow sudo build-base curl-dev openssl-dev expat-dev gettext \
> pcre2-dev python3 musl-libintl perl-utils ncurses \
> - apache2 bash cvs gnupg perl-cgi perl-dbd-sqlite >/dev/null
> + apache2 apache2-http2 apache2-proxy apache2-ssl apache2-webdav apr-util-dbd_sqlite3 \
> + bash cvs gnupg perl-cgi perl-dbd-sqlite >/dev/null
> ;;
> linux-*)
> apt update -q &&
> diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
> index 9791f94b16f..9ea74927c40 100644
> --- a/t/lib-httpd.sh
> +++ b/t/lib-httpd.sh
> @@ -128,6 +128,20 @@ else
> "Could not identify web server at '$LIB_HTTPD_PATH'"
> fi
>
> +if test -n "$LIB_HTTPD_DAV" && test -f /etc/os-release
> +then
> + case "$(grep "^ID=" /etc/os-release | cut -d= -f2-)" in
> + alpine)
> + # The WebDAV module in Alpine Linux is broken at least up to
> + # Alpine v3.16 as the default DBM driver is missing.
> + #
> + # https://gitlab.alpinelinux.org/alpine/aports/-/issues/13112
> + test_skip_or_die GIT_TEST_HTTPD \
> + "Apache WebDAV module does not have default DBM backend driver"
> + ;;
> + esac
> +fi
> +
> install_script () {
> write_script "$HTTPD_ROOT_PATH/$1" <"$TEST_PATH/$1"
> }
>
> I might as well roll it into this patch series now. It increases test
> coverage on musl libc and doesn't have any significant downsides. In
> fact, it uncovers that tests on Alpine Linux don't work right now, so it
> fixes real issues to have the test coverage in our pipelines.
>
> Patrick
^ permalink raw reply
* Re: please add link / url to remote - when - git push
From: Taylor Blau @ 2023-10-30 15:55 UTC (permalink / raw)
To: Alexander Mills; +Cc: git
In-Reply-To: <CA+KyZp5mwGJ6YOvjKtfnDMDb9ci3vSq5KNUep6-8EfkHNaxREg@mail.gmail.com>
Hi Alexander,
On Sun, Oct 29, 2023 at 06:15:35PM -0500, Alexander Mills wrote:
> Having the link in the console saves me tremendous time and is
> extremely effective/efficient. Can we get links in the console plz?
That link is generated at the remote end (in your case, this is on
GitHub) and then sent over the wire before being printed out with the
"remote:" prefix at your terminal.
The Git project does not itself generate these messages, so you may want
to relay your request to GitHub's support address at <support@github.com>.
Thanks,
Taylor
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox