Git Mailing List Archive mirror
 help / color / mirror / Atom feed
* [Bug] In `git-rev-list(1)`, using the `--objects` flag doesn't work well with the `--not` flag, as non-commit objects are not excluded
@ 2023-08-15 16:44 Karthik Nayak
  2023-08-15 19:31 ` Junio C Hamano
  2023-08-15 20:52 ` [Bug???] " Junio C Hamano
  0 siblings, 2 replies; 8+ messages in thread
From: Karthik Nayak @ 2023-08-15 16:44 UTC (permalink / raw)
  To: git

Hello!

What did you do before the bug happened? (Steps to reproduce your issue)

Assume a repository structure as follows:

- commit1 9f2aa2eb987c2281bb4901dbccd1398ad2c39722
  - tree: 205f6b799e7d5c2524468ca006a0131aa57ecce7
    - 100644 blob 257cc5642cb1a054f08cc83f2d943e56fd3ebe99    foo
      - content: foo
- commit2 9e02481f4df3a8997335b0a68882580e3b9b588f (parent:
9f2aa2eb987c2281bb4901dbccd1398ad2c39722)
  - tree: 672d0aa883d095369c56416587bc397eee4ac37e
    - 100644 blob 257cc5642cb1a054f08cc83f2d943e56fd3ebe99    foo
      - content: foo
    - 100644 blob eec8c88a93f6ee1515fb8348f2c122cfda4302cd    moo
      - content: moo
- commit3 91fa9611a355db77a07f963c746d57f75af380da (parent:
9e02481f4df3a8997335b0a68882580e3b9b588f)
   - tree 0c16a6cc9eef3fdd3034c1ffe2fc5e6d0bba2192
     - tree 086885f71429e3599c8c903b0e9ed491f6522879    bar
       - 100644 blob 7a67abed5f99fdd3ee203dd137b9818d88b1bafd    goo
         - content: goo
     - 100644 blob 257cc5642cb1a054f08cc83f2d943e56fd3ebe99    foo
       - content: foo
     - 100644 blob eec8c88a93f6ee1515fb8348f2c122cfda4302cd    moo
       - content: moo
     - 100644 blob 8baef1b4abc478178b004d62031cf7fe6db6f903    abc
       - content: abc
- commit4 6b52ed5b176604a0740689b5bb9be7bd79f4bced (parent:
9f2aa2eb987c2281bb4901dbccd1398ad2c39722)
  - tree ff05824d2f76436c61d2c971e11a27514aba6948
    - tree 086885f71429e3599c8c903b0e9ed491f6522879    bar
      - 100644 blob 7a67abed5f99fdd3ee203dd137b9818d88b1bafd    goo
        - content: goo
    - 100644 blob 257cc5642cb1a054f08cc83f2d943e56fd3ebe99    foo
      - content: foo
    - 100644 blob 8baef1b4abc478178b004d62031cf7fe6db6f903    abc
      - content: abc

What did you expect to happen? (Expected behavior)

In such a repository, the output for the command, should have the
output provided below

❯ git rev-list --objects 6b52ed5b176604a0740689b5bb9be7bd79f4bced
--not 91fa9611a355db77a07f963c746d57f75af380da
6b52ed5b176604a0740689b5bb9be7bd79f4bced
ff05824d2f76436c61d2c971e11a27514aba6948

What happened instead? (Actual behavior)

Instead, the output is as follows:

❯ git rev-list --objects 6b52ed5b176604a0740689b5bb9be7bd79f4bced
--not 91fa9611a355db77a07f963c746d57f75af380da
6b52ed5b176604a0740689b5bb9be7bd79f4bced
ff05824d2f76436c61d2c971e11a27514aba6948
8baef1b4abc478178b004d62031cf7fe6db6f903 abc
086885f71429e3599c8c903b0e9ed491f6522879 bar
7a67abed5f99fdd3ee203dd137b9818d88b1bafd bar/goo

What's different between what you expected and what actually happened?

If you notice here, the objects
`8baef1b4abc478178b004d62031cf7fe6db6f903`,
`086885f71429e3599c8c903b0e9ed491f6522879` and
`7a67abed5f99fdd3ee203dd137b9818d88b1bafd` are included in the output,
these objects are reachable from
`91fa9611a355db77a07f963c746d57f75af380da` and shouldn't have been
included since we used the `--not` flag.

Anything else you want to add:

