* [PATCH] tag: fix sign_buffer() call to create a signed tag
From: Junio C Hamano @ 2024-02-07 18:46 UTC (permalink / raw)
To: git; +Cc: Sergey Kosukhin
The command "git tag -s" internally calls sign_buffer() to make a
cryptographic signature using the chosen backend like GPG and SSH.
The internal helper functions used by "git tag" implementation seem
to use a "negative return values are errors, zero or positive return
values are not" convention, and there are places (e.g., verify_tag()
that calls gpg_verify_tag()) that these internal helper functions
translate return values that signal errors to conform to this
convention, but do_sign() that calls sign_buffer() forgets to do so.
Fix it, so that a failed call to sign_buffer() that can return the
exit status from pipe_command() will not be overlooked.
Reported-by: Sergey Kosukhin <skosukhin@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* We alternatively could fix individual sign_buffer() backend that
signals an error with a positive value (sign_buffer_ssh() in this
case) to return a negative value, but this would hopefully be
more future-proof.
builtin/tag.c | 2 +-
gpg-interface.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/tag.c b/builtin/tag.c
index 3918eacbb5..b28ead06ea 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -176,7 +176,7 @@ static int verify_tag(const char *name, const char *ref UNUSED,
static int do_sign(struct strbuf *buffer)
{
- return sign_buffer(buffer, buffer, get_signing_key());
+ return sign_buffer(buffer, buffer, get_signing_key()) ? -1 : 0;
}
static const char tag_template[] =
diff --git a/gpg-interface.h b/gpg-interface.h
index 143cdc1c02..7cd98161f7 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -66,7 +66,7 @@ size_t parse_signed_buffer(const char *buf, size_t size);
* Create a detached signature for the contents of "buffer" and append
* it after "signature"; "buffer" and "signature" can be the same
* strbuf instance, which would cause the detached signature appended
- * at the end.
+ * at the end. Returns 0 on success, non-zero on failure.
*/
int sign_buffer(struct strbuf *buffer, struct strbuf *signature,
const char *signing_key);
--
2.43.0-561-g235986be82
^ permalink raw reply related
* Supporting --no-edit for git rebase --continue
From: Orgad Shaneh @ 2024-02-07 20:46 UTC (permalink / raw)
To: git
Hi,
Is it possible to add --no-edit for git rebase --continue, similar to
the functionality of this flag for git cherry-pick --continue and
similar commands?
This should continue the rebase without activating the commit message
editor, and just keep the existing message.
Thanks,
- Orgad
^ permalink raw reply
* Re: [RFC PATCH v2 4/6] test-tool run-command testsuite: support unit tests
From: Junio C Hamano @ 2024-02-07 20:48 UTC (permalink / raw)
To: Josh Steadmon; +Cc: git, johannes.schindelin, peff, phillip.wood
In-Reply-To: <b5665386b56df91fa5d95ee5b11288b5853546f0.1706921262.git.steadmon@google.com>
Josh Steadmon <steadmon@google.com> writes:
> Teach the testsuite runner in `test-tool run-command testsuite` how to
> run unit tests: if TEST_SHELL_PATH is not set, assume that we're running
> the programs directly from CWD, rather than defaulting to "sh" as an
> interpreter.
Hmph. It sounds more like "the run-command testsuite subcommand
only runs programs in the current directory", not "assume" (which
implies there is a way to override the assumption). Not that the
limitation would hurt us in any way, though.
> + /*
> + * If we run without a shell, we have to provide the relative path to
> + * the executables.
> + */
It sounds more like "If TEST_SHELL_PATH is not given, then we run
them in the current directory.".
It is perfectly fine, because ...
> suite.shell_path = getenv("TEST_SHELL_PATH");
> if (!suite.shell_path)
> - suite.shell_path = "sh";
> + strbuf_addstr(&progpath, "./");
> + path_prefix_len = progpath.len;
>
> dir = opendir(".");
> if (!dir)
> @@ -180,13 +187,17 @@ static int testsuite(int argc, const char **argv)
> ...
> for (i = 0; i < argc; i++)
> if (!wildmatch(argv[i], p, 0)) {
> - string_list_append(&suite.tests, p);
> + strbuf_setlen(&progpath, path_prefix_len);
> + strbuf_addstr(&progpath, p);
> + string_list_append(&suite.tests, progpath.buf);
> break;
> }
> }
... this "prefixing" is done to a path discovered by readdir() from
a directory handle obtained by opendir("."). If there were a way to
pass paths to executables directly, possibly as absolute paths, the
unconditional prefixing of "./" would have been a problem, but we do
not have such a facility, so this should be OK.
^ permalink raw reply
* Re: [PATCH 3/3] rev-list: add --allow-missing-tips to be used with --missing=...
From: Linus Arver @ 2024-02-07 20:48 UTC (permalink / raw)
To: Christian Couder
Cc: git, Junio C Hamano, Patrick Steinhardt, John Cai,
Christian Couder
In-Reply-To: <CAP8UFD3D=a-3cVNpjobOdq=F5dC+aW4qYu3fXCxTnDg=GrnSwA@mail.gmail.com>
Christian Couder <christian.couder@gmail.com> writes:
> On Wed, Feb 7, 2024 at 10:40 AM Linus Arver <linusa@google.com> wrote:
>>
>>
>> Christian Couder <christian.couder@gmail.com> writes:
>>
>> > In 9830926c7d (rev-list: add commit object support in `--missing`
>> > option, 2023-10-27) we fixed the `--missing` option in `git rev-list`
>> > so that it now works with commits too.
>> >
>> > Unfortunately, such a command would still fail with a "fatal: bad
>> > object <oid>" if it is passed a missing commit, blob or tree as an
>> > argument.
>>
>> Hmm, but according to the manpage for rev-list, it only accepts commits
>> as arguments. Conflict...?
>
> It actually kind of "works" when you pass blobs or trees. It looks
> like the command just doesn't use them (except for checking that they
> actually exist) unless options like --objects, --missing=print and
> perhaps others are used. So yeah, the doc might need an update, but I
> think it could be a separate issue, as it's not new and doesn't depend
> on this small series.
SG. But also, if there's a way to put invisible (developer-facing, not
user facing) comments inside the relevant doc file it might be easy
enough to add a to this series.
>> > When such a command is used to find the dependencies of some objects,
>> > for example the dependencies of quarantined objects, it would be
>> > better if the command would instead consider such missing objects,
>> > especially commits, in the same way as other missing objects.
>>
>> Could you define what quarantined objects are (it's not in the manpage
>> for rev-list)?
>
> "quarantined objects" refers to the following doc:
>
> https://www.git-scm.com/docs/git-receive-pack#_quarantine_environment
>
> So to clarify things, the above paragraph looks like the following now:
>
> "When such a command is used to find the dependencies of some objects,
> for example the dependencies of quarantined objects (see the
> "QUARANTINE ENVIRONMENT" section in the git-receive-pack(1)
> documentation), it would be better if the command would instead
> consider such missing objects, especially commits, in the same way as
> other missing objects."
This reads much better, and is a good motivation to keep in the log
message.
>> But also, I'm not sure how much additional value this
>> paragraph is adding on top of what you already said in the first two
>> paragraphs.
>
> It's a more specific example, and it's how we detected the issue we
> would like to address, so I think it has some value.
Agreed.
>> > If, for example `--missing=print` is used, it would be nice for some
>> > use cases if the missing tips passed as arguments were reported in
>> > the same way as other missing objects instead of the command just
>> > failing.
>> >
>> > Let's introduce a new `--allow-missing-tips` option to make it work
>> > like this.
>>
>> I assume "tips" means "the starting commits which are passed into this
>> command from which it begins the rev walk". Might be worth including in
>> the commit message.
>
> I think "tips" is better than "commits" because blobs and trees are
> allowed (see above).
Makes sense.
>> > + oidset_iter_init(&revs.missing_commits, &iter);
>> > + while ((oid = oidset_iter_next(&iter)))
>> > + oidset_insert(&missing_objects, oid);
>> > + oidset_clear(&revs.missing_commits);
>> > + }
>>
>> Aside: I am surprised that there is no helper function already that
>> simply copies things in one oidset into another oidset.
>
> Yeah, I was surprised too. I only found the following similar code in
> list-objects-filter.c:
>
> oidset_iter_init(src, &iter);
> while ((src_oid = oidset_iter_next(&iter)) != NULL)
> oidset_insert(dest, src_oid);
>
> So yeah perhaps we could create such a helper function.
Perhaps a NEEDSWORK comment is appropriate?
>> > traverse_commit_list_filtered(
>> > &revs, show_commit, show_object, &info,
>> > (arg_print_omitted ? &omitted_objects : NULL));
>> > diff --git a/revision.c b/revision.c
>> > index 4c5cd7c3ce..9f25faa249 100644
>> > --- a/revision.c
>> > +++ b/revision.c
>> >
>> > [...]
>> >
>> > @@ -2184,7 +2189,7 @@ static int handle_revision_arg_1(const char *arg_, struct rev_info *revs, int fl
>> > verify_non_filename(revs->prefix, arg);
>> > object = get_reference(revs, arg, &oid, flags ^ local_flags);
>> > if (!object)
>> > - return revs->ignore_missing ? 0 : -1;
>> > + return (revs->ignore_missing || revs->do_not_die_on_missing_tips) ? 0 : -1;
>> > add_rev_cmdline(revs, object, arg_, REV_CMD_REV, flags ^ local_flags);
>> > add_pending_object_with_path(revs, object, arg, oc.mode, oc.path);
>> > free(oc.path);
>>
>> With a few more context lines, the diff looks like
>>
>> --8<---------------cut here---------------start------------->8---
>> if (get_oid_with_context(revs->repo, arg, get_sha1_flags, &oid, &oc))
>> return revs->ignore_missing ? 0 : -1;
>> if (!cant_be_filename)
>> verify_non_filename(revs->prefix, arg);
>> object = get_reference(revs, arg, &oid, flags ^ local_flags);
>> if (!object)
>> - return revs->ignore_missing ? 0 : -1;
>> + return (revs->ignore_missing || revs->do_not_die_on_missing_tips) ? 0 : -1;
>> add_rev_cmdline(revs, object, arg_, REV_CMD_REV, flags ^ local_flags);
>> add_pending_object_with_path(revs, object, arg, oc.mode, oc.path);
>> free(oc.path);
>> return 0;
>> --8<---------------cut here---------------end--------------->8---
>>
>> and it's not obvious to me why you only touched the "if (!object)" case
>> but not the "if (get_oid_with_context(...))" case. Are there any
>> subtleties here that would not be obvious to reviewers?
>
> I thought that if we can't get an oid, we cannot put anything in the
> 'missing_commit' oidset anyway, so it might be better to error out.
Ah, makes sense. This is a subtle detail, and perhaps worth keeping
either as a comment (just above the "if (get_oid_with_context(...))"
case) or in the log message.
>> > diff --git a/t/t6022-rev-list-missing.sh b/t/t6022-rev-list-missing.sh
>> > index 527aa94f07..283e8fc2c2 100755
>> > --- a/t/t6022-rev-list-missing.sh
>> > +++ b/t/t6022-rev-list-missing.sh
>> > @@ -77,4 +77,55 @@ do
>> > done
>> > done
>> >
>> > +for obj in "HEAD~1" "HEAD~1^{tree}" "HEAD:1.t"
>> > +do
>> > + for tip in "" "HEAD"
>>
>> So far from the patch diff it was not obvious that you wanted to check
>> for the empty string as a "tip".
>
> We want to check that things work as expected both:
>
> - when we pass only one missing tip, and:
> - when we pass one missing tip and a tip that is not missing.
Ah, I see. I think you could add a comment like
We want to check that things work as expected both:
- when we pass only one missing tip (when $tip is ""), and:
- when we pass one missing tip and a tip that is not missing (when
$tip is "HEAD").
at the top of the test case, and probably rename $obj to $missing_tip,
and rename $tip to $existing_tip.
Aside: it's a bit unfortunate that the meaning of "missing" becomes
overloaded slightly here because one could say '$tip == ""' is a
"missing" tip becauuse the name is not provided. Subtle. Plus there's
some interplay with the "if (get_oid_with_context(...))" case above
because we will (?) hit that path if we do pass in "" (empty string arg)
as a tip to rev-list. It might be worth saying in the comments in the
implementation, something like
The term "missing" here means the case where we could not find the object
given the object_id. For example, given HEAD~1, its object id (oid)
could be 82335b23aa7234320d6f8055243c852e4b6a5ca3. If no real object
with this oid exists, it is considered missing. Providing an empty
string to rev-list does not touch the "missing tip" codepath.
I wrote the above hastily so it may need further edits to make it
succinct. But the point is that this definition isn't spelled out in the
test case, which makes the "" argument for $tip that much more subtle.
>> > + do
>> > + for action in "allow-any" "print"
>> > + do
>> > + test_expect_success "--missing=$action --allow-missing-tips with tip '$obj' missing and tip '$tip'" '
>>
>> If I run this test without the new --allow-missing-tips flag, I get
>>
>> fatal: bad object 82335b23aa7234320d6f8055243c852e4b6a5ca3
>> not ok 11 - --missing=allow-any --allow-missing-tips with tip 'HEAD~1' missing and tip ''
>>
>> and I think the last "tip ''" part is misleading because IIUC it's not
>> actually passed in as a tip (see my comment below about shell quoting).
>>
>> > + oid="$(git rev-parse $obj)" &&
>> > + path=".git/objects/$(test_oid_to_path $oid)" &&
>> > +
>> > + # Before the object is made missing, we use rev-list to
>> > + # get the expected oids.
>> > + if [ "$tip" = "HEAD" ]; then
>> > + git rev-list --objects --no-object-names \
>> > + HEAD ^$obj >expect.raw
>> > + else
>> > + >expect.raw
>> > + fi &&
>>
>> OK so you are using this empty string to clear the expect.raw file. If
>> that's true then what stops us from doing this at the start of every
>> iteration?
>
> I am not sure what you mean. Both:
>
> git rev-list --objects --no-object-names HEAD ^$obj >expect.raw
>
> and:
>
> >expect.raw #2
>
> are clearing "expect.raw" as ">expect.raw" is used in both cases.
>
> If we did the latter at the start of every iteration, then when we do
> the former afterwards, we would clear "expect.raw" raw again, while
> there is no point in clearing it twice.
Yes but doing it that way would separate "set up a clean environment for
this test case" from "find the oid of the non-missing tip" from each
other in the same if/else stanza, which I believe helps readability.
>> for the case where tip is the empty string. I wonder if we could avoid
>> being nuanced here with subtle shell quoting rules, especially because
>> these are tests (where making everything as explicit as possible is
>> desirable).
>
> I think it's not very subtle, but perhaps a comment would help. So I
> added the following to my current version before the loop:
>
> # We want to check that things work when both
> # - all the tips passed are missing (case tip = ""), and
> # - there is one missing tip and one existing tip (case tip = "HEAD")
Great, thanks.
>> > + # When the action is to print, we should also add the missing
>> > + # oid to the expect list.
>> > + case $action in
>> > + allow-any)
>> > + ;;
>> > + print)
>> > + grep ?$oid actual.raw &&
>> > + echo ?$oid >>expect.raw
>> > + ;;
>> > + esac &&
>> > +
>> > + sort actual.raw >actual &&
>> > + sort expect.raw >expect &&
>> > + test_cmp expect actual
>> > + '
>> > + done
>> > + done
>> > +done
>> > +
>>
>> Wait, but where are we actually making the $tip commit's object
>> _missing_? Hmm...
>>
>> Ah, actually the tips are just the $obj variables. So it seems like you
>> could have done
>>
>> for tip in "HEAD~1" "HEAD~1^{tree}" "HEAD:1.t"
>>
>> in the beginning to make it more obvious.
>
> The previous tests in this test script used "oid" as the variable name
> for possibly missing objects, so I did the same in the tests I added.
Didn't the previous tests _always_ make those objects missing?
> I could have used "tip" and "extra_tip" instead of "oid" and "tip" or
> perhaps "oid" and "extra_oid", but I don't think it makes a big
> difference in understanding these tests.
I agree that it doesn't make a big difference now (to me at least), but
I worry for the other future Git developers who'll need to do that small
amount of mental effort to gain the same understanding as us in this
review process. I would still prefer the names "missing_tip" (or
"possibly_missing_tip" if my immediate comment above is not correct) and
"existing_tip" as suggested in my other comment.
>> Also, given how most of this is identical from the loop already in t6022
>> just above it, it would help to add a comment at the top of this one
>> explaining how it's different than the one above it.
>
> The one I added has an extra `for tip in "" "HEAD"` loop. Anyway I
> think I will just modify the existing test in the next version I will
> send, as I plan to implement Junio's suggestion and there will be no
> new option.
SG.
> Thanks for the review!
You're welcome.
Now that I have your attention (was this my plan all along? ;) ), I will
take this opportunity to ping you directly for a review of my own patch
series for the trailers subsystem:
https://lore.kernel.org/git/pull.1632.v4.git.1707196348.gitgitgadget@gmail.com/
which is in its 4th iteration after many helpful comments from Junio.
Please don't let the patch count (28) raise any alarms --- they used to
be 10 but I broke them down into smaller steps to ease the review
process.
Thanks.
^ permalink raw reply
* Re: [RFC PATCH v2 5/6] unit tests: add rule for running with test-tool
From: Junio C Hamano @ 2024-02-07 20:50 UTC (permalink / raw)
To: Josh Steadmon; +Cc: git, johannes.schindelin, peff, phillip.wood
In-Reply-To: <f2746703d554d65d41afe0e41b1c9757427cda26.1706921262.git.steadmon@google.com>
Josh Steadmon <steadmon@google.com> writes:
> +unit-tests-test-tool:
> + @echo "*** test-tool - unit tests **"
> + ( \
> + cd unit-tests/bin && \
> + ../../helper/test-tool$X run-command testsuite $(UNIT_TESTS_NO_DIR)\
> + )
This limits us to the tests that the Makefile knows about, which is
good. Of course, we might bust the command line length limit some
day when we have too many unit test executables, but for now this is
simple, clean, and readable.
Thanks.
^ permalink raw reply
* git gc destroys autostash
From: Orgad Shaneh @ 2024-02-07 20:52 UTC (permalink / raw)
To: git
Running git gc --prune=now during rebase with autostash deletes the
autostash object, and it cannot be recovered when the rebase ends.
Example:
#!/bin/sh
git init
echo 1 > foo; git add foo; git commit -m 'Initial commit'
echo 2 > foo; git add foo; git commit -m 'Second commit'
echo 3 > foo; git rebase -i --autostash HEAD^
# Choose edit
git gc --prune=now
git rebase --continue
# fatal: '3b88163a1bff3859a005554c168d94e5357ee45b' is not a stash-like commit
# error: cannot store 3b88163a1bff3859a005554c168d94e5357ee45b
# Successfully rebased and updated refs/heads/master.
Thanks,
- Orgad
^ permalink raw reply
* Re: [RFC PATCH v2 6/6] t/Makefile: run unit tests alongside shell tests
From: Junio C Hamano @ 2024-02-07 20:55 UTC (permalink / raw)
To: Josh Steadmon; +Cc: git, johannes.schindelin, peff, phillip.wood
In-Reply-To: <cd7467a7bd51fbc01c999ee1bd7688770b1d11e5.1706921262.git.steadmon@google.com>
Josh Steadmon <steadmon@google.com> writes:
> From: Jeff King <peff@peff.net>
>
> Add a wrapper script to allow `prove` to run both shell tests and unit
> tests from a single invocation. This avoids issues around running prove
> twice in CI, as discussed in [1].
>
> Additionally, this moves the unit tests into the main dev workflow, so
> that errors can be spotted more quickly.
>
> NEEDS WORK: as discussed in previous commits in this series, there's a
> desire to avoid `prove` specifically and (IIUC) unnecessary
> fork()/exec()ing in general on Windows. This change adds an extra exec()
> for each shell and unit test execution, will that be a problem for
> Windows?
>
> [1] https://lore.kernel.org/git/pull.1613.git.1699894837844.gitgitgadget@gmail.com/
>
> Signed-off-by: Jeff King <peff@peff.net>
> Signed-off-by: Josh Steadmon <steadmon@google.com>
> ---
> t/Makefile | 2 +-
> t/run-test.sh | 13 +++++++++++++
> 2 files changed, 14 insertions(+), 1 deletion(-)
> create mode 100755 t/run-test.sh
>
> diff --git a/t/Makefile b/t/Makefile
> index 6e6316c29b..6a67fc22d7 100644
> --- a/t/Makefile
> +++ b/t/Makefile
> @@ -64,7 +64,7 @@ failed:
> test -z "$$failed" || $(MAKE) $$failed
>
> prove: pre-clean check-chainlint $(TEST_LINT)
> - @echo "*** prove ***"; $(CHAINLINTSUPPRESS) $(PROVE) --exec '$(TEST_SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS)
> + @echo "*** prove (shell & unit tests) ***"; $(CHAINLINTSUPPRESS) $(PROVE) --exec ./run-test.sh $(GIT_PROVE_OPTS) $(T) $(UNIT_TESTS) :: $(GIT_TEST_OPTS)
> $(MAKE) clean-except-prove-cache
>
> $(T):
> diff --git a/t/run-test.sh b/t/run-test.sh
> new file mode 100755
> index 0000000000..c29fef48dc
> --- /dev/null
> +++ b/t/run-test.sh
> @@ -0,0 +1,13 @@
> +#!/bin/sh
> +
> +# A simple wrapper to run shell tests via TEST_SHELL_PATH,
> +# or exec unit tests directly.
> +
> +case "$1" in
> +*.sh)
> + exec ${TEST_SHELL_PATH:-/bin/sh} "$@"
> + ;;
> +*)
> + exec "$@"
> + ;;
> +esac
Hmph. This penalizes the non-unit tests by doing an extra "exec",
once per program?
Of course we cannot run two $(PROVE) invocations serially, one for
doing $(T) and the other for doing $(UNIT_TESTS)?
^ permalink raw reply
* Re: [RFC PATCH v2 1/6] t0080: turn t-basic unit test into a helper
From: Junio C Hamano @ 2024-02-07 20:55 UTC (permalink / raw)
To: Josh Steadmon; +Cc: git, johannes.schindelin, peff, phillip.wood
In-Reply-To: <da756b4bfb9d1ce0d1213d585e72acfbf667e2a2.1706921262.git.steadmon@google.com>
Josh Steadmon <steadmon@google.com> writes:
> -UNIT_TEST_PROGRAMS = $(patsubst unit-tests/%.c,unit-tests/bin/%$(X),$(UNIT_TEST_SOURCES))
> -UNIT_TESTS = $(sort $(filter-out unit-tests/bin/t-basic%,$(UNIT_TEST_PROGRAMS)))
> +UNIT_TESTS = $(patsubst unit-tests/%.c,unit-tests/bin/%$(X),$(UNIT_TEST_SOURCES))
Nice that we no longer need the special casing.
> diff --git a/t/unit-tests/t-basic.c b/t/helper/test-example-tap.c
> similarity index 95%
> rename from t/unit-tests/t-basic.c
> rename to t/helper/test-example-tap.c
> index fda1ae59a6..21e4848e78 100644
> --- a/t/unit-tests/t-basic.c
> +++ b/t/helper/test-example-tap.c
> @@ -1,4 +1,5 @@
> -#include "test-lib.h"
> +#include "t/unit-tests/test-lib.h"
> +#include "test-tool.h"
As the first thing both of these headers include is
"git-compat-util.h", so the ordering should be safe either way,
because everybody else in the directory seems to include
"test-tool.h" before including headers that are specific to the
subsystem it is testing, and "t/unit-tests/test-lib.h" in this case
is the header that is specific to the unit-test subsystem being
tested, it may raise fewer eyebrows if we swapped the order of the
inclusion here.
Other than that, looks good to me.
Thanks.
^ permalink raw reply
* Re: [RFC PATCH v2 2/6] test-tool run-command testsuite: get shell from env
From: Junio C Hamano @ 2024-02-07 20:55 UTC (permalink / raw)
To: Josh Steadmon; +Cc: git, johannes.schindelin, peff, phillip.wood
In-Reply-To: <c8448406d71151714e89893208c46b8a4c369cb5.1706921262.git.steadmon@google.com>
Josh Steadmon <steadmon@google.com> writes:
> When running tests through `test-tool run-command testsuite`, we
> currently hardcode `sh` as the command interpreter. As discussed in [1],
> this is incorrect, and we should be using the shell set in
> TEST_SHELL_PATH instead.
>
> Add a shell_path field in struct testsuite so that we can pass this to
> the task runner callback. If this is non-null, we'll use it as the
> argv[0] of the subprocess. Otherwise, we'll just execute the test
> program directly.
That sounds nice in theory, but ...
> When setting up the struct testsuite in testsuite(), use the value
> of TEST_SHELL_PATH if it's set, otherwise default to `sh`.
... this done in the testsuite() function, doesn't suite.shell_path
always gets some non-NULL value? Perhaps in a later step we will
add a mechanism to allow suite.shell_path to be NULL when we know
we are running an executable, or something?
Leaving readers in a bit of suspense may, especially in a series
that is short like this, be a good technique to entice them to keep
reading, perhaps, but anyway, if that is what is intended, a simple
"this feature is not used in this step, but we will take advantage
of it soon in a later step" would be a good idea.
^ permalink raw reply
* Re: [RFC PATCH v2 3/6] test-tool run-command testsuite: remove hardcoded filter
From: Junio C Hamano @ 2024-02-07 20:55 UTC (permalink / raw)
To: Josh Steadmon; +Cc: git, johannes.schindelin, peff, phillip.wood
In-Reply-To: <e1b89ae93e930cd902d1527955d588c3d0c15490.1706921262.git.steadmon@google.com>
Josh Steadmon <steadmon@google.com> writes:
> `test-tool run-command testsuite` currently assumes that it will only be
> running the shell test suite, and therefore filters out anything that
> does not match a hardcoded pattern of "t[0-9][0-9][0-9][0-9]-*.sh".
>
> Later in this series, we'll adapt `test-tool run-command testsuite` to
> also support unit tests, which do not follow the same naming conventions
> as the shell tests, so this hardcoded pattern is inconvenient.
Makes sense to explain what future steps this prepares the codebase
for like this.
> Since `testsuite` also allows specifying patterns on the command-line,
> let's just remove this pattern. As noted in [1], there are no longer any
> uses of `testsuite` in our codebase, it should be OK to break backwards
> compatibility in this case. We also add a new filter to avoid trying to
> execute "." and "..", so that users who wish to execute every test in a
> directory can do so without specifying a pattern.
As we discussed in Peff's Makefile change that enumerates "which are
the unit-test programs?" Generally, $(wildcard) and readdir() to
slurp everything in a directory, including stuff that is an
untracked cruft, is not an excellent idea.
This is not an end-user facing program and we are in full control of
its input (most notably, "which ones should we be running?"), I do
not think it would be a huge issue, though.
> [1] https://lore.kernel.org/git/850ea42c-f103-68d5-896b-9120e2628686@gmx.de/
>
> Signed-off-by: Josh Steadmon <steadmon@google.com>
> ---
> t/helper/test-run-command.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c
> index a41a54d9cb..e6bd792274 100644
> --- a/t/helper/test-run-command.c
> +++ b/t/helper/test-run-command.c
> @@ -175,9 +175,7 @@ static int testsuite(int argc, const char **argv)
> while ((d = readdir(dir))) {
> const char *p = d->d_name;
>
> - if (*p != 't' || !isdigit(p[1]) || !isdigit(p[2]) ||
> - !isdigit(p[3]) || !isdigit(p[4]) || p[5] != '-' ||
> - !ends_with(p, ".sh"))
> + if (!strcmp(p, ".") || !strcmp(p, ".."))
> continue;
>
> /* No pattern: match all */
^ permalink raw reply
* Interest in options for stash -k and -u by default
From: Ricardo C @ 2024-02-07 20:59 UTC (permalink / raw)
To: git
[-- Attachment #1.1.1: Type: text/plain, Size: 1220 bytes --]
Hello,
After some discussion on Mastodon[1], I decided to try my hand at creating
configuration options that enable --keep-index and --include-untracked in
git-stash by default. I have made a preliminary such patch[2], and some
additional discussion[3] has encouraged me to try submitting it upstream.
I am wondering whether this change is something that would even be considered
for being accepted. The patch still needs some work (mainly documentation and
tests), which I'd be happy to do if I knew there was upstream interest in this
work.
I would also appreciate feedback on the patch. Currently, it creates two
separate options: stash.keepindex and stash.includeuntracked. These options
set the default value of --keep-index and --include-untracked if not otherwise
specified (e.g., --no-keep-index) and do not conflict with other options
(e.g., --patch). More details are of course available in the source code[2].
Thank you,
Ricardo "MithicSpirit" Prado Cunha
[1] https://bsd.network/@ed1conf/111783574839749798
[2]
https://github.com/MithicSpirit/git/commit/f33c8d5d26d8438ddb123781bd5db3bff8618247.patch
[3] https://social.jvns.ca/@b0rk/111880230506448122
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3215 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply
* Re: git gc destroys autostash
From: Junio C Hamano @ 2024-02-07 21:05 UTC (permalink / raw)
To: Orgad Shaneh; +Cc: git
In-Reply-To: <CAGHpTBKpYp370QTw93wK_RP+X2S+44jd-8kjodiUj4k0BoAEqA@mail.gmail.com>
Orgad Shaneh <orgads@gmail.com> writes:
> Running git gc --prune=now during rebase with autostash deletes the
> autostash object, and it cannot be recovered when the rebase ends.
>
> Example:
>
> #!/bin/sh
>
> git init
> echo 1 > foo; git add foo; git commit -m 'Initial commit'
> echo 2 > foo; git add foo; git commit -m 'Second commit'
> echo 3 > foo; git rebase -i --autostash HEAD^
> # Choose edit
> git gc --prune=now
This is totally expected, unfortunately, as the autostash does not
use the ref API to use the refs/stash (presumably in order to avoid
shifting the shash@{$N} numbers). Because of that, the stash entry
is not protected from the garbage collection. This currently falls
into "it hurts when I twist my arm this way. --do not do it then"
category.
It may be a simple fix to teach sequencer.c:apply_save_autostash()
not to use a random on-disk file to store the returned value from
"git stash create", but use a dedicated ref that is not refs/stash
for its own use via proper use of the ref API.
> git rebase --continue
> # fatal: '3b88163a1bff3859a005554c168d94e5357ee45b' is not a stash-like commit
> # error: cannot store 3b88163a1bff3859a005554c168d94e5357ee45b
> # Successfully rebased and updated refs/heads/master.
>
> Thanks,
> - Orgad
^ permalink raw reply
* Re: Interest in options for stash -k and -u by default
From: Junio C Hamano @ 2024-02-07 21:08 UTC (permalink / raw)
To: Ricardo C; +Cc: git
In-Reply-To: <3470180e-2ef7-4393-9d32-92cd419727f5@gmail.com>
Ricardo C <rpc01234@gmail.com> writes:
> I am wondering whether this change is something that would even be
> considered for being accepted. The patch still needs some work (mainly
> documentation and tests), which I'd be happy to do if I knew there was
> upstream interest in this work.
There is a canned response for it ;-)
I've seen from time to time people ask "I am thinking of doing this;
will a patch be accepted? If so, I'll work on it." before showing
any work, and my response always has been:
(1) We don't know how useful and interesting your contribution would
be for our audience, until we see it; and
(2) If you truly believe in your work (find it useful, find writing
it fun, etc.), that would be incentive enough for you to work
on it, whether or not the result will land in my tree. You
should instead aim for something so brilliant that we would
come to you begging for your permission to include it in our
project.
^ permalink raw reply
* Re: [RFC PATCH v2 0/6] test-tool: add unit test suite runner
From: Junio C Hamano @ 2024-02-07 21:14 UTC (permalink / raw)
To: Josh Steadmon; +Cc: git, johannes.schindelin, peff, phillip.wood
In-Reply-To: <cover.1706921262.git.steadmon@google.com>
Josh Steadmon <steadmon@google.com> writes:
> Some known remaining bits of work:
> * We should investigate switching the Windows CI to use `test-tool`
> instead of prove. However, Windows CI seems broken on
> jk/unit-tests-buildfix, and I haven't had time to determine why.
> * We should determine whether it is confusing or otherwise harmful to
> people's workflow to have the unit tests run in parallel with shell
> tests when using prove as the default test target.
The first one seems to have been resolved. The latter can happen
while it cooks, I would presume. If there is no other comments,
let's mark the topic for 'next'.
Thanks.
^ permalink raw reply
* Re: [PATCH v2 3/5] t4301: verify that merge-tree fails on missing blob objects
From: Junio C Hamano @ 2024-02-07 21:24 UTC (permalink / raw)
To: Eric Sunshine
Cc: Johannes Schindelin via GitGitGadget, git, Patrick Steinhardt,
Johannes Schindelin
In-Reply-To: <CAPig+cSJz3U+vT==NhX5hcrTjsCggnAzhzQOvZcSXbcEGuYaKQ@mail.gmail.com>
Eric Sunshine <sunshine@sunshineco.com> writes:
>> diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh
>> @@ -961,4 +961,18 @@ test_expect_success 'error out on missing tree objects' '
>> +test_expect_success 'error out on missing blob objects' '
>> + seq1=$(test_seq 1 10 | git hash-object -w --stdin) &&
>> + seq2=$(test_seq 1 11 | git hash-object -w --stdin) &&
>> + seq3=$(test_seq 0 10 | git hash-object -w --stdin) &&
>
> Is there significance to the ranges passed to test_seq()? Or, can the
> same be achieved by using arbitrary content for each blob?
>
> blob1=$(echo "one" | git hash-object -w --stdin) &&
> blob2=$(echo "two" | git hash-object -w --stdin) &&
> blob3=$(echo "three" | git hash-object -w --stdin) &&
>
>> + tree1=$(printf "100644 blob %s\tsequence" $seq1 | git mktree) &&
>> + tree2=$(printf "100644 blob %s\tsequence" $seq2 | git mktree) &&
>> + tree3=$(printf "100644 blob %s\tsequence" $seq3 | git mktree) &&
>
> I found the lack of terminating "\n" in the `printf` confusing,
> especially since the variable names (seq1, seq2, etc.) and the use of
> `printf` seem to imply, at first glance, that each git-mktree
> invocation is receiving multiple lines as input which, it turns out,
> is not the case. Adding the missing "\n" would help:
>
> tree1=$(printf "100644 blob %s\tsequence\n" $seq1 | git mktree) &&
> tree2=$(printf "100644 blob %s\tsequence\n" $seq2 | git mktree) &&
> tree3=$(printf "100644 blob %s\tsequence\n" $seq3 | git mktree) &&
>
> Interpolating the $seqN variable directly into the string rather than
> using %s would make it even clearer that only a single line is being
> generated as input to git-mktree:
>
> tree1=$(printf "100644 blob $seq1\tsequence\n" | git mktree) &&
> tree2=$(printf "100644 blob $seq2\tsequence\n" | git mktree) &&
> tree3=$(printf "100644 blob $seq3\tsequence\n" | git mktree) &&
All good points. Thanks for excellent reviews (not just this one
but another one in the series).
^ permalink raw reply
* Re: Interest in options for stash -k and -u by default
From: Kristoffer Haugsbakk @ 2024-02-07 21:42 UTC (permalink / raw)
To: Ricardo C; +Cc: git
In-Reply-To: <3470180e-2ef7-4393-9d32-92cd419727f5@gmail.com>
On Wed, Feb 7, 2024, at 21:59, Ricardo C wrote:
> Hello,
>
>
> After some discussion on Mastodon[1], I decided to try my hand at creating
> configuration options that enable --keep-index and --include-untracked in
> git-stash by default. I have made a preliminary such patch[2], and some
> additional discussion[3] has encouraged me to try submitting it upstream.
>
> I am wondering whether this change is something that would even be considered
> for being accepted. The patch still needs some work (mainly documentation and
> tests), which I'd be happy to do if I knew there was upstream interest in this
> work.
>
> I would also appreciate feedback on the patch. Currently, it creates two
> separate options: stash.keepindex and stash.includeuntracked. These options
> set the default value of --keep-index and --include-untracked if not otherwise
> specified (e.g., --no-keep-index) and do not conflict with other options
> (e.g., --patch). More details are of course available in the source code[2].
The way I understand it:
• You already have a working implementation
• There already exists conventions for using config variables for
defaults which then can be in turn overridden using the corresponding
flags (the `--[no]-<flag>` pair)
So what’s the hold-up? :)
> I would also appreciate feedback on the patch.
That would be part of the patch series in any case.
--
Kristoffer Haugsbakk
^ permalink raw reply
* [PATCH 0/2] minute "git bisect" doc updates
From: Junio C Hamano @ 2024-02-07 21:44 UTC (permalink / raw)
To: git
In-Reply-To: <24a42fa6-7bc4-4a3b-8bf4-a0ef85dc457a@matthieu-moy.fr>
"git bisect" documentation was a bit sketchy on alternative keywords
"new" and "old", that are used to signal if a command is from the
part of the history that is newer or older than "a significant
event" the bisection is trying to find. Here are two small patches
that improves the documentation.
I am trying to flush my "stalled topics" queue. Here is a small and
(hopefully) easy-to-finish one.
The original discussion was from early December 2023 and can be
found at
https://lore.kernel.org/git/CAC4O8c9ieZC4SBJf54ZuTfAvnkhGuDaibBQ-m9Zw_n5VhUFPag@mail.gmail.com/
Junio C Hamano (2):
bisect: document "terms" subcommand more fully
bisect: document command line arguments for "bisect start"
Documentation/git-bisect.txt | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
--
2.43.0-561-g235986be82
^ permalink raw reply
* [PATCH 1/2] bisect: document "terms" subcommand more fully
From: Junio C Hamano @ 2024-02-07 21:44 UTC (permalink / raw)
To: git; +Cc: Matthieu Moy
In-Reply-To: <20240207214436.538586-1-gitster@pobox.com>
The documentation for "git bisect terms", although it did not hide
any information, was a bit incomplete and forced readers to fill in
the blanks to get the complete picture.
Acked-by: Matthieu Moy <git@matthieu-moy.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/git-bisect.txt | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index fbb39fbdf5..3d813f9c77 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -20,7 +20,7 @@ on the subcommand:
[--no-checkout] [--first-parent] [<bad> [<good>...]] [--] [<paths>...]
git bisect (bad|new|<term-new>) [<rev>]
git bisect (good|old|<term-old>) [<rev>...]
- git bisect terms [--term-good | --term-bad]
+ git bisect terms [--term-(good|old) | --term-(bad|new)]
git bisect skip [(<rev>|<range>)...]
git bisect reset [<commit>]
git bisect (visualize|view)
@@ -165,8 +165,10 @@ To get a reminder of the currently used terms, use
git bisect terms
------------------------------------------------
-You can get just the old (respectively new) term with `git bisect terms
---term-old` or `git bisect terms --term-good`.
+You can get just the old term with `git bisect terms --term-old`
+or `git bisect terms --term-good`; `git bisect terms --term-new`
+and `git bisect terms --term-bad` can be used to learn how to call
+the commits more recent than the sought change.
If you would like to use your own terms instead of "bad"/"good" or
"new"/"old", you can choose any names you like (except existing bisect
--
2.43.0-561-g235986be82
^ permalink raw reply related
* [PATCH 2/2] bisect: document command line arguments for "bisect start"
From: Junio C Hamano @ 2024-02-07 21:44 UTC (permalink / raw)
To: git; +Cc: Matthieu Moy
In-Reply-To: <20240207214436.538586-1-gitster@pobox.com>
The syntax commonly used for alternatives is --opt-(a|b), not
--opt-{a,b}.
List bad/new and good/old consistently in this order, to be
consistent with the description for "git bisect terms". Clarify
<term> to either <term-old> or <term-new> to make them consistent
with the description of "git bisect (good|bad)" subcommands.
Suggested-by: Matthieu Moy <git@matthieu-moy.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/git-bisect.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index 3d813f9c77..73f889b97b 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -16,7 +16,7 @@ DESCRIPTION
The command takes various subcommands, and different options depending
on the subcommand:
- git bisect start [--term-{new,bad}=<term> --term-{old,good}=<term>]
+ git bisect start [--term-(bad|new)=<term-new> --term-(good|old)=<term-old>]
[--no-checkout] [--first-parent] [<bad> [<good>...]] [--] [<paths>...]
git bisect (bad|new|<term-new>) [<rev>]
git bisect (good|old|<term-old>) [<rev>...]
--
2.43.0-561-g235986be82
^ permalink raw reply related
* Re: git gc destroys autostash
From: Kristoffer Haugsbakk @ 2024-02-07 21:47 UTC (permalink / raw)
To: Orgad Shaneh; +Cc: git
In-Reply-To: <CAGHpTBKpYp370QTw93wK_RP+X2S+44jd-8kjodiUj4k0BoAEqA@mail.gmail.com>
On Wed, Feb 7, 2024, at 21:52, Orgad Shaneh wrote:
> Running git gc --prune=now during rebase with autostash deletes the
> autostash object, and it cannot be recovered when the rebase ends.
Things like this is why I don’t use the stash.[1][2] I just commit
everything of value and non-value.
† 1: With very rare exceptions where the stash is popped before the
whole operation leaves my own working memory
† 2: Also how you have to find unreachable objects using git-fsck(1) if
you pop the last stash
--
Kristoffer Haugsbakk
^ permalink raw reply
* Re: [PATCH v3 5/7] refs: add pseudorefs array and iteration functions
From: Junio C Hamano @ 2024-02-07 22:02 UTC (permalink / raw)
To: Kousik Sanagavarapu; +Cc: Andy Koppe, git
In-Reply-To: <ZcEvLwp0t8-rcyGn@five231003>
Kousik Sanagavarapu <five231003@gmail.com> writes:
> Andy Koppe <andy.koppe@gmail.com> wrote:
> ...
>> +static const char *const pseudorefs[] = {
>> + "FETCH_HEAD",
>> + "ORIG_HEAD",
>> + "MERGE_HEAD",
>> + "REBASE_HEAD",
>> + "CHERRY_PICK_HEAD",
>> + "REVERT_HEAD",
>> + "BISECT_HEAD",
>> + "AUTO_MERGE",
>> +};
>> +
>> struct ref_namespace_info ref_namespace[] = {
>> [NAMESPACE_HEAD] = {
>> .ref = "HEAD",
>> @@ -1549,6 +1564,33 @@ int head_ref(each_ref_fn fn, void *cb_data)
>> return refs_head_ref(get_main_ref_store(the_repository), fn, cb_data);
>> }
>
> The first thing that popped up in my head was "Should we somehow use
> is_pseudoref_syntax() instead of manually listing these?" (although I
> read in this thread later that Junio was okay with the listing) but then ...
>
> I thought I saw something similar in some other thread (which entered
> the mailing list much after this patch series was submitted) ...
>
> https://lore.kernel.org/git/20231221170715.110565-2-karthik.188@gmail.com/T/
We are halting Karthik's topic to rethink its UI for now, but your
point stands. We should use a unified definition of what pseudorefs
there are across the codebase for consistency, and Karthik's topic
would be a better place to do so.
Andy, let me drop this topic for now from my tree, and let's wait
until Karthik's "iterate over all refs" topic solidifies, at which
time an updated iteration (v4?) of this topic hopefully can build
on top of it.
Thanks.
^ permalink raw reply
* Re: [PATCH v2 0/5] completion: remove hardcoded config variable names
From: Junio C Hamano @ 2024-02-07 22:08 UTC (permalink / raw)
To: git; +Cc: Philippe Blain via GitGitGadget, Philippe Blain
In-Reply-To: <pull.1660.v2.git.git.1706534881.gitgitgadget@gmail.com>
"Philippe Blain via GitGitGadget" <gitgitgadget@gmail.com> writes:
> Changes since v1:
>
> * Corrected my email in PATCH 2/5 (sorry for the noise)
>
> v1: This series removes hardcoded config variable names in the
> __git_complete_config_variable_name function, partly by adding a new mode to
> 'git help'. It also adds completion for 'submodule.*' config variables,
> which were previously missing.
>
> I think it makes sense to do that in the same series since it's closely
> related, and splitting it would result in textual conflicts between both
> series if one does not build on top of the other, but I'm open to other
> suggestions.
>
> Thanks,
Neither rounds of this series unfortunately got any review.
Comments from anybody interested in helping to improve completion
scripts?
Thanks.
^ permalink raw reply
* Re: [PATCH v2 1/1] completion: don't complete revs when --no-format-patch
From: Junio C Hamano @ 2024-02-07 22:12 UTC (permalink / raw)
To: git; +Cc: Britton Leo Kerin
In-Reply-To: <a718b5ee-afb0-44bd-a299-3208fac43506@smtp-relay.sendinblue.com>
"Britton Leo Kerin" <britton.kerin@gmail.com> writes:
> In this case the user has specifically said they don't want send-email
> to run format-patch so revs aren't valid argument completions (and it's
> likely revs and dirs do have some same names or prefixes as in
> Documentation/MyFirstContribution.txt 'psuh').
>
> Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
> ---
> contrib/completion/git-completion.bash | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
Any reviews people want to offer to this one?
Thanks.
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 185b47d802..c983f3b2ab 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1242,10 +1242,12 @@ __git_find_last_on_cmdline ()
> while test $# -gt 1; do
> case "$1" in
> --show-idx) show_idx=y ;;
> + --) shift && break ;;
> *) return 1 ;;
> esac
> shift
> done
> + [ $# -eq 1 ] || return 1 # return 1 if we got wrong # of non-opts
> local wordlist="$1"
>
> while [ $c -gt "$__git_cmd_idx" ]; do
> @@ -2429,7 +2431,9 @@ _git_send_email ()
> return
> ;;
> esac
> - __git_complete_revlist
> + if [ "$(__git_find_last_on_cmdline -- "--format-patch --no-format-patch")" != "--no-format-patch" ]; then
> + __git_complete_revlist
> + fi
> }
>
> _git_stage ()
^ permalink raw reply
* Re: [PATCH v4 0/3] apply with core.filemode=false
From: Junio C Hamano @ 2024-02-07 22:15 UTC (permalink / raw)
To: git
In-Reply-To: <20231226233218.472054-1-gitster@pobox.com>
Junio C Hamano <gitster@pobox.com> writes:
> Chandra Pratap noticed that "git apply" on a filesystem without
> executable bit support gives a warning when applying a patch that
> expects the preimage file to have executable bit on. Dscho noticed
> that the initial fix by Chandra did not work well when applying a
> patch in reverse. It turns out that apply.c:reverse_patches()
> invalidates the "a patch that does not change mode bits have the
> mode bits in .old_mode member and not in .new_mode member" invariant
> we rely on.
>
> Here is the result of concerted effort.
>
> Chandra Pratap (1):
> apply: ignore working tree filemode when !core.filemode
>
> Junio C Hamano (2):
> apply: correctly reverse patch's pre- and post-image mode bits
> apply: code simplification
>
> apply.c | 16 +++++++++++++---
> t/t4129-apply-samemode.sh | 27 +++++++++++++++++++++++++++
> 2 files changed, 40 insertions(+), 3 deletions(-)
Anybody wants to offer a review on this? I actually am fairly
confortable with these without any additional review, but since I am
sweeping the "Needs review" topics in the What's cooking report, I
thought I would ask for this one, too.
^ permalink raw reply
* Re: [PATCH v2 00/30] initial support for multiple hash functions
From: Junio C Hamano @ 2024-02-07 22:18 UTC (permalink / raw)
To: git; +Cc: Eric W. Biederman, brian m. carlson, Eric Sunshine,
Eric W. Biederman
In-Reply-To: <878r8l929e.fsf@gmail.froward.int.ebiederm.org>
"Eric W. Biederman" <ebiederm@gmail.com> writes:
> This addresses all of the known test failures from v1 of this set of
> changes. In particular I have reworked commit_tree_extended which
> was flagged by smatch, -Werror=array-bounds, and the leak detector.
>
> One functional bug was fixed in repo_for_each_abbrev where it was
> mistakenly displaying too many ambiguous oids.
>
> I am posting this so that people review and testing of this patchset
> won't be distracted by the known and fixed issues.
We haven't seen any reviews on this second round, and have had it
outside 'next' for too long. I am tempted to say that we merge it
to 'next' and see if anybody screams at this point.
Thanks.
^ 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