* Re: [PATCH v4 12/15] replay: disallow revision specific options and pathspecs
From: Christian Couder @ 2023-10-10 12:49 UTC (permalink / raw)
To: Johannes Schindelin
Cc: git, Junio C Hamano, Patrick Steinhardt, Elijah Newren, John Cai,
Derrick Stolee, Phillip Wood, Calvin Wan, Toon Claes,
Christian Couder
In-Reply-To: <f0e75d47-c277-9fbb-7bcd-53e4e5686f3c@gmx.de>
Hi Dscho,
On Thu, Sep 7, 2023 at 1:03 PM Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> As pointed out elsewhere in this mail thread, I consider this patch to do
> more harm than good. After switching the command to plumbingmanipulators
> it should be possible to just forego all command-line validation and leave
> that job to the caller.
>
> Therefore I would love to see this patch dropped.
Ok, I have dropped this patch in version 5.
^ permalink raw reply
* Re: [PATCH v4 00/15] Introduce new `git replay` command
From: Christian Couder @ 2023-10-10 12:50 UTC (permalink / raw)
To: Johannes Schindelin
Cc: git, Junio C Hamano, Patrick Steinhardt, Elijah Newren, John Cai,
Derrick Stolee, Phillip Wood, Calvin Wan, Toon Claes
In-Reply-To: <d07487c2-4ee6-3ffa-014d-418793a5a584@gmx.de>
Hi Dscho,
On Thu, Sep 7, 2023 at 1:04 PM Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
>
> Hi Christian,
>
> hope you had a restful vacation!
Yes, thanks! I hope you had a good summer too.
> I left a bit of feedback and think that once my concerns are addressed, a
> v5 will be ready for `next`.
Thanks for your review!
I think I have addressed most (if not all) of your concerns in version 5.
^ permalink raw reply
* Re: [RFC] Define "precious" attribute and support it in `git clean`
From: Kristoffer Haugsbakk @ 2023-10-10 13:38 UTC (permalink / raw)
To: Sebastian Thiel; +Cc: Josh Triplett, git
In-Reply-To: <79901E6C-9839-4AB2-9360-9EBCA1AAE549@icloud.com>
Hi Sebastian
On Tue, Oct 10, 2023, at 14:37, Sebastian Thiel wrote:
> This highlights precious files by calling them out, but doesn't change the
> behaviour of existing flags. Instead, the new flag `-p` is added which lets
> `git clean` spare precious files.
Why can't `clean` preserve precious files by default? And then delete them
as well with something like `--no-keep-precious`? Is there some backwards
compatibility concern?
^ permalink raw reply
* Re: [PATCH v3 11/15] replay: use standard revision ranges
From: Dragan Simic @ 2023-10-10 14:02 UTC (permalink / raw)
To: Christian Couder
Cc: Toon Claes, git, Junio C Hamano, Patrick Steinhardt,
Johannes Schindelin, Elijah Newren, John Cai, Derrick Stolee,
Phillip Wood, Felipe Contreras, Calvin Wan, Christian Couder
In-Reply-To: <CAP8UFD213MwZmBuK0At5r0O8tHYDnxRTkw9AHak9RwDBdVVq_A@mail.gmail.com>
On 2023-10-10 14:44, Christian Couder wrote:
> On Thu, Sep 7, 2023 at 11:02 PM Dragan Simic <dsimic@manjaro.org>
> wrote:
>>
>> On 2023-09-07 10:32, Christian Couder wrote:
>> > On Thu, Jun 22, 2023 at 12:03 PM Toon Claes <toon@iotcl.com> wrote:
>> >>
>> >> Christian Couder <christian.couder@gmail.com> writes:
>> >>
>> >> > +DESCRIPTION
>> >> > +-----------
>> >> > +
>> >> > +Takes a range of commits, and replays them onto a new location. Does
>> >> > +not touch the working tree or index, and does not update any
>> >> > +references. However, the output of this command is meant to be used
>> >>
>> >> Small suggestion here:
>> >>
>> >> Takes a range of commits, and replays them onto a new location. Does
>> >> neither touch the working tree nor index, and does not update any
>> >> references.
>> >
>> > I am not a native speaker, so I am not sure what's best here. I find
>> > your suggestion a bit less clear though, so until a native speaker
>> > agrees with it or maybe finds something even better, I prefer to leave
>> > it as-is.
>>
>> I'm also not a native English speaker, but I spent about 2.5 years
>> contributing a whole lot to English Wikipedia, so I'd dare to say I've
>> honed my English skills rather well. Thus, here's my take on this:
>>
>> Takes a range of commits and replays them onto a new location.
>> Leaves the working tree and the index untouched, and updates no
>> references. The output of this command is to be used...
>>
>> This is written in a concise yet slightly imperative way, which should
>> be suitable for the purpose. I hope you agree.
>
> I agree and I like it, so I have changed it to the above in version 5
> I just sent.
Great, thanks.
^ permalink raw reply
* Re: [RFC] Define "precious" attribute and support it in `git clean`
From: Josh Triplett @ 2023-10-10 14:10 UTC (permalink / raw)
To: Kristoffer Haugsbakk; +Cc: Sebastian Thiel, git
In-Reply-To: <98387b86-1732-42bc-9ac5-d64a6617b2db@app.fastmail.com>
On Tue, Oct 10, 2023 at 03:38:51PM +0200, Kristoffer Haugsbakk wrote:
> Hi Sebastian
>
> On Tue, Oct 10, 2023, at 14:37, Sebastian Thiel wrote:
> > This highlights precious files by calling them out, but doesn't change the
> > behaviour of existing flags. Instead, the new flag `-p` is added which lets
> > `git clean` spare precious files.
>
> Why can't `clean` preserve precious files by default? And then delete them
> as well with something like `--no-keep-precious`? Is there some backwards
> compatibility concern?
While I'd love for it to default to that and require an extra option to
clean away precious files, I'd expect that that would break people's
workflows and finger memory. If someone expects `git clean -x -d -f` to
clean away everything, including `.config`, and then it leaves some
files in place, that seems likely to cause problems. (Leaving aside that
it might break scripted workflows.)
It seems safer to keep the existing behavior for existing options, and
add a new option for "remove everything except precious files".
^ permalink raw reply
* Re: [silly] worldview documents?
From: Emily Shaffer @ 2023-10-10 14:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Elijah Newren, git
In-Reply-To: <xmqqo7h7urgd.fsf@gitster.g>
On Mon, Oct 9, 2023 at 7:44 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Elijah Newren <newren@gmail.com> writes:
>
> >> [Footnote]
> >> ...
> >
> > Thanks for writing this up. In the past, I didn't know how to put
> > into words why I didn't particularly care for this mode. You explain
> > it rather well.
>
> I am glad it helped non-zero number of people.
>
> It probably owes big to my failing, but because we strongly view it
> a virtue not to be opinionated, we have many discrete tools and
> features that can be used in combination to support a workflow well,
> even when some of these tools and features are not useful for a
> different workflow. It indeed is a good thing to be flexible and to
> support different workflows well, and we tend not to single out a
> workflow among many and advocate it, but because our documentation
> lacks description of major possible workflows, what their underlying
> philosophies and their strengths are, how some of our tools and
> features support them, and why some others are not good fit. Being
> given a toolbox with too many tools without being taught how they
> are to be used together and for what purpose may be a fun puzzle to
> figure out for tinkerers, but when you have a problem to solve and
> tinkering is not your main focus, which is true for most people, it
> is not fun.
>
> In short, in pursuit of not to be opinionated, we fail to give the
> readers best current practices. The first place to start rectifying
> it might be to have some write-ups for various major workflows and
> the worldview behind them.
I completely agree with you. The #1 conversation I have with friends
who are new to Git is figuring out which of the major workflows they
should use, and what are the drawbacks and benefits. And how to
diagnose based on the existing commit history, review tool in use, etc
which one is used by their peers. Having this documented somewhere -
maybe in Pro Git book, maybe in manpage - would be hugely useful. I
could envision a diagram of a sample commit history, and "if it
already looks like this or you want it to look like this, your
workflow should be blah" and finally a list of some drawbacks,
benefits, and warnings about what to avoid in that workflow. Plus,
perhaps, many shout-outs to `git reflog` for fixing when you
misunderstood and made a mess. (that's the #2 conversation I have :) )
> The importance given to first-parenthood
> offers two quite different worldviews that affects the choice of
> tools (e.g. "merge --no-ff", "checkout origin/master && merge mine
> && branch -f mine" aka "reverse merge").
>
> I suspect that this also relates to your "would --cc be totally
> unnecessary now we have --remerge-diff?" as well. What kind of
> conflicts are interesting highly depends on what you are looking
> for, which in turn is influenced by the workflow employed by the
> project and what role you are playing in it.
^ permalink raw reply
* Re: [PATCH v3] merge-ort: initialize repo in index state
From: John Cai @ 2023-10-10 15:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: John Cai via GitGitGadget, git, Elijah Newren
In-Reply-To: <xmqq4jizxyla.fsf@gitster.g>
Hi Junio
On 9 Oct 2023, at 17:41, Junio C Hamano wrote:
> "John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> Fix this by initializing the repository in the index state.
>>
>> Changes since V2:
>>
>> * fixed test by using printf instead of echo
>
> Much better than using unportable \n with echo.
>
>> -+ echo "foo\nbar\nbaz" >expect &&
>> ++ printf "foo\nbar\nbaz\n" >expect &&
>
> But if we are using printf, it would be easier to read lines
> separately, which would look more like
>
> printf "%s\n" foo bar baz >expect
>
> And we have
>
> test_write_lines foo bar baz >expect
>
> to make it even more discoverable.
wasn't aware of test_write_lines, thanks.
>
>> + git cat-file -p "$tree:file1" >actual &&
>> + test_cmp expect actual
>> + )
>>
>>
>> merge-ort.c | 1 +
>> t/t4300-merge-tree.sh | 27 +++++++++++++++++++++++++++
>> 2 files changed, 28 insertions(+)
>>
>> diff --git a/merge-ort.c b/merge-ort.c
>> index 7857ce9fbd1..36537256613 100644
>> --- a/merge-ort.c
>> +++ b/merge-ort.c
>> @@ -1902,6 +1902,7 @@ static void initialize_attr_index(struct merge_options *opt)
>> struct index_state *attr_index = &opt->priv->attr_index;
>> struct cache_entry *ce;
>>
>> + attr_index->repo = opt->repo;
>> attr_index->initialized = 1;
>>
>> if (!opt->renormalize)
>> diff --git a/t/t4300-merge-tree.sh b/t/t4300-merge-tree.sh
>> index 57c4f26e461..c3a03e54187 100755
>> --- a/t/t4300-merge-tree.sh
>> +++ b/t/t4300-merge-tree.sh
>> @@ -86,6 +86,33 @@ EXPECTED
>> test_cmp expected actual
>> '
>>
>> +test_expect_success '3-way merge with --attr-source' '
>> + test_when_finished rm -rf 3-way &&
>> + git init 3-way &&
>> + (
>> + cd 3-way &&
>> + test_commit initial file1 foo &&
>> + base=$(git rev-parse HEAD) &&
>> + git checkout -b brancha &&
>> + echo bar >>file1 &&
>> + git commit -am "adding bar" &&
>> + source=$(git rev-parse HEAD) &&
>> + git checkout @{-1} &&
>> + git checkout -b branchb &&
>> + echo baz >>file1 &&
>> + git commit -am "adding baz" &&
>> + merge=$(git rev-parse HEAD) &&
>> + git checkout -b gitattributes &&
>> + test_commit "gitattributes" .gitattributes "file1 merge=union" &&
>
> OK, the branch "gitattributes" will be used to drive merge of file1
> using the union merge to avoid conflicting.
>
>> + git checkout @{-1} &&
>
> But such attribute will only be available in that branch, not in the
> checked out working tree. And then
>
>> + tree=$(git --attr-source=gitattributes merge-tree --write-tree \
>> + --merge-base "$base" --end-of-options "$source" "$merge") &&
>
> we use the gitattributes branch as the tree-ish to take the
> attribute information from. Makes sense.
>
>> + printf "foo\nbar\nbaz\n" >expect &&
>
> I'll squash in the "test_write_lines" change while queuing.
thank you!
John
>
>> + git cat-file -p "$tree:file1" >actual &&
>> + test_cmp expect actual
>> + )
>> +'
>> +
>> test_expect_success 'file change A, B (same)' '
>> git reset --hard initial &&
>> test_commit "change-a-b-same-A" "initial-file" "AAA" &&
>>
>> base-commit: 493f4622739e9b64f24b465b21aa85870dd9dc09
>
> Thanks. Looking good.
^ permalink raw reply
* Re: [PATCH 1/1] [OUTREACHY] Fixed add.c file to conform to guidelines when using die() listed in issue #635
From: Naomi Ibe @ 2023-10-10 15:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqlecbzl5e.fsf@gitster.g>
Thank you very much! I'd definitely make those changes on my next patch.
Should I begin work on version 2 or should I still wait for additional
input on the version 1?
On Mon, Oct 9, 2023 at 7:49 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Naomi Ibe <naomi.ibeh69@gmail.com> writes:
>
> > Subject: Re: [PATCH 1/1] [OUTREACHY] Fixed add.c file to conform to guidelines when using d
>
> Subject: [OUTREACHY][PATCH 1/1] builtin/add.c: clean up die() messages
>
> > From: Naomi <naomi.ibeh69@gmail.com>
>
> The name and address on this line come from your commit object,
> which in turn would have came from your configuration. You have
>
> [user]
> name = Naomi
> email = naomi.ibeh69@gmail.com
>
> somewhere in your configuration file, perhaps in $HOME/.gitconfig or
> somewhere. When contributiong to this project, you want that "name"
> line to also include your family name, as it should match what you
> write on your Signed-off-by: line. A focused way to do so without
> affecting your author identity for other projects is to add
>
> [user]
> name = Naomi Ibe
>
> in .git/config of the repository that you use to contribute to this
> project, e.g.,
>
> $ cd ... to the working tree of your clone of git you work in ...
> $ git config user.name "Naomi Ibe"
>
> The space above your Sign off is to fill the details you omitted on
> the title of the message (which would say something about "conform
> to guidelines" or "clean up" without mentioning what rule the
> original violates), making the whole thing something like:
>
> builtin/add.c: clean up die() messages
>
> As described in the CodingGuidelines document, a single line
> message given to die() and its friends should not capitalize its
> first word, and should not add full-stop at the end.
>
> Signed-off-by: ...
>
> > Signed-off-by: Naomi Ibe <naomi.ibeh69@gmail.com>
> > ---
> > builtin/add.c | 10 +++++-----
> > 1 file changed, 5 insertions(+), 5 deletions(-)
>
> Thanks. Otherwise the patch looks good.
>
> >
> > diff --git a/builtin/add.c b/builtin/add.c
> > index c27254a5cd..5126d2ede3 100644
> > --- a/builtin/add.c
> > +++ b/builtin/add.c
> > @@ -182,7 +182,7 @@ static int edit_patch(int argc, const char **argv, const char *prefix)
> > git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
> >
> > if (repo_read_index(the_repository) < 0)
> > - die(_("Could not read the index"));
> > + die(_("could not read the index"));
> >
> > repo_init_revisions(the_repository, &rev, prefix);
> > rev.diffopt.context = 7;
> > @@ -200,15 +200,15 @@ static int edit_patch(int argc, const char **argv, const char *prefix)
> > die(_("editing patch failed"));
> >
> > if (stat(file, &st))
> > - die_errno(_("Could not stat '%s'"), file);
> > + die_errno(_("could not stat '%s'"), file);
> > if (!st.st_size)
> > - die(_("Empty patch. Aborted."));
> > + die(_("empty patch. aborted"));
> >
> > child.git_cmd = 1;
> > strvec_pushl(&child.args, "apply", "--recount", "--cached", file,
> > NULL);
> > if (run_command(&child))
> > - die(_("Could not apply '%s'"), file);
> > + die(_("could not apply '%s'"), file);
> >
> > unlink(file);
> > free(file);
> > @@ -568,7 +568,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
> > finish:
> > if (write_locked_index(&the_index, &lock_file,
> > COMMIT_LOCK | SKIP_IF_UNCHANGED))
> > - die(_("Unable to write new index file"));
> > + die(_("unable to write new index file"));
> >
> > dir_clear(&dir);
> > clear_pathspec(&pathspec);
^ permalink raw reply
* Re: [PATCH v4 2/4] wrapper: reduce scope of remove_or_warn()
From: Junio C Hamano @ 2023-10-10 16:13 UTC (permalink / raw)
To: phillip.wood123; +Cc: Jonathan Tan, git, Calvin Wan
In-Reply-To: <066b3162-6a81-45d7-b164-17b74e6c92dc@gmail.com>
phillip.wood123@gmail.com writes:
> Hi Jonathan
>
> On 29/09/2023 22:20, Jonathan Tan wrote:
>> From: Calvin Wan <calvinwan@google.com>
>> remove_or_warn() is only used by entry.c and apply.c, but it is
>> currently declared and defined in wrapper.{h,c}, so it has a scope much
>> greater than it needs. This needlessly large scope also causes wrapper.c
>> to need to include object.h, when this file is largely unconcerned with
>> Git objects.
>> Move remove_or_warn() to entry.{h,c}. The file apply.c still has
>> access
>> to it, since it already includes entry.h for another reason.
>
> This looks good. On a related note wrapper.c includes repository.h but
> does use anything declared in that header.
>
> Best Wishes
>
> Phillip
Thanks for a review. I just checked 'master', 'next', and 'seen'
and in all '#include <repository.h>' can safely be dropped from
there, it seems. It may be too trivial even for a microproject,
but nevertheless a nice clean-up.
^ permalink raw reply
* Re: [PATCH v4 0/4] Preliminary patches before git-std-lib
From: Jonathan Tan @ 2023-10-10 16:21 UTC (permalink / raw)
To: phillip.wood123; +Cc: Jonathan Tan, git, Calvin Wan, Junio C Hamano
In-Reply-To: <4670774d-a899-492c-9b36-98ee243c8d4d@gmail.com>
phillip.wood123@gmail.com writes:
> Hi Jonathan
>
> On 29/09/2023 22:20, Jonathan Tan wrote:
> > Calvin will be away for a few weeks and I'll be handling the git-std-lib
> > effort in the meantime. My goals will be:
> >
> > - Get the preliminary patches in Calvin's patch set (patches 1-4) merged
> > first.
> >
> > - Updating patches 5-6 based on reviewer feedback (including my
> > feedback). I have several aims including reducing or eliminating the
> > need for the GIT_STD_LIB preprocessor variable, and making stubs a test-
> > only concern (I think Phillip has some similar ideas [1] but I haven't
> > looked at their repo on GitHub yet).
>
> It sounds like we're thinking along similar lines, do feel free get in
> touch on or off the list if you want to ask anything about those patches
> I pushed to github.
Thanks. I'm updating patches 5-6 now and basing on your work, in fact.
> > [1] https://lore.kernel.org/git/98f3edcf-7f37-45ff-abd2-c0038d4e0589@gmail.com/
> >
> > This patch set is in service of the first goal. Because the libification
> > patches are no longer included in this patch set, I have rewritten the
> > commit messages to justify the patches in terms of code organization.
> > There are no changes in the code itself. Also, I have retained Calvin's
> > name as the author.
>
> I agree it makes sense to get the preliminary patches merged on their
> own. I think the argument that they reduce the scope of includes is a
> reasonable justification on its own. I've left a couple of comments but
> they're looking pretty good.
>
> Best Wishes
>
> Phillip
Thanks.
^ permalink raw reply
* Re: [RFC] Define "precious" attribute and support it in `git clean`
From: Junio C Hamano @ 2023-10-10 17:02 UTC (permalink / raw)
To: Sebastian Thiel; +Cc: git, Josh Triplett
In-Reply-To: <79901E6C-9839-4AB2-9360-9EBCA1AAE549@icloud.com>
Sebastian Thiel <sebastian.thiel@icloud.com> writes:
> I'd like to propose adding a new standard gitattribute "precious".
;-).
Over the years, I've seen many times scenarios that would have been
helped if we had not just "tracked? ignored? unignored?" but also
the fourth kind [*]. The word "ignored" (or "excluded") has always
meant "not tracked, not to be tracked, and expendable" to Git, and
"ignored but unexpendable" class was missing. I even used the term
"precious" myself in those discussions. At the concept level, I
support the effort 100%, but as always, the devil will be in the
details.
Scenarios that people wished for "precious" traditionally have been
* You are working on 'master'. You have in your .gitignore or
.git/info/exclude a line to ignore path A, and have random
scribbles in a throw-away file there. There is another branch
'seen', where they added some tracked contents at path A/B. You
do "git checkout seen" and your file A that is an expendable file,
because it is listed as ignored in .git/info/exclude, is removed
to make room for creating A/B.
* Similar situation, but this time, 'seen' branch added a tracked
contents at path A. Again, "git checkout seen" will discard the
expendable file A and replace it with tracked contents.
* Instead of "git checkout", you decide to merge the branch 'seen'
to the checkout of 'master', where you have an ignored path A.
Because merging 'seen' would need to bring the tracked contents
of either A/B (in the first scenario above) or A (in the second
scenario), your "expendable" A will be removed to make room.
In previous discussions, nobody was disturbed that "git clean" was
unaware of the "precious" class, but if we were to have the
"precious" class in addition to "ignored" aka "expendable", I would
not oppose to teach "git clean" about it, too.
There was an early and rough design draft there in
https://lore.kernel.org/git/7vipsnar23.fsf@alter.siamese.dyndns.org/
which probably is worth a read, too.
Even though I referred to the precious _attribute_ in some of these
discussions, between the attribute mechanism and the ignore
mechanism, I am actually leaning toward suggesting to extend the
exclude/ignore mechanism to introduce the "precious" class. That
way, we can avoid possible snafu arising from marking a path in
.gitignore as ignored, and in .gitattrbutes as precious, and have to
figure out how these two settings are to work together.
In any case, the "precious" paths are expected to be small minority
of what people never want to "git add" or "git commit", so coming up
with a special syntax to be used in .gitignore, even if that special
syntax is ugly and cumbersome to type, would be perfectly OK.
[Reference]
* https://lore.kernel.org/git/7viptp9jos.fsf@alter.siamese.dyndns.org/
* https://lore.kernel.org/git/xmqqva534vnb.fsf@gitster-ct.c.googlers.com/
^ permalink raw reply
* Re: [RFC] Define "precious" attribute and support it in `git clean`
From: Junio C Hamano @ 2023-10-10 17:07 UTC (permalink / raw)
To: Josh Triplett; +Cc: Kristoffer Haugsbakk, Sebastian Thiel, git
In-Reply-To: <ZSVbUSRUQlNy0bj-@localhost>
Josh Triplett <josh@joshtriplett.org> writes:
> While I'd love for it to default to that and require an extra option to
> clean away precious files, I'd expect that that would break people's
> workflows and finger memory. If someone expects `git clean -x -d -f` to
> clean away everything, including `.config`, and then it leaves some
> files in place, that seems likely to cause problems. (Leaving aside that
> it might break scripted workflows.)
I thought the point of introducing the new "precious" class of
paths, in addition to the current "tracked", "ignored, untracked,
and expendable", "not ignored and untracked", is so that people can
do "git clean -x -d -f" and expect the ".config" that is marked as
"precious" to stay. Before their Git learned the precious class, if
they marked ".config" as "ignored, untracked, and expendable", then
such an invocation of "clean" would have removed it, but if they add
it to the new "precious" class, their expectation ought to be that
precious ones are not removed, no? Otherwise I am not quite sure
what the point of adding such a new protection is.
^ permalink raw reply
* Re: [PATCH 0/3] rev-list: add support for commits in `--missing`
From: Junio C Hamano @ 2023-10-10 17:09 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Karthik Nayak, git
In-Reply-To: <ZSTs3BUVtaI9QIoA@tanuki>
Patrick Steinhardt <ps@pks.im> writes:
> I had already reviewed the patches internally at GitLab, so for what
> it's worth please feel free to add my Reviewed-by.
Great. It seems that 'seen' with this series fails to pass the
tests, though.
https://github.com/git/git/actions/runs/6462854176/job/17545104753
Thanks.
>
> Patrick
>
>> Karthik Nayak (3):
>> revision: rename bit to `do_not_die_on_missing_objects`
>> rev-list: move `show_commit()` to the bottom
>> rev-list: add commit object support in `--missing` option
>>
>> builtin/reflog.c | 2 +-
>> builtin/rev-list.c | 93 +++++++++++++++++++------------------
>> list-objects.c | 2 +-
>> object.h | 2 +-
>> revision.c | 9 +++-
>> revision.h | 20 ++++----
>> t/t6022-rev-list-missing.sh | 74 +++++++++++++++++++++++++++++
>> 7 files changed, 145 insertions(+), 57 deletions(-)
>> create mode 100755 t/t6022-rev-list-missing.sh
>>
>> --
>> 2.41.0
>>
^ permalink raw reply
* Re: [PATCH v4 2/4] wrapper: reduce scope of remove_or_warn()
From: Jonathan Tan @ 2023-10-10 17:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jonathan Tan, phillip.wood123, git, Calvin Wan
In-Reply-To: <xmqqjzruv4k1.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> phillip.wood123@gmail.com writes:
>
> > Hi Jonathan
> >
> > On 29/09/2023 22:20, Jonathan Tan wrote:
> >> From: Calvin Wan <calvinwan@google.com>
> >> remove_or_warn() is only used by entry.c and apply.c, but it is
> >> currently declared and defined in wrapper.{h,c}, so it has a scope much
> >> greater than it needs. This needlessly large scope also causes wrapper.c
> >> to need to include object.h, when this file is largely unconcerned with
> >> Git objects.
> >> Move remove_or_warn() to entry.{h,c}. The file apply.c still has
> >> access
> >> to it, since it already includes entry.h for another reason.
> >
> > This looks good. On a related note wrapper.c includes repository.h but
> > does use anything declared in that header.
> >
> > Best Wishes
> >
> > Phillip
>
> Thanks for a review. I just checked 'master', 'next', and 'seen'
> and in all '#include <repository.h>' can safely be dropped from
> there, it seems. It may be too trivial even for a microproject,
> but nevertheless a nice clean-up.
Ah, Calvin fixed this in one of the subsequent patches, but I'll put it
into its own patch in my updated version.
^ permalink raw reply
* Re: [PATCH v4 4/4] parse: separate out parsing functions from config.h
From: Jonathan Tan @ 2023-10-10 17:43 UTC (permalink / raw)
To: phillip.wood123; +Cc: Jonathan Tan, git, Calvin Wan, Junio C Hamano
In-Reply-To: <1de0a6f3-e223-4e84-a6d2-51d9b51a02f6@gmail.com>
phillip.wood123@gmail.com writes:
> Hi Jonathan
>
> On 29/09/2023 22:20, Jonathan Tan wrote:
> > diff --git a/parse.h b/parse.h
> > new file mode 100644
> > index 0000000000..07d2193d69
> > --- /dev/null
> > +++ b/parse.h
> > @@ -0,0 +1,20 @@
> > +#ifndef PARSE_H
> > +#define PARSE_H
> > +
> > +int git_parse_signed(const char *value, intmax_t *ret, intmax_t max);
>
> Previously this function was private to config.c, now it needs to be
> public because it is still called by
> git_config_get_expiry_date_in_days(). As this is essentially an internal
> helper for git_parse_int() and friends it is a bit unfortunate that it
> is now public. Perhaps we should change
> git_config_get_expiry_date_in_days() to call git_parse_int() instead.
> Then we can keep git_parse_signed() and git_parse_unsigned() private to
> parse.c.
It could be argued also that it fits in with the rest of
the parsing functions - this one parses intmax, and we have
others of various signedness and size. I'm open to changing
git_config_get_expiry_date_in_days() too, though...we probably don't
need so many days.
> > +/**
> > + * Same as `git_config_bool`, except that it returns -1 on error rather
> > + * than dying.
> > + */
> > +int git_parse_maybe_bool(const char *);
> > +int git_parse_maybe_bool_text(const char *value);
>
> This used to be private to config.c and now has callers in parse.c and
> config.c. We should make it clear that non-config code is likely to want
> git_parse_maybe_bool() rather than this function.
>
> Best Wishes
>
> Phillip
The difference between these 2 functions here is that bool_text supports
only the textual forms (used, for example, in git_config_bool_or_int()
which accepts both boolean strings and integers), which might be useful
elsewhere too. But it could be better documented, yes.
Looking at "What's Cooking", this series is about to be merged to
master. We could hold off merging that, but I think we don't need to
- it could be argued that git_parse_maybe_bool_text() could be better
documented, but even if we wrote it from scratch, I would probably put
the extra documentation in its own patch anyway (so one patch for moving
the code, and another for adding documentation).
^ permalink raw reply
* Re: [PATCH] doc/git-worktree: mention "refs/rewritten" as per-worktree refs
From: Eric Sunshine @ 2023-10-10 17:49 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Johannes Schindelin
In-Reply-To: <985ac850eb6e60ae76601acc8bfbcd56f99348b4.1696935657.git.ps@pks.im>
On Tue, Oct 10, 2023 at 7:01 AM Patrick Steinhardt <ps@pks.im> wrote:
> Some references are special in the context of worktrees as they are
> considered to be per-worktree instead of shared across all of the
> worktrees. Most importantly, this includes "refs/worktree/" that have
> explicitly been designed such that users can create per-woorktree refs.
s/woorktree/worktree/
> But there are also special references that have an associated meaning
> like "refs/bisect/", which is used to track state of git-bisect(1).
>
> These special per-worktree references are documented in git-worktree(1),
> but one instance is missing. In a9be29c9817 (sequencer: make refs
> generated by the `label` command worktree-local, 2018-04-25), we have
> converted "refs/rewritten/" to be a per-worktree reference as well.
> These references are used by our sequencer infrastructure to generate
> labels for rebased commits. So in order to allow for multiple concurrent
> rebases to happen in different worktrees, these references need to be
> tracked per worktree.
>
> We forgot to update our documentation to mention these new per-worktree
> references, which is fixed by this patch.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
> @@ -286,7 +286,8 @@ rules and how to access refs of one worktree from another.
> In general, all pseudo refs are per-worktree and all refs starting with
> `refs/` are shared. Pseudo refs are ones like `HEAD` which are directly
> under `$GIT_DIR` instead of inside `$GIT_DIR/refs`. There are exceptions,
> -however: refs inside `refs/bisect` and `refs/worktree` are not shared.
> +however: refs inside `refs/bisect`, `refs/worktree` and `refs/rewritten` are
> +not shared.
To simplify future maintenance, eventually we might want to turn this
into a bulleted list, but that doesn't necessarily have to be done by
this patch, which is fine as-is.
> @@ -363,8 +364,8 @@ linked worktree `git rev-parse --git-path HEAD` returns
> `$GIT_COMMON_DIR` and returns `/path/main/.git/refs/heads/master`,
> -since refs are shared across all worktrees, except `refs/bisect` and
> -`refs/worktree`.
> +since refs are shared across all worktrees, except `refs/bisect`,
> +`refs/worktree` and `refs/rewritten`.
Ditto.
^ permalink raw reply
* Re: [PATCH v4 4/4] parse: separate out parsing functions from config.h
From: Phillip Wood @ 2023-10-10 17:58 UTC (permalink / raw)
To: Jonathan Tan; +Cc: git, Calvin Wan, Junio C Hamano
In-Reply-To: <20231010174348.2150150-1-jonathantanmy@google.com>
On 10/10/2023 18:43, Jonathan Tan wrote:
> phillip.wood123@gmail.com writes:
>> Hi Jonathan
>>
>> On 29/09/2023 22:20, Jonathan Tan wrote:
>>> diff --git a/parse.h b/parse.h
>>> new file mode 100644
>>> index 0000000000..07d2193d69
>>> --- /dev/null
>>> +++ b/parse.h
>>> @@ -0,0 +1,20 @@
>>> +#ifndef PARSE_H
>>> +#define PARSE_H
>>> +
>>> +int git_parse_signed(const char *value, intmax_t *ret, intmax_t max);
>>
>> Previously this function was private to config.c, now it needs to be
>> public because it is still called by
>> git_config_get_expiry_date_in_days(). As this is essentially an internal
>> helper for git_parse_int() and friends it is a bit unfortunate that it
>> is now public. Perhaps we should change
>> git_config_get_expiry_date_in_days() to call git_parse_int() instead.
>> Then we can keep git_parse_signed() and git_parse_unsigned() private to
>> parse.c.
>
> It could be argued also that it fits in with the rest of
> the parsing functions - this one parses intmax, and we have
> others of various signedness and size.
This one differs from the others because it expects the caller to pass a
maximum value, the intmax_t equivalent to git_parse_int() would be
int git_parse_intmax(const char*, intmax_t*);
We now expose git_parse_int64() which covers a similar case.
> I'm open to changing
> git_config_get_expiry_date_in_days() too, though...we probably don't
> need so many days.
Indeed, the existing code passes maximum_signed_value_of_type(int) as
the third argument to limit it to INT_MAX already.
>>> +/**
>>> + * Same as `git_config_bool`, except that it returns -1 on error rather
>>> + * than dying.
>>> + */
>>> +int git_parse_maybe_bool(const char *);
>>> +int git_parse_maybe_bool_text(const char *value);
>>
>> This used to be private to config.c and now has callers in parse.c and
>> config.c. We should make it clear that non-config code is likely to want
>> git_parse_maybe_bool() rather than this function.
>>
>> Best Wishes
>>
>> Phillip
>
> The difference between these 2 functions here is that bool_text supports
> only the textual forms (used, for example, in git_config_bool_or_int()
> which accepts both boolean strings and integers), which might be useful
> elsewhere too. But it could be better documented, yes.
>
> Looking at "What's Cooking", this series is about to be merged to
> master. We could hold off merging that, but I think we don't need to
> - it could be argued that git_parse_maybe_bool_text() could be better
> documented, but even if we wrote it from scratch, I would probably put
> the extra documentation in its own patch anyway (so one patch for moving
> the code, and another for adding documentation).
I agree it's not worth re-rolling just to add some documentation here.
Best Wishes
Phillip
^ permalink raw reply
* Re: [RFC] Define "precious" attribute and support it in `git clean`
From: Kristoffer Haugsbakk @ 2023-10-10 19:10 UTC (permalink / raw)
To: Josh Triplett; +Cc: Sebastian Thiel, git
In-Reply-To: <ZSVbUSRUQlNy0bj-@localhost>
Hi Josh
On Tue, Oct 10, 2023, at 16:10, Josh Triplett wrote:
> > [snip]
>
> While I'd love for it to default to that and require an extra option to
> clean away precious files, I'd expect that that would break people's
> workflows and finger memory. If someone expects `git clean -x -d -f` to
> clean away everything, including `.config`, and then it leaves some
> files in place, that seems likely to cause problems. (Leaving aside that
> it might break scripted workflows.)
>
> It seems safer to keep the existing behavior for existing options, and
> add a new option for "remove everything except precious files".
What's a scenario where it breaks? I'm guessing:
1. Someone clones a project
2. That project has precious files marked via `.gitattributes`
3. They later do a `clean`
4. The precious files are left alone even though they expected them to be
deleted; they don't check what `clean` did (it deletes everything
untracked (they expect) so nothing to check)
5. This hurts them somehow
It seems that the only files that should be deleted with expediency are
secrets. But then why or how would:
1. The project mark such files as precious
2. The user introduces these files (they are precious hence they were not
part of the clone)
3. They are never deleted
This sounds unlikely to me. And if it was some kind of malignant vector
then all would be vulnerable to it (not just legacy scripts/legacy hands).
What am I missing?
--
Kristoffer
^ permalink raw reply
* Re: [PATCH 1/1] [OUTREACHY] Fixed add.c file to conform to guidelines when using die() listed in issue #635
From: Christian Couder @ 2023-10-10 19:22 UTC (permalink / raw)
To: Naomi Ibe; +Cc: Junio C Hamano, git
In-Reply-To: <CACS=G2yUGGJwD05KOFZK+AV3TSNDvDEfC=pFRsLwKX_-dgt+gA@mail.gmail.com>
On Tue, Oct 10, 2023 at 5:20 PM Naomi Ibe <naomi.ibeh69@gmail.com> wrote:
>
> Thank you very much! I'd definitely make those changes on my next patch.
> Should I begin work on version 2 or should I still wait for additional
> input on the version 1?
I think you can work on version 2. Also please reply inline instead of
top-posting (see https://en.wikipedia.org/wiki/Posting_style).
^ permalink raw reply
* Re: [RFC PATCH] Not computing changed path filter for root commits
From: Taylor Blau @ 2023-10-10 19:47 UTC (permalink / raw)
To: Jonathan Tan; +Cc: SZEDER Gábor, git
In-Reply-To: <20231009205925.1915096-1-jonathantanmy@google.com>
On Mon, Oct 09, 2023 at 01:59:25PM -0700, Jonathan Tan wrote:
> Taylor Blau <me@ttaylorr.com> writes:
> > This only happens when we return REV_TREE_NEW from a call to
> > `rev_compare_tree(revs, p, commit, nth_parent)`. But we'll only get
> > REV_TREE_NEW back if
> >
> > repo_get_commit_tree(the_repository, p);
> >
> > returns NULL. But when we call rev_same_tree_as_empty(revs, p) in the
> > REV_TREE_NEW case, we return early as follows:
> >
> > struct tree *t1 = repo_get_commit_tree(revs, p);
> > if (!t1)
> > return 0;
> >
> > So we won't even consult the Bloom filter in that case, since t1 is NULL
> > for the same reason as what caused rev_compare_tree() to return
> > REV_TREE_NEW in the first place.
> >
> > I am still dumbfounded by how we would ever get REV_TREE_NEW in the
> > first place, but if we did, I think we would be OK here.
> >
> > Thanks,
> > Taylor
>
> Ah, good point. Your patch in
> https://lore.kernel.org/git/ZQnmTXUO94%2FQy8mq@nand.local/ looks good to
> me, then.
Oops, I made a mistake in the quoted portion, which is that we could get
REV_TREE_NEW if the tree-diff itself only adds files. This is the
non-trivial case that we get when t1 is non-NULL, and we end up calling
`diff_tree_oid()` which sets the static `tree_difference` variable.
So I think adding an nth_parent field (like you originally
suggested[^1]) makes sense.
Thanks,
Taylor
[^1]: Thanks for being patient with me ;-).
^ permalink raw reply
* [PATCH v3 0/2] attr: add attr.tree config
From: John Cai via GitGitGadget @ 2023-10-10 19:49 UTC (permalink / raw)
To: git; +Cc: Jeff King, Jonathan Tan, John Cai
In-Reply-To: <pull.1577.v2.git.git.1696443502.gitgitgadget@gmail.com>
44451a2e5e (attr: teach "--attr-source=" global option to "git", 2023-05-06)
provided the ability to pass in a treeish as the attr source. When a
revision does not resolve to a valid tree is passed, Git will die. At
GitLab, we server repositories as bare repos and would like to always read
attributes from the default branch, so we'd like to pass in HEAD as the
treeish to read gitattributes from on every command. In this context we
would not want Git to die if HEAD is unborn, like in the case of empty
repositories.
Instead of modifying the default behavior of --attr-source, create a new
config attr.tree with which an admin can configure a ref for all commands to
read gitattributes from. Also make the default tree to read from HEAD on
bare repositories.
Changes since v2:
* relax the restrictions around attr.tree so that if it does not resolve to
a valid treeish, ignore it.
* add a commit to default to HEAD in bare repositories
Changes since v1:
* Added a commit to add attr.tree config
John Cai (2):
attr: read attributes from HEAD when bare repo
attr: add attr.tree for setting the treeish to read attributes from
Documentation/config.txt | 2 +
Documentation/config/attr.txt | 5 +++
attr.c | 19 ++++++++-
attr.h | 2 +
config.c | 14 +++++++
t/t0003-attributes.sh | 76 +++++++++++++++++++++++++++++++++++
t/t5001-archive-attr.sh | 2 +-
7 files changed, 118 insertions(+), 2 deletions(-)
create mode 100644 Documentation/config/attr.txt
base-commit: 1fc548b2d6a3596f3e1c1f8b1930d8dbd1e30bf3
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1577%2Fjohn-cai%2Fjc%2Fconfig-attr-invalid-source-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1577/john-cai/jc/config-attr-invalid-source-v3
Pull-Request: https://github.com/git/git/pull/1577
Range-diff vs v2:
2: 52d9e180352 ! 1: cef206d47c7 attr: add attr.allowInvalidSource config to allow invalid revision
@@ Metadata
Author: John Cai <johncai86@gmail.com>
## Commit message ##
- attr: add attr.allowInvalidSource config to allow invalid revision
+ attr: read attributes from HEAD when bare repo
- The previous commit provided the ability to pass in a treeish as the
- attr source via the attr.tree config. The default behavior is that when
- a revision does not resolve to a valid tree is passed, Git will die.
+ The motivation for 44451a2e5e (attr: teach "--attr-source=<tree>" global
+ option to "git" , 2023-05-06), was to make it possible to use
+ gitattributes with bare repositories.
- When HEAD is unborn however, it does not point to a valid treeish,
- causing Git to die. In the context of serving repositories through bare
- repositories, we'd like to be able to set attr.tree to HEAD and allow
- cases where HEAD does not resolve to a valid tree to be treated as if
- there were no treeish provided.
-
- Add attr.allowInvalidSource that allows this.
+ To make it easier to read gitattributes in bare repositories however,
+ let's just make HEAD:.gitattributes the default. This is in line with
+ how mailmap works, 8c473cecfd (mailmap: default mailmap.blob in bare
+ repositories, 2012-12-13).
Signed-off-by: John Cai <johncai86@gmail.com>
- ## Documentation/config/attr.txt ##
-@@ Documentation/config/attr.txt: attr.tree:
- linkgit:gitattributes[5]. This is equivalent to setting the
- `GIT_ATTR_SOURCE` environment variable, or passing in --attr-source to
- the Git command.
-+
-+attr.allowInvalidSource::
-+ If `attr.tree` cannot resolve to a valid tree object, ignore
-+ `attr.tree` instead of erroring out, and fall back to looking for
-+ attributes in the default locations. Useful when passing `HEAD` into
-+ `attr-source` since it allows `HEAD` to point to an unborn branch in
-+ cases like an empty repository.
-
## attr.c ##
-@@ attr.c: void set_git_attr_source(const char *tree_object_name)
+@@ attr.c: static void collect_some_attrs(struct index_state *istate,
+ }
- static void compute_default_attr_source(struct object_id *attr_source)
+ static const char *default_attr_source_tree_object_name;
++static int ignore_bad_attr_tree;
+
+ void set_git_attr_source(const char *tree_object_name)
{
-+ int attr_source_from_config = 0;
-+
+@@ attr.c: static void compute_default_attr_source(struct object_id *attr_source)
if (!default_attr_source_tree_object_name)
default_attr_source_tree_object_name = getenv(GIT_ATTR_SOURCE_ENVIRONMENT);
- if (!default_attr_source_tree_object_name) {
- char *attr_tree;
-
-- if (!git_config_get_string("attr.tree", &attr_tree))
-+ if (!git_config_get_string("attr.tree", &attr_tree)) {
-+ attr_source_from_config = 1;
- default_attr_source_tree_object_name = attr_tree;
-+ }
- }
-
++ if (!default_attr_source_tree_object_name &&
++ startup_info->have_repository &&
++ is_bare_repository()) {
++ default_attr_source_tree_object_name = "HEAD";
++ ignore_bad_attr_tree = 1;
++ }
++
if (!default_attr_source_tree_object_name || !is_null_oid(attr_source))
return;
- if (repo_get_oid_treeish(the_repository, default_attr_source_tree_object_name, attr_source))
-- die(_("bad --attr-source or GIT_ATTR_SOURCE"));
-+ if (repo_get_oid_treeish(the_repository, default_attr_source_tree_object_name, attr_source)) {
-+ int allow_invalid_attr_source = 0;
-+
-+ git_config_get_bool("attr.allowinvalidsource", &allow_invalid_attr_source);
-+
-+ if (!(allow_invalid_attr_source && attr_source_from_config))
-+ die(_("bad --attr-source or GIT_ATTR_SOURCE"));
-+ }
++ if (repo_get_oid_treeish(the_repository,
++ default_attr_source_tree_object_name,
++ attr_source) && !ignore_bad_attr_tree)
+ die(_("bad --attr-source or GIT_ATTR_SOURCE"));
}
- static struct object_id *default_attr_source(void)
## t/t0003-attributes.sh ##
@@ t/t0003-attributes.sh: test_expect_success 'bare repository: check that .gitattribute is ignored' '
)
'
-+bad_attr_source_err="fatal: bad --attr-source or GIT_ATTR_SOURCE"
+
-+test_expect_success 'attr.allowInvalidSource when HEAD is unborn' '
-+ test_when_finished rm -rf empty &&
-+ echo $bad_attr_source_err >expect_err &&
-+ echo "f/path: test: unspecified" >expect &&
-+ git init empty &&
-+ test_must_fail git -C empty --attr-source=HEAD check-attr test -- f/path 2>err &&
-+ test_cmp expect_err err &&
-+ git -C empty -c attr.tree=HEAD -c attr.allowInvalidSource=true check-attr test -- f/path >actual 2>err &&
-+ test_must_be_empty err &&
-+ test_cmp expect actual
-+'
-+
-+test_expect_success 'attr.allowInvalidSource when --attr-source points to non-existing ref' '
-+ test_when_finished rm -rf empty &&
-+ echo $bad_attr_source_err >expect_err &&
-+ echo "f/path: test: unspecified" >expect &&
-+ git init empty &&
-+ test_must_fail git -C empty --attr-source=refs/does/not/exist check-attr test -- f/path 2>err &&
-+ test_cmp expect_err err &&
-+ git -C empty -c attr.tree=refs/does/not/exist -c attr.allowInvalidSource=true check-attr test -- f/path >actual 2>err &&
-+ test_must_be_empty err &&
-+ test_cmp expect actual
-+'
-+
-+test_expect_success 'bad attr source defaults to reading .gitattributes file' '
-+ test_when_finished rm -rf empty &&
-+ git init empty &&
-+ echo "f/path test=val" >empty/.gitattributes &&
++test_expect_success 'bare repo defaults to reading .gitattributes from HEAD' '
++ test_when_finished rm -rf test bare_with_gitattribute &&
++ git init test &&
++ (
++ cd test &&
++ test_commit gitattributes .gitattributes "f/path test=val"
++ ) &&
++ git clone --bare test bare_with_gitattribute &&
+ echo "f/path: test: val" >expect &&
-+ git -C empty -c attr.tree=HEAD -c attr.allowInvalidSource=true check-attr test -- f/path >actual 2>err &&
-+ test_must_be_empty err &&
++ git -C bare_with_gitattribute check-attr test -- f/path >actual &&
+ test_cmp expect actual
+'
-+
-+test_expect_success 'attr.allowInvalidSource has no effect on --attr-source' '
-+ test_when_finished rm -rf empty &&
-+ echo $bad_attr_source_err >expect_err &&
-+ echo "f/path: test: unspecified" >expect &&
-+ git init empty &&
-+ test_must_fail git -C empty -c attr.allowInvalidSource=true --attr-source=HEAD check-attr test -- f/path 2>err &&
-+ test_cmp expect_err err
-+'
+
test_expect_success 'bare repository: with --source' '
(
cd bare.git &&
+
+ ## t/t5001-archive-attr.sh ##
+@@ t/t5001-archive-attr.sh: test_expect_success 'git archive with worktree attributes, bare' '
+ '
+
+ test_expect_missing bare-worktree/ignored
+-test_expect_exists bare-worktree/ignored-by-tree
++test_expect_missing bare-worktree/ignored-by-tree
+ test_expect_exists bare-worktree/ignored-by-worktree
+
+ test_expect_success 'export-subst' '
1: 446bce03a96 ! 2: dadb822da99 attr: add attr.tree for setting the treeish to read attributes from
@@ Metadata
## Commit message ##
attr: add attr.tree for setting the treeish to read attributes from
- 44451a2e5e (attr: teach "--attr-source=<tree>" global option to "git",
+ 44451a2 (attr: teach "--attr-source=<tree>" global option to "git",
2023-05-06) provided the ability to pass in a treeish as the attr
source. In the context of serving Git repositories as bare repos like we
do at GitLab however, it would be easier to point --attr-source to HEAD
@@ Documentation/config/attr.txt (new)
@@
+attr.tree:
+ A <tree-ish> to read gitattributes from instead of the worktree. See
-+ linkgit:gitattributes[5]. This is equivalent to setting the
-+ `GIT_ATTR_SOURCE` environment variable, or passing in --attr-source to
-+ the Git command.
++ linkgit:gitattributes[5]. If `attr.tree` does not resolve to a valid tree,
++ treat it as an empty tree. --attr-source and GIT_ATTR_SOURCE take
++ precedence over attr.tree.
## attr.c ##
+@@
+ #include "tree-walk.h"
+ #include "object-name.h"
+
++const char *git_attr_tree;
++
+ const char git_attr__true[] = "(builtin)true";
+ const char git_attr__false[] = "\0(builtin)false";
+ static const char git_attr__unknown[] = "(builtin)unknown";
@@ attr.c: static void compute_default_attr_source(struct object_id *attr_source)
if (!default_attr_source_tree_object_name)
default_attr_source_tree_object_name = getenv(GIT_ATTR_SOURCE_ENVIRONMENT);
+ if (!default_attr_source_tree_object_name) {
-+ char *attr_tree;
-+
-+ if (!git_config_get_string("attr.tree", &attr_tree))
-+ default_attr_source_tree_object_name = attr_tree;
++ default_attr_source_tree_object_name = git_attr_tree;
++ ignore_bad_attr_tree = 1;
+ }
+
- if (!default_attr_source_tree_object_name || !is_null_oid(attr_source))
- return;
+ if (!default_attr_source_tree_object_name &&
+ startup_info->have_repository &&
+ is_bare_repository()) {
+
+ ## attr.h ##
+@@ attr.h: const char *git_attr_global_file(void);
+ /* Return whether the system gitattributes file is enabled and should be used. */
+ int git_attr_system_is_enabled(void);
+
++extern const char *git_attr_tree;
++
+ #endif /* ATTR_H */
+
+ ## config.c ##
+@@
+ #include "repository.h"
+ #include "lockfile.h"
+ #include "mailmap.h"
++#include "attr.h"
+ #include "exec-cmd.h"
+ #include "strbuf.h"
+ #include "quote.h"
+@@ config.c: static int git_default_mailmap_config(const char *var, const char *value)
+ return 0;
+ }
+
++static int git_default_attr_config(const char *var, const char *value)
++{
++ if (!strcmp(var, "attr.tree"))
++ return git_config_string(&git_attr_tree, var, value);
++
++ /* Add other attribute related config variables here and to
++ Documentation/config/attr.txt. */
++ return 0;
++}
++
+ int git_default_config(const char *var, const char *value,
+ const struct config_context *ctx, void *cb)
+ {
+@@ config.c: int git_default_config(const char *var, const char *value,
+ if (starts_with(var, "mailmap."))
+ return git_default_mailmap_config(var, value);
+
++ if (starts_with(var, "attr."))
++ return git_default_attr_config(var, value);
++
+ if (starts_with(var, "advice.") || starts_with(var, "color.advice"))
+ return git_default_advice_config(var, value);
## t/t0003-attributes.sh ##
@@ t/t0003-attributes.sh: attr_check_source () {
GIT_ATTR_SOURCE="$source" git $git_opts check-attr test -- "$path" >actual 2>err &&
test_cmp expect actual &&
test_must_be_empty err
+@@ t/t0003-attributes.sh: test_expect_success 'bare repository: check that .gitattribute is ignored' '
+ )
+ '
+
++bad_attr_source_err="fatal: bad --attr-source or GIT_ATTR_SOURCE"
++
++test_expect_success 'attr.tree when HEAD is unborn' '
++ test_when_finished rm -rf empty &&
++ git init empty &&
++ (
++ cd empty &&
++ echo $bad_attr_source_err >expect_err &&
++ echo "f/path: test: unspecified" >expect &&
++ git -c attr.tree=HEAD check-attr test -- f/path >actual 2>err &&
++ test_must_be_empty err &&
++ test_cmp expect actual
++ )
++'
++
++test_expect_success 'attr.tree points to non-existing ref' '
++ test_when_finished rm -rf empty &&
++ git init empty &&
++ (
++ cd empty &&
++ echo $bad_attr_source_err >expect_err &&
++ echo "f/path: test: unspecified" >expect &&
++ git -c attr.tree=refs/does/not/exist check-attr test -- f/path >actual 2>err &&
++ test_must_be_empty err &&
++ test_cmp expect actual
++ )
++'
++
++test_expect_success 'bad attr source defaults to reading .gitattributes file' '
++ test_when_finished rm -rf empty &&
++ git init empty &&
++ (
++ cd empty &&
++ echo "f/path test=val" >.gitattributes &&
++ echo "f/path: test: val" >expect &&
++ git -c attr.tree=HEAD check-attr test -- f/path >actual 2>err &&
++ test_must_be_empty err &&
++ test_cmp expect actual
++ )
++'
+
+ test_expect_success 'bare repo defaults to reading .gitattributes from HEAD' '
+ test_when_finished rm -rf test bare_with_gitattribute &&
+@@ t/t0003-attributes.sh: test_expect_success 'bare repo defaults to reading .gitattributes from HEAD' '
+ test_cmp expect actual
+ '
+
++test_expect_success '--attr-source and GIT_ATTR_SOURCE take precedence over attr.tree' '
++ test_when_finished rm -rf empty &&
++ git init empty &&
++ (
++ cd empty &&
++ git checkout -b attr-source &&
++ test_commit "val1" .gitattributes "f/path test=val1" &&
++ git checkout -b attr-tree &&
++ test_commit "val2" .gitattributes "f/path test=val2" &&
++ git checkout attr-source &&
++ echo "f/path: test: val1" >expect &&
++ git -c attr.tree=attr-tree --attr-source=attr-source check-attr test -- f/path >actual &&
++ test_cmp expect actual &&
++ GIT_ATTR_SOURCE=attr-source git -c attr.tree=attr-tree check-attr test -- f/path >actual &&
++ test_cmp expect actual
++ )
++'
++
+ test_expect_success 'bare repository: with --source' '
+ (
+ cd bare.git &&
--
gitgitgadget
^ permalink raw reply
* [PATCH v3 1/2] attr: read attributes from HEAD when bare repo
From: John Cai via GitGitGadget @ 2023-10-10 19:49 UTC (permalink / raw)
To: git; +Cc: Jeff King, Jonathan Tan, John Cai, John Cai
In-Reply-To: <pull.1577.v3.git.git.1696967380.gitgitgadget@gmail.com>
From: John Cai <johncai86@gmail.com>
The motivation for 44451a2e5e (attr: teach "--attr-source=<tree>" global
option to "git" , 2023-05-06), was to make it possible to use
gitattributes with bare repositories.
To make it easier to read gitattributes in bare repositories however,
let's just make HEAD:.gitattributes the default. This is in line with
how mailmap works, 8c473cecfd (mailmap: default mailmap.blob in bare
repositories, 2012-12-13).
Signed-off-by: John Cai <johncai86@gmail.com>
---
attr.c | 12 +++++++++++-
t/t0003-attributes.sh | 14 ++++++++++++++
t/t5001-archive-attr.sh | 2 +-
3 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/attr.c b/attr.c
index 71c84fbcf86..bf2ea1626a6 100644
--- a/attr.c
+++ b/attr.c
@@ -1194,6 +1194,7 @@ static void collect_some_attrs(struct index_state *istate,
}
static const char *default_attr_source_tree_object_name;
+static int ignore_bad_attr_tree;
void set_git_attr_source(const char *tree_object_name)
{
@@ -1205,10 +1206,19 @@ static void compute_default_attr_source(struct object_id *attr_source)
if (!default_attr_source_tree_object_name)
default_attr_source_tree_object_name = getenv(GIT_ATTR_SOURCE_ENVIRONMENT);
+ if (!default_attr_source_tree_object_name &&
+ startup_info->have_repository &&
+ is_bare_repository()) {
+ default_attr_source_tree_object_name = "HEAD";
+ ignore_bad_attr_tree = 1;
+ }
+
if (!default_attr_source_tree_object_name || !is_null_oid(attr_source))
return;
- if (repo_get_oid_treeish(the_repository, default_attr_source_tree_object_name, attr_source))
+ if (repo_get_oid_treeish(the_repository,
+ default_attr_source_tree_object_name,
+ attr_source) && !ignore_bad_attr_tree)
die(_("bad --attr-source or GIT_ATTR_SOURCE"));
}
diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
index 26e082f05b4..e6b1a117228 100755
--- a/t/t0003-attributes.sh
+++ b/t/t0003-attributes.sh
@@ -342,6 +342,20 @@ test_expect_success 'bare repository: check that .gitattribute is ignored' '
)
'
+
+test_expect_success 'bare repo defaults to reading .gitattributes from HEAD' '
+ test_when_finished rm -rf test bare_with_gitattribute &&
+ git init test &&
+ (
+ cd test &&
+ test_commit gitattributes .gitattributes "f/path test=val"
+ ) &&
+ git clone --bare test bare_with_gitattribute &&
+ echo "f/path: test: val" >expect &&
+ git -C bare_with_gitattribute check-attr test -- f/path >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'bare repository: with --source' '
(
cd bare.git &&
diff --git a/t/t5001-archive-attr.sh b/t/t5001-archive-attr.sh
index 0ff47a239db..eaf959d8f63 100755
--- a/t/t5001-archive-attr.sh
+++ b/t/t5001-archive-attr.sh
@@ -138,7 +138,7 @@ test_expect_success 'git archive with worktree attributes, bare' '
'
test_expect_missing bare-worktree/ignored
-test_expect_exists bare-worktree/ignored-by-tree
+test_expect_missing bare-worktree/ignored-by-tree
test_expect_exists bare-worktree/ignored-by-worktree
test_expect_success 'export-subst' '
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 2/2] attr: add attr.tree for setting the treeish to read attributes from
From: John Cai via GitGitGadget @ 2023-10-10 19:49 UTC (permalink / raw)
To: git; +Cc: Jeff King, Jonathan Tan, John Cai, John Cai
In-Reply-To: <pull.1577.v3.git.git.1696967380.gitgitgadget@gmail.com>
From: John Cai <johncai86@gmail.com>
44451a2 (attr: teach "--attr-source=<tree>" global option to "git",
2023-05-06) provided the ability to pass in a treeish as the attr
source. In the context of serving Git repositories as bare repos like we
do at GitLab however, it would be easier to point --attr-source to HEAD
for all commands by setting it once.
Add a new config attr.tree that allows this.
Signed-off-by: John Cai <johncai86@gmail.com>
---
Documentation/config.txt | 2 ++
Documentation/config/attr.txt | 5 +++
attr.c | 7 ++++
attr.h | 2 ++
config.c | 14 ++++++++
t/t0003-attributes.sh | 62 +++++++++++++++++++++++++++++++++++
6 files changed, 92 insertions(+)
create mode 100644 Documentation/config/attr.txt
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 229b63a454c..b1891c2b5af 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -371,6 +371,8 @@ other popular tools, and describe them in your documentation.
include::config/advice.txt[]
+include::config/attr.txt[]
+
include::config/core.txt[]
include::config/add.txt[]
diff --git a/Documentation/config/attr.txt b/Documentation/config/attr.txt
new file mode 100644
index 00000000000..be882523f8b
--- /dev/null
+++ b/Documentation/config/attr.txt
@@ -0,0 +1,5 @@
+attr.tree:
+ A <tree-ish> to read gitattributes from instead of the worktree. See
+ linkgit:gitattributes[5]. If `attr.tree` does not resolve to a valid tree,
+ treat it as an empty tree. --attr-source and GIT_ATTR_SOURCE take
+ precedence over attr.tree.
diff --git a/attr.c b/attr.c
index bf2ea1626a6..0ae6852d12b 100644
--- a/attr.c
+++ b/attr.c
@@ -24,6 +24,8 @@
#include "tree-walk.h"
#include "object-name.h"
+const char *git_attr_tree;
+
const char git_attr__true[] = "(builtin)true";
const char git_attr__false[] = "\0(builtin)false";
static const char git_attr__unknown[] = "(builtin)unknown";
@@ -1206,6 +1208,11 @@ static void compute_default_attr_source(struct object_id *attr_source)
if (!default_attr_source_tree_object_name)
default_attr_source_tree_object_name = getenv(GIT_ATTR_SOURCE_ENVIRONMENT);
+ if (!default_attr_source_tree_object_name) {
+ default_attr_source_tree_object_name = git_attr_tree;
+ ignore_bad_attr_tree = 1;
+ }
+
if (!default_attr_source_tree_object_name &&
startup_info->have_repository &&
is_bare_repository()) {
diff --git a/attr.h b/attr.h
index 2b745df4054..127998ae013 100644
--- a/attr.h
+++ b/attr.h
@@ -236,4 +236,6 @@ const char *git_attr_global_file(void);
/* Return whether the system gitattributes file is enabled and should be used. */
int git_attr_system_is_enabled(void);
+extern const char *git_attr_tree;
+
#endif /* ATTR_H */
diff --git a/config.c b/config.c
index 3846a37be97..21a1590b505 100644
--- a/config.c
+++ b/config.c
@@ -18,6 +18,7 @@
#include "repository.h"
#include "lockfile.h"
#include "mailmap.h"
+#include "attr.h"
#include "exec-cmd.h"
#include "strbuf.h"
#include "quote.h"
@@ -1904,6 +1905,16 @@ static int git_default_mailmap_config(const char *var, const char *value)
return 0;
}
+static int git_default_attr_config(const char *var, const char *value)
+{
+ if (!strcmp(var, "attr.tree"))
+ return git_config_string(&git_attr_tree, var, value);
+
+ /* Add other attribute related config variables here and to
+ Documentation/config/attr.txt. */
+ return 0;
+}
+
int git_default_config(const char *var, const char *value,
const struct config_context *ctx, void *cb)
{
@@ -1927,6 +1938,9 @@ int git_default_config(const char *var, const char *value,
if (starts_with(var, "mailmap."))
return git_default_mailmap_config(var, value);
+ if (starts_with(var, "attr."))
+ return git_default_attr_config(var, value);
+
if (starts_with(var, "advice.") || starts_with(var, "color.advice"))
return git_default_advice_config(var, value);
diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
index e6b1a117228..b0949125f26 100755
--- a/t/t0003-attributes.sh
+++ b/t/t0003-attributes.sh
@@ -40,6 +40,10 @@ attr_check_source () {
test_cmp expect actual &&
test_must_be_empty err
+ git $git_opts -c "attr.tree=$source" check-attr test -- "$path" >actual 2>err &&
+ test_cmp expect actual &&
+ test_must_be_empty err
+
GIT_ATTR_SOURCE="$source" git $git_opts check-attr test -- "$path" >actual 2>err &&
test_cmp expect actual &&
test_must_be_empty err
@@ -342,6 +346,46 @@ test_expect_success 'bare repository: check that .gitattribute is ignored' '
)
'
+bad_attr_source_err="fatal: bad --attr-source or GIT_ATTR_SOURCE"
+
+test_expect_success 'attr.tree when HEAD is unborn' '
+ test_when_finished rm -rf empty &&
+ git init empty &&
+ (
+ cd empty &&
+ echo $bad_attr_source_err >expect_err &&
+ echo "f/path: test: unspecified" >expect &&
+ git -c attr.tree=HEAD check-attr test -- f/path >actual 2>err &&
+ test_must_be_empty err &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'attr.tree points to non-existing ref' '
+ test_when_finished rm -rf empty &&
+ git init empty &&
+ (
+ cd empty &&
+ echo $bad_attr_source_err >expect_err &&
+ echo "f/path: test: unspecified" >expect &&
+ git -c attr.tree=refs/does/not/exist check-attr test -- f/path >actual 2>err &&
+ test_must_be_empty err &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'bad attr source defaults to reading .gitattributes file' '
+ test_when_finished rm -rf empty &&
+ git init empty &&
+ (
+ cd empty &&
+ echo "f/path test=val" >.gitattributes &&
+ echo "f/path: test: val" >expect &&
+ git -c attr.tree=HEAD check-attr test -- f/path >actual 2>err &&
+ test_must_be_empty err &&
+ test_cmp expect actual
+ )
+'
test_expect_success 'bare repo defaults to reading .gitattributes from HEAD' '
test_when_finished rm -rf test bare_with_gitattribute &&
@@ -356,6 +400,24 @@ test_expect_success 'bare repo defaults to reading .gitattributes from HEAD' '
test_cmp expect actual
'
+test_expect_success '--attr-source and GIT_ATTR_SOURCE take precedence over attr.tree' '
+ test_when_finished rm -rf empty &&
+ git init empty &&
+ (
+ cd empty &&
+ git checkout -b attr-source &&
+ test_commit "val1" .gitattributes "f/path test=val1" &&
+ git checkout -b attr-tree &&
+ test_commit "val2" .gitattributes "f/path test=val2" &&
+ git checkout attr-source &&
+ echo "f/path: test: val1" >expect &&
+ git -c attr.tree=attr-tree --attr-source=attr-source check-attr test -- f/path >actual &&
+ test_cmp expect actual &&
+ GIT_ATTR_SOURCE=attr-source git -c attr.tree=attr-tree check-attr test -- f/path >actual &&
+ test_cmp expect actual
+ )
+'
+
test_expect_success 'bare repository: with --source' '
(
cd bare.git &&
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH v3 1/2] attr: read attributes from HEAD when bare repo
From: Eric Sunshine @ 2023-10-10 19:58 UTC (permalink / raw)
To: John Cai via GitGitGadget; +Cc: git, Jeff King, Jonathan Tan, John Cai
In-Reply-To: <cef206d47c724f54220b0b915e5405b48f5eb2cb.1696967380.git.gitgitgadget@gmail.com>
On Tue, Oct 10, 2023 at 3:49 PM John Cai via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> The motivation for 44451a2e5e (attr: teach "--attr-source=<tree>" global
> option to "git" , 2023-05-06), was to make it possible to use
> gitattributes with bare repositories.
>
> To make it easier to read gitattributes in bare repositories however,
> let's just make HEAD:.gitattributes the default. This is in line with
> how mailmap works, 8c473cecfd (mailmap: default mailmap.blob in bare
> repositories, 2012-12-13).
>
> Signed-off-by: John Cai <johncai86@gmail.com>
> ---
> diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
> @@ -342,6 +342,20 @@ test_expect_success 'bare repository: check that .gitattribute is ignored' '
> +test_expect_success 'bare repo defaults to reading .gitattributes from HEAD' '
> + test_when_finished rm -rf test bare_with_gitattribute &&
> + git init test &&
> + (
> + cd test &&
> + test_commit gitattributes .gitattributes "f/path test=val"
> + ) &&
Not at all worth a reroll, rather just for future reference... these
days test_commit() supports -C, so the same could be accomplished
without the subshell or `cd`:
test_commit -C test gitattributes .gitattributes "f/path test=val" &&
> + git clone --bare test bare_with_gitattribute &&
> + echo "f/path: test: val" >expect &&
> + git -C bare_with_gitattribute check-attr test -- f/path >actual &&
> + test_cmp expect actual
> +'
^ permalink raw reply
* [PATCH v3 00/17] bloom: changed-path Bloom filters v2 (& sundries)
From: Taylor Blau @ 2023-10-10 20:33 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, Junio C Hamano, Jeff King, SZEDER Gábor
In-Reply-To: <cover.1692654233.git.me@ttaylorr.com>
(Rebased onto the tip of 'master', which is 3a06386e31 (The fifteenth
batch, 2023-10-04), at the time of writing).
This series is a reroll of the combined efforts of [1] and [2] to
introduce the v2 changed-path Bloom filters, which fixes a bug in our
existing implementation of murmur3 paths with non-ASCII characters (when
the "char" type is signed).
In large part, this is the same as the previous round. But this round
includes some extra bits that address issues pointed out by SZEDER
Gábor, which are:
- not reading Bloom filters for root commits
- corrupting Bloom filter reads by tweaking the filter settings
between layers.
These issues were discussed in (among other places) [3], and [4],
respectively.
Thanks to Jonathan, Peff, and SZEDER who have helped a great deal in
assembling these patches. As usual, a range-diff is included below.
Thanks in advance for your
review!
[1]: https://lore.kernel.org/git/cover.1684790529.git.jonathantanmy@google.com/
[2]: https://lore.kernel.org/git/cover.1691426160.git.me@ttaylorr.com/
[3]: https://public-inbox.org/git/20201015132147.GB24954@szeder.dev/
[4]: https://lore.kernel.org/git/20230830200218.GA5147@szeder.dev/
Jonathan Tan (4):
gitformat-commit-graph: describe version 2 of BDAT
t4216: test changed path filters with high bit paths
repo-settings: introduce commitgraph.changedPathsVersion
commit-graph: new filter ver. that fixes murmur3
Taylor Blau (13):
t/t4216-log-bloom.sh: harden `test_bloom_filters_not_used()`
revision.c: consult Bloom filters for root commits
commit-graph: ensure Bloom filters are read with consistent settings
t/helper/test-read-graph.c: extract `dump_graph_info()`
bloom.h: make `load_bloom_filter_from_graph()` public
t/helper/test-read-graph: implement `bloom-filters` mode
bloom: annotate filters with hash version
bloom: prepare to discard incompatible Bloom filters
commit-graph.c: unconditionally load Bloom filters
commit-graph: drop unnecessary `graph_read_bloom_data_context`
object.h: fix mis-aligned flag bits table
commit-graph: reuse existing Bloom filters where possible
bloom: introduce `deinit_bloom_filters()`
Documentation/config/commitgraph.txt | 26 ++-
Documentation/gitformat-commit-graph.txt | 9 +-
bloom.c | 208 ++++++++++++++++-
bloom.h | 38 +++-
commit-graph.c | 61 ++++-
object.h | 3 +-
oss-fuzz/fuzz-commit-graph.c | 2 +-
repo-settings.c | 6 +-
repository.h | 2 +-
revision.c | 26 ++-
t/helper/test-bloom.c | 9 +-
t/helper/test-read-graph.c | 67 ++++--
t/t0095-bloom.sh | 8 +
t/t4216-log-bloom.sh | 272 ++++++++++++++++++++++-
14 files changed, 682 insertions(+), 55 deletions(-)
Range-diff against v2:
10: 002a06d1e9 ! 1: fe671d616c t/t4216-log-bloom.sh: harden `test_bloom_filters_not_used()`
@@ Commit message
indicating that no filters were used.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
- Signed-off-by: Junio C Hamano <gitster@pobox.com>
- Signed-off-by: Taylor Blau <me@ttaylorr.com>
## t/t4216-log-bloom.sh ##
@@ t/t4216-log-bloom.sh: test_bloom_filters_used () {
-: ---------- > 2: 7d0fa93543 revision.c: consult Bloom filters for root commits
-: ---------- > 3: 2ecc0a2d58 commit-graph: ensure Bloom filters are read with consistent settings
1: 5fa681b58e ! 4: 17703ed89a gitformat-commit-graph: describe version 2 of BDAT
@@ Commit message
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
- Signed-off-by: Junio C Hamano <gitster@pobox.com>
- Signed-off-by: Taylor Blau <me@ttaylorr.com>
## Documentation/gitformat-commit-graph.txt ##
@@ Documentation/gitformat-commit-graph.txt: All multi-byte numbers are in network byte order.
2: 623d840575 ! 5: 94552abf45 t/helper/test-read-graph.c: extract `dump_graph_info()`
@@ Commit message
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
- Signed-off-by: Junio C Hamano <gitster@pobox.com>
- Signed-off-by: Taylor Blau <me@ttaylorr.com>
## t/helper/test-read-graph.c ##
@@
3: bc9d77ae60 ! 6: 3d81efa27b bloom.h: make `load_bloom_filter_from_graph()` public
@@ Commit message
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
- Signed-off-by: Junio C Hamano <gitster@pobox.com>
- Signed-off-by: Taylor Blau <me@ttaylorr.com>
## bloom.c ##
@@ bloom.c: static inline unsigned char get_bitmask(uint32_t pos)
4: ac7008aed3 ! 7: d23cd89037 t/helper/test-read-graph: implement `bloom-filters` mode
@@ Commit message
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
- Signed-off-by: Junio C Hamano <gitster@pobox.com>
- Signed-off-by: Taylor Blau <me@ttaylorr.com>
## t/helper/test-read-graph.c ##
@@ t/helper/test-read-graph.c: static void dump_graph_info(struct commit_graph *graph)
@@ t/helper/test-read-graph.c: int cmd__read_graph(int argc UNUSED, const char **ar
- return 0;
+ return ret;
}
++
++
5: 71755ba856 ! 8: cba766f224 t4216: test changed path filters with high bit paths
@@ Commit message
Signed-off-by: Taylor Blau <me@ttaylorr.com>
## t/t4216-log-bloom.sh ##
-@@ t/t4216-log-bloom.sh: test_expect_success 'Bloom generation backfills empty commits' '
- )
+@@ t/t4216-log-bloom.sh: test_expect_success 'merge graph layers with incompatible Bloom settings' '
+ ! grep "disabling Bloom filters" err
'
+get_first_changed_path_filter () {
6: 9768d92c0f ! 9: a08a961f41 repo-settings: introduce commitgraph.changedPathsVersion
@@ Commit message
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
- Signed-off-by: Junio C Hamano <gitster@pobox.com>
- Signed-off-by: Taylor Blau <me@ttaylorr.com>
## Documentation/config/commitgraph.txt ##
@@ Documentation/config/commitgraph.txt: commitGraph.maxNewFilters::
7: f911b4bfab = 10: 61d44519a5 commit-graph: new filter ver. that fixes murmur3
8: 35009900df ! 11: a8c10f8de8 bloom: annotate filters with hash version
@@ Commit message
Bloom filter.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
- Signed-off-by: Junio C Hamano <gitster@pobox.com>
- Signed-off-by: Taylor Blau <me@ttaylorr.com>
## bloom.c ##
@@ bloom.c: int load_bloom_filter_from_graph(struct commit_graph *g,
9: 138bc16905 ! 12: 2ba10a4b4b bloom: prepare to discard incompatible Bloom filters
@@ Commit message
`get_or_compute_bloom_filter()`.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
- Signed-off-by: Junio C Hamano <gitster@pobox.com>
- Signed-off-by: Taylor Blau <me@ttaylorr.com>
## bloom.c ##
@@ bloom.c: static void init_truncated_large_filter(struct bloom_filter *filter,
11: 2437e62813 ! 13: 09d8669c3a commit-graph.c: unconditionally load Bloom filters
@@ Commit message
either "1" or "2".
Signed-off-by: Taylor Blau <me@ttaylorr.com>
- Signed-off-by: Junio C Hamano <gitster@pobox.com>
- Signed-off-by: Taylor Blau <me@ttaylorr.com>
## commit-graph.c ##
@@ commit-graph.c: static int graph_read_bloom_data(const unsigned char *chunk_start,
12: fe8fb2f5fe ! 14: 0d4f9dc4ee commit-graph: drop unnecessary `graph_read_bloom_data_context`
@@ Commit message
Noticed-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
- Signed-off-by: Junio C Hamano <gitster@pobox.com>
- Signed-off-by: Taylor Blau <me@ttaylorr.com>
## commit-graph.c ##
@@ commit-graph.c: static int graph_read_oid_lookup(const unsigned char *chunk_start,
13: 825af91e11 ! 15: 1f7f27bc47 object.h: fix mis-aligned flag bits table
@@ Commit message
Bit position 23 is one column too far to the left.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
- Signed-off-by: Junio C Hamano <gitster@pobox.com>
- Signed-off-by: Taylor Blau <me@ttaylorr.com>
## object.h ##
@@ object.h: void object_array_init(struct object_array *array);
14: 593b317192 ! 16: abbef95ae8 commit-graph: reuse existing Bloom filters where possible
@@ Commit message
commits by their generation number.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
- Signed-off-by: Junio C Hamano <gitster@pobox.com>
- Signed-off-by: Taylor Blau <me@ttaylorr.com>
## bloom.c ##
@@
15: 8bf2c9cf98 = 17: ca362408d5 bloom: introduce `deinit_bloom_filters()`
--
2.42.0.342.g8bb3a896ee
^ 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