I did some preliminary walkthrough of the code to understand why this
happens, and my understanding is as follows:
1. In rev-list.c: we first set up the revisions provided via the
`setup_revisions()` function. Here, any revision provided under the
`--not` flag is marked as `UNINTERESTING`.
2. In rev-list.c: we then call `prepare_revision_walk()`, this
function internally goes through the commits and calls
`handle_commit()` on each of the commit. In our case
(6b52ed5b176604a0740689b5bb9be7bd79f4bced,
91fa9611a355db77a07f963c746d57f75af380da).
3. In revision.c: In `handle_commit()` we set `revs->limited = 1`
since one of our commits is marked as `UNINTERESTING`.
4. In revision.c: Back in `prepare_revision_walk()`, since
`revs->limited` is set, we call `limit_list()`.
5. In revision.c: Not sure what the purpose of `limit_list()` is, but
seems like it is to optimize the revision walk to reduce the traversal
later on. In our case, we can mark all parents of the commit as
uninteresting and remove the commit from the rev list entirely.
6. In rev-list.c: Finally, when we call
`traverse_commit_list_filtered` for the traversal, we recursively show
commit/object unless we come across something `UNINTERESTING`. Since
only the commits were marked as `UNINTERESTING`, any shared
trees/blobs will still be printed to output.

The diff below fixes the issue:

@@ -3790,11 +3833,12 @@ int prepare_revision_walk(struct rev_info *revs)
         commit_list_sort_by_date(&revs->commits);
     if (revs->no_walk)
         return 0;
-    if (revs->limited) {
+    if (revs->limited && !revs->tree_objects) {
         if (limit_list(revs) < 0)
             return -1;
         if (revs->topo_order)

But this is definitely a very _naive_ fix. Before diving into fixing
this, it would be nice to hear some thoughts on this.

[System Info]
git version:
git version 2.41.0
cpu: x86_64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
uname: Linux 6.4.9-200.fc38.x86_64 #1 SMP PREEMPT_DYNAMIC Tue Aug  8
21:21:11 UTC 2023 x86_64
compiler info: gnuc: 13.1
libc info: glibc: 2.37
$SHELL (typically, interactive shell): /bin/fish


[Enabled Hooks]
not run from a git repository - no hooks to show

- Karthik

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Bug] In `git-rev-list(1)`, using the `--objects` flag doesn't work well with the `--not` flag, as non-commit objects are not excluded
  2023-08-15 16:44 [Bug] In `git-rev-list(1)`, using the `--objects` flag doesn't work well with the `--not` flag, as non-commit objects are not excluded Karthik Nayak
@ 2023-08-15 19:31 ` Junio C Hamano
  2023-08-15 22:11   ` Taylor Blau
  2023-08-15 22:56   ` Karthik Nayak
  2023-08-15 20:52 ` [Bug???] " Junio C Hamano
  1 sibling, 2 replies; 8+ messages in thread
From: Junio C Hamano @ 2023-08-15 19:31 UTC (permalink / raw)
  To: Karthik Nayak; +Cc: git

Karthik Nayak <karthik.188@gmail.com> writes:

> If you notice here, the objects
> `8baef1b4abc478178b004d62031cf7fe6db6f903`,
> `086885f71429e3599c8c903b0e9ed491f6522879` and
> `7a67abed5f99fdd3ee203dd137b9818d88b1bafd` are included in the output,
> these objects are reachable from
> `91fa9611a355db77a07f963c746d57f75af380da` and shouldn't have been
> included since we used the `--not` flag.

For performance reasons, we cannot afford to enumerate all objects
that are reachable from --not objects and exclude them from the
output.  So it is a balancing act to decide where to draw the line.

Spending more cycles and heaps for traversing the --not side deeper
may make the --objects output smaller and more precise, but there of
course is cost associated with it.  And --objects do not promise
that it gives absolute minimum---the reason it exists is to make
sure the objects listed are sufficient to fill gaps between the
--not tips and positive ones, which is the primary reason for its
existence.

> The diff below fixes the issue:
>
> @@ -3790,11 +3833,12 @@ int prepare_revision_walk(struct rev_info *revs)
>          commit_list_sort_by_date(&revs->commits);
>      if (revs->no_walk)
>          return 0;
> -    if (revs->limited) {
> +    if (revs->limited && !revs->tree_objects) {
>          if (limit_list(revs) < 0)
>              return -1;
>          if (revs->topo_order)

This might change the size of the output and "fix" the output in
your particular small test case, but I am not sure what kind of bugs
this will introduce in the more general codepath.

Not calling limit_list() when the .limited bit is on is breaking one
of the most fundamental assumptions in the revision traversal.  When
a feature is enabled that needs to paint the graph upfront before it
can compute its result, the code that enables the feature flips the
.limited to ask this part of the code to make sure it calls
limit_list() to paint the graph with UNINTERESTING bit.

This area to paint uninteresting trees have changed quite
drastically at d5d2e935 (revision: implement sparse algorithm,
2019-01-16).  Some of what it removed may be contributing the "over
counting" of trees that are relevant in your example.




^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Bug???] In `git-rev-list(1)`, using the `--objects` flag doesn't work well with the `--not` flag, as non-commit objects are not excluded
  2023-08-15 16:44 [Bug] In `git-rev-list(1)`, using the `--objects` flag doesn't work well with the `--not` flag, as non-commit objects are not excluded Karthik Nayak
  2023-08-15 19:31 ` Junio C Hamano
@ 2023-08-15 20:52 ` Junio C Hamano
  1 sibling, 0 replies; 8+ messages in thread
From: Junio C Hamano @ 2023-08-15 20:52 UTC (permalink / raw)
  To: Karthik Nayak; +Cc: git

Karthik Nayak <karthik.188@gmail.com> writes:

> Hello!
>
> What did you do before the bug happened? (Steps to reproduce your issue)
>
> Assume a repository structure as follows:
>
> - commit1 9f2aa2eb987c2281bb4901dbccd1398ad2c39722
>   - tree: 205f6b799e7d5c2524468ca006a0131aa57ecce7
>     - 100644 blob 257cc5642cb1a054f08cc83f2d943e56fd3ebe99    foo
>       - content: foo
> - commit2 9e02481f4df3a8997335b0a68882580e3b9b588f (parent:
> 9f2aa2eb987c2281bb4901dbccd1398ad2c39722)
>   - tree: 672d0aa883d095369c56416587bc397eee4ac37e
>     - 100644 blob 257cc5642cb1a054f08cc83f2d943e56fd3ebe99    foo
>       - content: foo
>     - 100644 blob eec8c88a93f6ee1515fb8348f2c122cfda4302cd    moo
>       - content: moo
> - commit3 91fa9611a355db77a07f963c746d57f75af380da (parent:
> 9e02481f4df3a8997335b0a68882580e3b9b588f)
>    - tree 0c16a6cc9eef3fdd3034c1ffe2fc5e6d0bba2192
>      - tree 086885f71429e3599c8c903b0e9ed491f6522879    bar
>        - 100644 blob 7a67abed5f99fdd3ee203dd137b9818d88b1bafd    goo
>          - content: goo
>      - 100644 blob 257cc5642cb1a054f08cc83f2d943e56fd3ebe99    foo
>        - content: foo
>      - 100644 blob eec8c88a93f6ee1515fb8348f2c122cfda4302cd    moo
>        - content: moo
>      - 100644 blob 8baef1b4abc478178b004d62031cf7fe6db6f903    abc
>        - content: abc
> - commit4 6b52ed5b176604a0740689b5bb9be7bd79f4bced (parent:
> 9f2aa2eb987c2281bb4901dbccd1398ad2c39722)
>   - tree ff05824d2f76436c61d2c971e11a27514aba6948
>     - tree 086885f71429e3599c8c903b0e9ed491f6522879    bar
>       - 100644 blob 7a67abed5f99fdd3ee203dd137b9818d88b1bafd    goo
>         - content: goo
>     - 100644 blob 257cc5642cb1a054f08cc83f2d943e56fd3ebe99    foo
>       - content: foo
>     - 100644 blob 8baef1b4abc478178b004d62031cf7fe6db6f903    abc
>       - content: abc

The differences in commit object names become distracting, but I do
not think your example depends on them, so a minimum reproduction
recipe should be

	$ rm -fr new ; git init new ; cd new
	$ echo foo >foo
	$ git add -A; git commit -m one; git rev-parse HEAD:
	205f6b799e7d5c2524468ca006a0131aa57ecce7
	$ echo moo >moo
	$ git add -A; git commit -m two; git rev-parse HEAD:
	672d0aa883d095369c56416587bc397eee4ac37e
	$ mkdir bar; echo goo >bar/goo; echo abc >abc
	$ git add -A; git commit -m three; git rev-parse HEAD:
	0c16a6cc9eef3fdd3034c1ffe2fc5e6d0bba2192
	$ git rm moo; git commit -m four; git rev-parse HEAD:
	ff05824d2f76436c61d2c971e11a27514aba6948

	$ git rev-list --objects HEAD^..HEAD
	5a1b93c9c4c0c9e5c969f8e0b92a02184f8f9aab
	ff05824d2f76436c61d2c971e11a27514aba6948

that output lists HEAD and HEAD^{tree} in this order, which seems to
be what you are expecting.

I am consistently getting the same result with Git 2.42-rc2, 2.41,
2.21, and 2.20 (I do not have ones older than 2.20 around).

> What did you expect to happen? (Expected behavior)
>
> In such a repository, the output for the command, should have the
> output provided below
>
> ❯ git rev-list --objects 6b52ed5b176604a0740689b5bb9be7bd79f4bced
> --not 91fa9611a355db77a07f963c746d57f75af380da
> 6b52ed5b176604a0740689b5bb9be7bd79f4bced
> ff05824d2f76436c61d2c971e11a27514aba6948
>
> What happened instead? (Actual behavior)
>
> Instead, the output is as follows:
>
> ❯ git rev-list --objects 6b52ed5b176604a0740689b5bb9be7bd79f4bced
> --not 91fa9611a355db77a07f963c746d57f75af380da
> 6b52ed5b176604a0740689b5bb9be7bd79f4bced
> ff05824d2f76436c61d2c971e11a27514aba6948
> 8baef1b4abc478178b004d62031cf7fe6db6f903 abc
> 086885f71429e3599c8c903b0e9ed491f6522879 bar
> 7a67abed5f99fdd3ee203dd137b9818d88b1bafd bar/goo

So, there is something else going on in *your* build of Git, or the
repository you prepared for testing, or both.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Bug] In `git-rev-list(1)`, using the `--objects` flag doesn't work well with the `--not` flag, as non-commit objects are not excluded
  2023-08-15 19:31 ` Junio C Hamano
@ 2023-08-15 22:11   ` Taylor Blau
  2023-08-15 22:59     ` Karthik Nayak
  2023-08-15 22:56   ` Karthik Nayak
  1 sibling, 1 reply; 8+ messages in thread
From: Taylor Blau @ 2023-08-15 22:11 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Karthik Nayak, git

On Tue, Aug 15, 2023 at 12:31:37PM -0700, Junio C Hamano wrote:
> Karthik Nayak <karthik.188@gmail.com> writes:
>
> > If you notice here, the objects
> > `8baef1b4abc478178b004d62031cf7fe6db6f903`,
> > `086885f71429e3599c8c903b0e9ed491f6522879` and
> > `7a67abed5f99fdd3ee203dd137b9818d88b1bafd` are included in the output,
> > these objects are reachable from
> > `91fa9611a355db77a07f963c746d57f75af380da` and shouldn't have been
> > included since we used the `--not` flag.
>
> For performance reasons, we cannot afford to enumerate all objects
> that are reachable from --not objects and exclude them from the
> output.  So it is a balancing act to decide where to draw the line.

As a hack, you can exploit the existing bitmap traversal routine to
build up an exact mapping of what is and isn't on either side of your
reachability query.

IOW, if you run:

    git repack -ad --write-bitmap-index

and then repeat the rev-list query with `--use-bitmap-index`, you should
get exact results.

Note that this will only work if pack.useBitmapBoundaryTraversal is set
to false, since the boundary-based traversal that is behind that
configuration option is susceptible to the same one-sided error.

Thanks,
Taylor

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Bug] In `git-rev-list(1)`, using the `--objects` flag doesn't work well with the `--not` flag, as non-commit objects are not excluded
  2023-08-15 19:31 ` Junio C Hamano
  2023-08-15 22:11   ` Taylor Blau
@ 2023-08-15 22:56   ` Karthik Nayak
  2023-08-16 18:24     ` Junio C Hamano
  1 sibling, 1 reply; 8+ messages in thread
From: Karthik Nayak @ 2023-08-15 22:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Tue, Aug 15, 2023 at 9:31 PM Junio C Hamano <gitster@pobox.com> wrote:
> > If you notice here, the objects
> > `8baef1b4abc478178b004d62031cf7fe6db6f903`,
> > `086885f71429e3599c8c903b0e9ed491f6522879` and
> > `7a67abed5f99fdd3ee203dd137b9818d88b1bafd` are included in the output,
> > these objects are reachable from
> > `91fa9611a355db77a07f963c746d57f75af380da` and shouldn't have been
> > included since we used the `--not` flag.
>
> For performance reasons, we cannot afford to enumerate all objects
> that are reachable from --not objects and exclude them from the
> output.  So it is a balancing act to decide where to draw the line.
>
> Spending more cycles and heaps for traversing the --not side deeper
> may make the --objects output smaller and more precise, but there of
> course is cost associated with it.  And --objects do not promise
> that it gives absolute minimum---the reason it exists is to make
> sure the objects listed are sufficient to fill gaps between the
> --not tips and positive ones, which is the primary reason for its
> existence.
>

I understand this, this makes sense from how rev-list is internally
used. But from a user
facing command perspective, it's not doing what it says. So either we
add to the docs
saying that there are quirks using `--not` with `--objects`  or we fix this.

So I would still consider it a bug in that sense.

> > The diff below fixes the issue:
> >
> > @@ -3790,11 +3833,12 @@ int prepare_revision_walk(struct rev_info *revs)
> >          commit_list_sort_by_date(&revs->commits);
> >      if (revs->no_walk)
> >          return 0;
> > -    if (revs->limited) {
> > +    if (revs->limited && !revs->tree_objects) {
> >          if (limit_list(revs) < 0)
> >              return -1;
> >          if (revs->topo_order)
>
> This might change the size of the output and "fix" the output in
> your particular small test case, but I am not sure what kind of bugs
> this will introduce in the more general codepath.
>
> Not calling limit_list() when the .limited bit is on is breaking one
> of the most fundamental assumptions in the revision traversal.  When
> a feature is enabled that needs to paint the graph upfront before it
> can compute its result, the code that enables the feature flips the
> .limited to ask this part of the code to make sure it calls
> limit_list() to paint the graph with UNINTERESTING bit.
>
> This area to paint uninteresting trees have changed quite
> drastically at d5d2e935 (revision: implement sparse algorithm,
> 2019-01-16).  Some of what it removed may be contributing the "over
> counting" of trees that are relevant in your example.
>

I never proposed this as the solution, this was merely me trying to understand
why it is working the way it is, hence calling it a _naive_ fix.

> The differences in commit object names become distracting, but I do
> not think your example depends on them, so a minimum reproduction
> recipe should be
>
>         $ rm -fr new ; git init new ; cd new
>         $ echo foo >foo
>         $ git add -A; git commit -m one; git rev-parse HEAD:
>         205f6b799e7d5c2524468ca006a0131aa57ecce7
>         $ echo moo >moo
>         $ git add -A; git commit -m two; git rev-parse HEAD:
>         672d0aa883d095369c56416587bc397eee4ac37e
>         $ mkdir bar; echo goo >bar/goo; echo abc >abc
>         $ git add -A; git commit -m three; git rev-parse HEAD:
>         0c16a6cc9eef3fdd3034c1ffe2fc5e6d0bba2192
>         $ git rm moo; git commit -m four; git rev-parse HEAD:
>         ff05824d2f76436c61d2c971e11a27514aba6948
>
>         $ git rev-list --objects HEAD^..HEAD
>         5a1b93c9c4c0c9e5c969f8e0b92a02184f8f9aab
>         ff05824d2f76436c61d2c971e11a27514aba6948
>
> that output lists HEAD and HEAD^{tree} in this order, which seems to
> be what you are expecting.
>
> I am consistently getting the same result with Git 2.42-rc2, 2.41,
> 2.21, and 2.20 (I do not have ones older than 2.20 around).

The provided reproduction recipe unfortunately uses a linear history and
therefore, is not the same as the example provided by me. Here is a reproducible
recipe following the same commands you used:

$ rm -fr new ; git init new ; cd new
$ echo foo >foo
$ git add -A; git commit -m one; git rev-parse HEAD
26fb965d7439c1760677377bf314d8933de0b716
$ mkdir bar; echo goo >bar/goo
$ git add -A; git commit -m two; git rev-parse HEAD
$ git checkout -B branch
$ git reset --hard @~1
HEAD is now at 26fb965 one
$ git add -A; git commit -m three; git rev-parse HEAD
91ef508167eb683486c3df6f8d07622b61ed698d

$ git rev-list --objects HEAD ^master
91ef508167eb683486c3df6f8d07622b61ed698d
ff05824d2f76436c61d2c971e11a27514aba6948
8baef1b4abc478178b004d62031cf7fe6db6f903 abc
086885f71429e3599c8c903b0e9ed491f6522879 bar
7a67abed5f99fdd3ee203dd137b9818d88b1bafd bar/goo

$ git cat-file -p HEAD^{tree}
100644 blob 8baef1b4abc478178b004d62031cf7fe6db6f903    abc
040000 tree 086885f71429e3599c8c903b0e9ed491f6522879    bar
100644 blob 257cc5642cb1a054f08cc83f2d943e56fd3ebe99    foo

$ git cat-file -p master^{tree}
040000 tree 086885f71429e3599c8c903b0e9ed491f6522879    bar
100644 blob 257cc5642cb1a054f08cc83f2d943e56fd3ebe99    foo

> So, there is something else going on in *your* build of Git, or the
> repository you prepared for testing, or both.
>

$ git version
git version 2.41.0

So the issue definitely exists, this is on an unmodified git. You can see that
the objects `086885f71429e3599c8c903b0e9ed491f6522879` and
`7a67abed5f99fdd3ee203dd137b9818d88b1bafd` are still shown (which shouldn't
have been shown).

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Bug] In `git-rev-list(1)`, using the `--objects` flag doesn't work well with the `--not` flag, as non-commit objects are not excluded
  2023-08-15 22:11   ` Taylor Blau
@ 2023-08-15 22:59     ` Karthik Nayak
  0 siblings, 0 replies; 8+ messages in thread
From: Karthik Nayak @ 2023-08-15 22:59 UTC (permalink / raw)
  To: Taylor Blau; +Cc: Junio C Hamano, git

On Wed, Aug 16, 2023 at 12:11 AM Taylor Blau <me@ttaylorr.com> wrote:
>
> On Tue, Aug 15, 2023 at 12:31:37PM -0700, Junio C Hamano wrote:
> > Karthik Nayak <karthik.188@gmail.com> writes:
> >
> > > If you notice here, the objects
> > > `8baef1b4abc478178b004d62031cf7fe6db6f903`,
> > > `086885f71429e3599c8c903b0e9ed491f6522879` and
> > > `7a67abed5f99fdd3ee203dd137b9818d88b1bafd` are included in the output,
> > > these objects are reachable from
> > > `91fa9611a355db77a07f963c746d57f75af380da` and shouldn't have been
> > > included since we used the `--not` flag.
> >
> > For performance reasons, we cannot afford to enumerate all objects
> > that are reachable from --not objects and exclude them from the
> > output.  So it is a balancing act to decide where to draw the line.
>
> As a hack, you can exploit the existing bitmap traversal routine to
> build up an exact mapping of what is and isn't on either side of your
> reachability query.
>
> IOW, if you run:
>
>     git repack -ad --write-bitmap-index
>
> and then repeat the rev-list query with `--use-bitmap-index`, you should
> get exact results.
>
> Note that this will only work if pack.useBitmapBoundaryTraversal is set
> to false, since the boundary-based traversal that is behind that
> configuration option is susceptible to the same one-sided error.
>
> Thanks,
> Taylor

Thanks! I tried using bitmap traversal and can confirm the issue doesn't
exist with this option.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Bug] In `git-rev-list(1)`, using the `--objects` flag doesn't work well with the `--not` flag, as non-commit objects are not excluded
  2023-08-15 22:56   ` Karthik Nayak
@ 2023-08-16 18:24     ` Junio C Hamano
  2023-08-16 20:58       ` Karthik Nayak
  0 siblings, 1 reply; 8+ messages in thread
From: Junio C Hamano @ 2023-08-16 18:24 UTC (permalink / raw)
  To: Karthik Nayak; +Cc: git

Karthik Nayak <karthik.188@gmail.com> writes:

> The provided reproduction recipe unfortunately uses a linear
> history and therefore, is not the same as the example provided by
> me. Here is a reproducible recipe following the same commands you
> used:
> $ rm -fr new ; git init new ; cd new
> $ echo foo >foo
> $ git add -A; git commit -m one; git rev-parse HEAD
> 26fb965d7439c1760677377bf314d8933de0b716
> $ mkdir bar; echo goo >bar/goo
> $ git add -A; git commit -m two; git rev-parse HEAD
> $ git checkout -B branch
> $ git reset --hard @~1
> HEAD is now at 26fb965 one
> $ git add -A; git commit -m three; git rev-parse HEAD
> 91ef508167eb683486c3df6f8d07622b61ed698d
>
> $ git rev-list --objects HEAD ^master
> 91ef508167eb683486c3df6f8d07622b61ed698d
> ff05824d2f76436c61d2c971e11a27514aba6948
> 8baef1b4abc478178b004d62031cf7fe6db6f903 abc
> 086885f71429e3599c8c903b0e9ed491f6522879 bar
> 7a67abed5f99fdd3ee203dd137b9818d88b1bafd bar/goo

Thanks, but the above is not recreating the same as your original
(where did "moo" go???).  Also "git rev-parse HEAD" for the sanity
checking should be spelled "git rev-parse HEAD:" if you want to help
others looking into the issue---anybody trying to reproduce will NOT
have the same commit object name, but the point of these checks is
to show the tree object name, which should reproduce for them.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Bug] In `git-rev-list(1)`, using the `--objects` flag doesn't work well with the `--not` flag, as non-commit objects are not excluded
  2023-08-16 18:24     ` Junio C Hamano
@ 2023-08-16 20:58       ` Karthik Nayak
  0 siblings, 0 replies; 8+ messages in thread
From: Karthik Nayak @ 2023-08-16 20:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Wed, Aug 16, 2023 at 8:24 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Karthik Nayak <karthik.188@gmail.com> writes:
>
> > The provided reproduction recipe unfortunately uses a linear
> > history and therefore, is not the same as the example provided by
> > me. Here is a reproducible recipe following the same commands you
> > used:
> > $ rm -fr new ; git init new ; cd new
> > $ echo foo >foo
> > $ git add -A; git commit -m one; git rev-parse HEAD
> > 26fb965d7439c1760677377bf314d8933de0b716
> > $ mkdir bar; echo goo >bar/goo
> > $ git add -A; git commit -m two; git rev-parse HEAD
> > $ git checkout -B branch
> > $ git reset --hard @~1
> > HEAD is now at 26fb965 one
> > $ git add -A; git commit -m three; git rev-parse HEAD
> > 91ef508167eb683486c3df6f8d07622b61ed698d
> >
> > $ git rev-list --objects HEAD ^master
> > 91ef508167eb683486c3df6f8d07622b61ed698d
> > ff05824d2f76436c61d2c971e11a27514aba6948
> > 8baef1b4abc478178b004d62031cf7fe6db6f903 abc
> > 086885f71429e3599c8c903b0e9ed491f6522879 bar
> > 7a67abed5f99fdd3ee203dd137b9818d88b1bafd bar/goo
>
> Thanks, but the above is not recreating the same as your original
> (where did "moo" go???).  Also "git rev-parse HEAD" for the sanity
> checking should be spelled "git rev-parse HEAD:" if you want to help
> others looking into the issue---anybody trying to reproduce will NOT
> have the same commit object name, but the point of these checks is
> to show the tree object name, which should reproduce for them.
>

It's not re-creating the exact structure I had in my original post, but it is
a shorter version which demonstrates the issue at hand.

I don't see the importance of "moo" here, the original repository structure
was something I was playing around with and found the issue.

Sorry about the misspelling, was just trying to provide a more deterministic
method of re-creating the issue. And I think that it still holds true.
In the newer
example you can see that although "branch" and "master" are TREESAME
with regards to the "bar" path, the tree for "bar" and its children
are still output
from git-rev-list(1). Even though we specify "^master".So ideally only objects
reachable from "branch" and not reachable from "master" should have been
output.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-08-16 21:00 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-15 16:44 [Bug] In `git-rev-list(1)`, using the `--objects` flag doesn't work well with the `--not` flag, as non-commit objects are not excluded Karthik Nayak
2023-08-15 19:31 ` Junio C Hamano
2023-08-15 22:11   ` Taylor Blau
2023-08-15 22:59     ` Karthik Nayak
2023-08-15 22:56   ` Karthik Nayak
2023-08-16 18:24     ` Junio C Hamano
2023-08-16 20:58       ` Karthik Nayak
2023-08-15 20:52 ` [Bug???] " Junio C Hamano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).