All of lore.kernel.org
 help / color / mirror / Atom feed
* Allow git bisect to auto-skip
@ 2024-03-22 22:18 Olliver Schinagl
  2024-03-22 22:31 ` Junio C Hamano
  0 siblings, 1 reply; 14+ messages in thread
From: Olliver Schinagl @ 2024-03-22 22:18 UTC (permalink / raw)
  To: git

In some cases, we know a commit will always break bisect. This is bad 
and evil but sometimes needed.

An example, in my particular case, when OpenWRT does a kernel bump, 
patches and config files get copied to the new version. This is fine, 
but there is not git copy (one could also see that as an individual 
feature request which would solve the actual issue for now :p). Since 
the files are copied, git treats them as new files, without history. 
This makes tracking history almost impossible (config-4.14 -> config 
5.15 -> config 6.6 etc). People have found out, there's some tricks to 
apply where we can make git not see the copy as such, and we'll have our 
history of both files [0]. In short, this hack is used to create two 
commits.

git checkout -b _tmp
git mv old new
git commit -m 'copy old to new' -m 'GIT_SKIP_BISECT'
git checkout HEAD~ old
git commit -m 'restore old'
git switch _tmp
git merge _tmp

and to remove the ugly merge commit

git rebase HEAD~1

which surely is a hack :p

But, it's a `git cp` if you will.

Now this does break git bisect, when the commit with the move comes in, 
things are broken until the next commit. It would be very nice if we can 
have a special marker keyword (GIT_SKIP_BISECT) in the commit message 
for example, to force git bisect to just skip this commit. It will fail, 
it is designed to fail.

There's probably also other very useful cases, but this is just one example.

And hey, if this leads to two patches, where a second one introduces 
`git cp` I'd be even more happy :)

Olliver

[0]: https://github.com/openwrt/openwrt/blob/main/scripts/kernel_bump.sh

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

* Re: Allow git bisect to auto-skip
  2024-03-22 22:18 Allow git bisect to auto-skip Olliver Schinagl
@ 2024-03-22 22:31 ` Junio C Hamano
  2024-03-23  1:59   ` Olliver Schinagl
  2024-03-23 13:51   ` Stefan Haller
  0 siblings, 2 replies; 14+ messages in thread
From: Junio C Hamano @ 2024-03-22 22:31 UTC (permalink / raw)
  To: Olliver Schinagl; +Cc: git

Olliver Schinagl <oliver@schinagl.nl> writes:

> In some cases, we know a commit will always break bisect. This is bad
> and evil but sometimes needed.
> ...
> git commit -m 'copy old to new' -m 'GIT_SKIP_BISECT'
> ...

If "I want a bisect to skip any commit that has 'Skip Me' in its
subject" is the case, perhaps your "git bisect run" script can say

    #!/bin/sh

    case "$(git show -s --oneline)" in
    *"Skip Me"*) exit 125 ;;
    esac

    ... your test script body comes here ...
    if test successful
    then
        exit 0
    else
        exit 1
    fi

The _clue_ to mark a commit to be skipped does not have to be
hardcoded commit title.  It often is discovered that a commit
breaks bisection after the fact and it is not feasible to rebase
all the history after the commit.  Maybe an approach more suitable
in such a situation would attach a note to such untestable commits
after the fact, and check if such a note is attached at the
beginning of "git bisect run" script and exit with 125.

And a new "git bisect --skip-when <condition>" option can be added to
manual bisection process.  The <condition> part would contain
something like

    case "$(git show -s --oneline)" in
    *"Skip Me"*) exit 125 ;;
    esac

taken from the above illustration.

But I am not sure what end result you are trying to achieve, so the
above are random collection of ideas.  Turning them into a patch is
left as an exercise to readers.




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

* Re: Allow git bisect to auto-skip
  2024-03-22 22:31 ` Junio C Hamano
@ 2024-03-23  1:59   ` Olliver Schinagl
  2024-03-23 13:51   ` Stefan Haller
  1 sibling, 0 replies; 14+ messages in thread
From: Olliver Schinagl @ 2024-03-23  1:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Hey Junio,

On March 22, 2024 11:31:29 p.m. GMT+01:00, Junio C Hamano <gitster@pobox.com> wrote:
>Olliver Schinagl <oliver@schinagl.nl> writes:
>
>> In some cases, we know a commit will always break bisect. This is bad
>> and evil but sometimes needed.
>> ...
>> git commit -m 'copy old to new' -m 'GIT_SKIP_BISECT'
>> ...
>
>If "I want a bisect to skip any commit that has 'Skip Me' in its
>subject" is the case, perhaps your "git bisect run" script can say
>
>    #!/bin/sh
>
>    case "$(git show -s --oneline)" in
>    *"Skip Me"*) exit 125 ;;
>    esac
>
>    ... your test script body comes here ...
>    if test successful
>    then
>        exit 0
>    else
>        exit 1
>    fi
>

This is a nice way to wrap got bisect, but from a UX point of view, having native support from the got client would be much nicer. E.g. the user doesn't have to learn about special scripts. I'd argue, if we can do it the same way everywhere (I.e. shipping such a script as part of the got distro), why not handle it nativily.

The magic word could be a default with an override in the gitconfig.

>The _clue_ to mark a commit to be skipped does not have to be
>hardcoded commit title.  It often is discovered that a commit
>breaks bisection after the fact and it is not feasible to rebase
>all the history after the commit.  Maybe an approach more suitable
>in such a situation would attach a note to such untestable commits
>after the fact, and check if such a note is attached at the
>beginning of "git bisect run" script and exit with 125.
>
>And a new "git bisect --skip-when <condition>" option can be added to
>manual bisection process.  The <condition> part would contain
>something like
>
>    case "$(git show -s --oneline)" in
>    *"Skip Me"*) exit 125 ;;
>    esac
>
>taken from the above illustration.
>

I've read the notes solution from 13 years ago ;) and I do like the elegance. But I think there's two cases. One during the fact, when you know this will be an issue andere in deed ons after the fact.

I'll admit, I was oblivious by git notes, and will read up on it now. Always using a note for this feature does make sense, as its part of the repository.


>But I am not sure what end result you are trying to achieve, so the
>above are random collection of ideas.  Turning them into a patch is
>left as an exercise to readers.

In the end, is it (internally) even possible for got bisect to check this, and would a patch that handles this 'behind the scenes' be accepted, without the user having to install special tooling (scripts)?

Having said that, on a different note is 'git copy' feasable? E.g. git CP old new, where both files share the same history?

Olliver

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

* Re: Allow git bisect to auto-skip
  2024-03-22 22:31 ` Junio C Hamano
  2024-03-23  1:59   ` Olliver Schinagl
@ 2024-03-23 13:51   ` Stefan Haller
  2024-03-23 18:43     ` Junio C Hamano
  1 sibling, 1 reply; 14+ messages in thread
From: Stefan Haller @ 2024-03-23 13:51 UTC (permalink / raw)
  To: Junio C Hamano, Olliver Schinagl; +Cc: git

On 22.03.24 23:31, Junio C Hamano wrote:
> It often is discovered that a commit
> breaks bisection after the fact and it is not feasible to rebase
> all the history after the commit.

This reminds me of a similar problem with git blame, for which we have
the blame.ignoreRevsFile config to work around it. Couldn't there be a
similar mechanism for bisect, e.g. bisect.skipRevsFile?

-Stefan


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

* Re: Allow git bisect to auto-skip
  2024-03-23 13:51   ` Stefan Haller
@ 2024-03-23 18:43     ` Junio C Hamano
  2024-03-23 20:51       ` Olliver Schinagl
  2024-03-24 10:16       ` Stefan Haller
  0 siblings, 2 replies; 14+ messages in thread
From: Junio C Hamano @ 2024-03-23 18:43 UTC (permalink / raw)
  To: Stefan Haller; +Cc: Olliver Schinagl, git

Stefan Haller <lists@haller-berlin.de> writes:

> On 22.03.24 23:31, Junio C Hamano wrote:
>> It often is discovered that a commit
>> breaks bisection after the fact and it is not feasible to rebase
>> all the history after the commit.
>
> This reminds me of a similar problem with git blame, for which we have
> the blame.ignoreRevsFile config to work around it. Couldn't there be a
> similar mechanism for bisect, e.g. bisect.skipRevsFile?

A Very good point.  If a breakage of a commit is "this does not even
build" kind of breakage, such a mechanism would be an excellent fit.

But if a breakage is "only this particular test fails and we know
the reason why it fails has nothing to do with the bug we are
chasing", then compiling such a fixed list of commits, or pointing
at such a list with a configuration variable, would not work very
well, I am afraid.

Thanks.

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

* Re: Allow git bisect to auto-skip
  2024-03-23 18:43     ` Junio C Hamano
@ 2024-03-23 20:51       ` Olliver Schinagl
  2024-03-24  7:47         ` Olliver Schinagl
  2024-03-24 10:16       ` Stefan Haller
  1 sibling, 1 reply; 14+ messages in thread
From: Olliver Schinagl @ 2024-03-23 20:51 UTC (permalink / raw)
  To: Junio C Hamano, Stefan Haller; +Cc: git

On 23-03-2024 19:43, Junio C Hamano wrote:
> Stefan Haller <lists@haller-berlin.de> writes:
> 
>> On 22.03.24 23:31, Junio C Hamano wrote:
>>> It often is discovered that a commit
>>> breaks bisection after the fact and it is not feasible to rebase
>>> all the history after the commit.
>>
>> This reminds me of a similar problem with git blame, for which we have
>> the blame.ignoreRevsFile config to work around it. Couldn't there be a
>> similar mechanism for bisect, e.g. bisect.skipRevsFile?
> 
> A Very good point.  If a breakage of a commit is "this does not even
> build" kind of breakage, such a mechanism would be an excellent fit.
> 
> But if a breakage is "only this particular test fails and we know
> the reason why it fails has nothing to do with the bug we are
> chasing", then compiling such a fixed list of commits, or pointing
> at such a list with a configuration variable, would not work very
> well, I am afraid.

This changes my view of the issue a little bit. Building vs testing, 
though in reality its the same of course.

I can totally see that a user would need a special bisect script to 
handle these cases 'this commit breaks test 54, 43 and 12; but the rest 
work'. This is too specific to handle generically. Probably in using a 
similar trick, store in the notes what works and does not work.

I think it still stands that having a 'generic' way of telling bisect to 
skip the entire commit is still reasonable. Kind of like how you can do 
`git push -o ci.skip=true` with GitLab.

Olliver

> 
> Thanks.

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

* Re: Allow git bisect to auto-skip
  2024-03-23 20:51       ` Olliver Schinagl
@ 2024-03-24  7:47         ` Olliver Schinagl
  0 siblings, 0 replies; 14+ messages in thread
From: Olliver Schinagl @ 2024-03-24  7:47 UTC (permalink / raw)
  To: Junio C Hamano, Stefan Haller; +Cc: git

On 23-03-2024 21:51, Olliver Schinagl wrote:
> On 23-03-2024 19:43, Junio C Hamano wrote:
>> Stefan Haller <lists@haller-berlin.de> writes:
>>
>>> On 22.03.24 23:31, Junio C Hamano wrote:
>>>> It often is discovered that a commit
>>>> breaks bisection after the fact and it is not feasible to rebase
>>>> all the history after the commit.
>>>
>>> This reminds me of a similar problem with git blame, for which we have
>>> the blame.ignoreRevsFile config to work around it. Couldn't there be a
>>> similar mechanism for bisect, e.g. bisect.skipRevsFile?
>>
>> A Very good point.  If a breakage of a commit is "this does not even
>> build" kind of breakage, such a mechanism would be an excellent fit.
>>
>> But if a breakage is "only this particular test fails and we know
>> the reason why it fails has nothing to do with the bug we are
>> chasing", then compiling such a fixed list of commits, or pointing
>> at such a list with a configuration variable, would not work very
>> well, I am afraid.
> 
> This changes my view of the issue a little bit. Building vs testing, 
> though in reality its the same of course.
> 
> I can totally see that a user would need a special bisect script to 
> handle these cases 'this commit breaks test 54, 43 and 12; but the rest 
> work'. This is too specific to handle generically. Probably in using a 
> similar trick, store in the notes what works and does not work.

P.S. One (imo big) issue to me is, that notes aren't automatically 
fetched or pushed according to what I found so far online (haven't 
personally tested it yet).

If that is the case, this makes the feature to be used in combination 
with bisect much more difficult for the un-informed. having a 
`refs/notes/<bisect-or-like-related-name>` notes tree is great, but only 
if it gets automatically pulled.

I think this also might be the reason it is a very undervalued thing maybe?

Olliver

> 
> I think it still stands that having a 'generic' way of telling bisect to 
> skip the entire commit is still reasonable. Kind of like how you can do 
> `git push -o ci.skip=true` with GitLab.
> 
> Olliver
> 
>>
>> Thanks.

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

* Re: Allow git bisect to auto-skip
  2024-03-23 18:43     ` Junio C Hamano
  2024-03-23 20:51       ` Olliver Schinagl
@ 2024-03-24 10:16       ` Stefan Haller
  2024-03-24 14:29         ` Christian Couder
  1 sibling, 1 reply; 14+ messages in thread
From: Stefan Haller @ 2024-03-24 10:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Olliver Schinagl, git

On 23.03.24 19:43, Junio C Hamano wrote:
> Stefan Haller <lists@haller-berlin.de> writes:
> 
>> On 22.03.24 23:31, Junio C Hamano wrote:
>>> It often is discovered that a commit
>>> breaks bisection after the fact and it is not feasible to rebase
>>> all the history after the commit.
>>
>> This reminds me of a similar problem with git blame, for which we have
>> the blame.ignoreRevsFile config to work around it. Couldn't there be a
>> similar mechanism for bisect, e.g. bisect.skipRevsFile?
> 
> A Very good point.  If a breakage of a commit is "this does not even
> build" kind of breakage, such a mechanism would be an excellent fit.
> 
> But if a breakage is "only this particular test fails and we know
> the reason why it fails has nothing to do with the bug we are
> chasing", then compiling such a fixed list of commits, or pointing
> at such a list with a configuration variable, would not work very
> well, I am afraid.

That's true, but the same can be said about blame.ignoreRevsFile. There
may be commits that contain both uninteresting whitespace changes and
real changes (not in a well-maintained project of course :-), so it
wouldn't be a good idea to add those to blame.ignoreRevsFile. But that's
not a reason not to offer the feature at all.

-Stefan

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

* Re: Allow git bisect to auto-skip
  2024-03-24 10:16       ` Stefan Haller
@ 2024-03-24 14:29         ` Christian Couder
  2024-03-24 16:04           ` rsbecker
  2024-03-24 18:34           ` Junio C Hamano
  0 siblings, 2 replies; 14+ messages in thread
From: Christian Couder @ 2024-03-24 14:29 UTC (permalink / raw)
  To: Stefan Haller; +Cc: Junio C Hamano, Olliver Schinagl, git

On Sun, Mar 24, 2024 at 11:16 AM Stefan Haller <lists@haller-berlin.de> wrote:
>
> On 23.03.24 19:43, Junio C Hamano wrote:
> > Stefan Haller <lists@haller-berlin.de> writes:
> >
> >> On 22.03.24 23:31, Junio C Hamano wrote:
> >>> It often is discovered that a commit
> >>> breaks bisection after the fact and it is not feasible to rebase
> >>> all the history after the commit.
> >>
> >> This reminds me of a similar problem with git blame, for which we have
> >> the blame.ignoreRevsFile config to work around it. Couldn't there be a
> >> similar mechanism for bisect, e.g. bisect.skipRevsFile?
> >
> > A Very good point.  If a breakage of a commit is "this does not even
> > build" kind of breakage, such a mechanism would be an excellent fit.
> >
> > But if a breakage is "only this particular test fails and we know
> > the reason why it fails has nothing to do with the bug we are
> > chasing", then compiling such a fixed list of commits, or pointing
> > at such a list with a configuration variable, would not work very
> > well, I am afraid.
>
> That's true, but the same can be said about blame.ignoreRevsFile. There
> may be commits that contain both uninteresting whitespace changes and
> real changes (not in a well-maintained project of course :-), so it
> wouldn't be a good idea to add those to blame.ignoreRevsFile. But that's
> not a reason not to offer the feature at all.

I am not against the feature, but I think it would be especially
useful if the file(s) containing the revs that should be skipped
is(are) tracked in Git. In this case though, any such file wouldn't be
used automatically after cloning the repo as the bisect.skipRevsFile
option would still need to be configured.

Also, how much better would this be compared to tracking  "git bisect
run" scripts in the repo, even if they have to be copied somewhere
else before they are launched? I wonder about this because writing the
conditions that decide whether the current commit is good or bad might
not be so easy either. So if the goal is to simplify things for users,
then simplifying all the way by providing example scripts with
comments about how they could be customized might be even better.

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

* RE: Allow git bisect to auto-skip
  2024-03-24 14:29         ` Christian Couder
@ 2024-03-24 16:04           ` rsbecker
  2024-03-24 18:34           ` Junio C Hamano
  1 sibling, 0 replies; 14+ messages in thread
From: rsbecker @ 2024-03-24 16:04 UTC (permalink / raw)
  To: 'Christian Couder', 'Stefan Haller'
  Cc: 'Junio C Hamano', 'Olliver Schinagl', git

On Sunday, March 24, 2024 10:29 AM, Christian Couder wrote:
>On Sun, Mar 24, 2024 at 11:16 AM Stefan Haller <lists@haller-berlin.de> wrote:
>>
>> On 23.03.24 19:43, Junio C Hamano wrote:
>> > Stefan Haller <lists@haller-berlin.de> writes:
>> >
>> >> On 22.03.24 23:31, Junio C Hamano wrote:
>> >>> It often is discovered that a commit breaks bisection after the
>> >>> fact and it is not feasible to rebase all the history after the
>> >>> commit.
>> >>
>> >> This reminds me of a similar problem with git blame, for which we
>> >> have the blame.ignoreRevsFile config to work around it. Couldn't
>> >> there be a similar mechanism for bisect, e.g. bisect.skipRevsFile?
>> >
>> > A Very good point.  If a breakage of a commit is "this does not even
>> > build" kind of breakage, such a mechanism would be an excellent fit.
>> >
>> > But if a breakage is "only this particular test fails and we know
>> > the reason why it fails has nothing to do with the bug we are
>> > chasing", then compiling such a fixed list of commits, or pointing
>> > at such a list with a configuration variable, would not work very
>> > well, I am afraid.
>>
>> That's true, but the same can be said about blame.ignoreRevsFile.
>> There may be commits that contain both uninteresting whitespace
>> changes and real changes (not in a well-maintained project of course
>> :-), so it wouldn't be a good idea to add those to
>> blame.ignoreRevsFile. But that's not a reason not to offer the feature at all.
>
>I am not against the feature, but I think it would be especially useful if the file(s)
>containing the revs that should be skipped
>is(are) tracked in Git. In this case though, any such file wouldn't be used
>automatically after cloning the repo as the bisect.skipRevsFile option would still
>need to be configured.
>
>Also, how much better would this be compared to tracking  "git bisect run" scripts in
>the repo, even if they have to be copied somewhere else before they are launched?
>I wonder about this because writing the conditions that decide whether the current
>commit is good or bad might not be so easy either. So if the goal is to simplify things
>for users, then simplifying all the way by providing example scripts with comments
>about how they could be customized might be even better.

In some situations, git bisect is used for compile issues, while others in debugging. In development branches, bisect is sufficient. However, if one has multiple production deploy branches, where feature merge squash happens multiple times into multiple branches, this makes finding the offending commit using bisect rather difficult as the defect may come in and out depending on the order in which feature branches are squashed (some may have the bug, while others don't). I can see the point of teaching bisect to knowingly skipping specific versions in this setting, although it is arguable that the investigation probably should not happen on the destination branches where functional stability cannot be guaranteed.

But this brought up a thought. What if bisect had the notion of parallel context: Meaning running two (or more) bisects in parallel on the same repo in search of two semi-independent defects, then analysing convergence between the two analyses - something like

 bisect 1: A(good) B(good) C(bad) D(bad);
 bisect 2: A(good) B(good) C(good) D(bad);
 bisect 3: A(good) B(bad) C(good) D(bad),   <- a production-like branch

with an interpretation that C and D may have interrelated defects but started showing in B in one situation. This is not a well-baked thought, but I can see this having some relevance when sub-modules are involved and bugs exist in more than one sub-module with bisects happening in the main repo. [note: I am not certain where I am going with this idea as of yet].

--Randall


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

* Re: Allow git bisect to auto-skip
  2024-03-24 14:29         ` Christian Couder
  2024-03-24 16:04           ` rsbecker
@ 2024-03-24 18:34           ` Junio C Hamano
  2024-03-27 18:33             ` Olliver Schinagl
  1 sibling, 1 reply; 14+ messages in thread
From: Junio C Hamano @ 2024-03-24 18:34 UTC (permalink / raw)
  To: Christian Couder; +Cc: Stefan Haller, Olliver Schinagl, git

Christian Couder <christian.couder@gmail.com> writes:

> Also, how much better would this be compared to tracking  "git bisect
> run" scripts in the repo, even if they have to be copied somewhere
> else before they are launched? I wonder about this because writing the
> conditions that decide whether the current commit is good or bad might
> not be so easy either. So if the goal is to simplify things for users,
> then simplifying all the way by providing example scripts with
> comments about how they could be customized might be even better.

If we are driving our bisection session via "bisect run" script,
computing the condition that we need to skip in the script is the
most natural and obvious thing to do, but the way I guessed (because
it was not explicitly written down) what the OP wanted was a way for
bisect_next() called after even a manual "git bisect (good|bad)" to
automatically skip certain set of commits.  Because there are cases
where you have to be testing manually and cannot afford to write
"bisect run" script, giving a manual bisection a way to compute if a
commit need to be skipped may be worth having, and that was where my
"git bisect --skip-when <script>" came from.  It would not be
necessary if you are doing "bisect run", which can dynamically tell
if the commit is untestable.  And if the user is going to decide
after manually testing the one that is suggested, it is not useful
either, as the point would be to avoid even asking to test ones that
need to be skipped.  So it is likely that the set of commits that
need skipping is known a-priori before the bisect session even
begins.  The end user experience may look like:

 * "git bisect start" takes "--skip-when <script>" and remembers it,
   together with other options "start" can take (like <good>, <bad>,
   <terms>, <pathspec>).

 - If <good> and <bad> are already given upon "start", the command
   may check out a revision and ask you to test.

 * Every time the command checks out a revision to be tested by the
   user, the command guarantees if it satisfies the --skip-when
   condition (and internally doing "git bisect skip").

 * Otherwise the interaction between the user and the session is
   exactly the same as usual.


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

* Re: Allow git bisect to auto-skip
  2024-03-24 18:34           ` Junio C Hamano
@ 2024-03-27 18:33             ` Olliver Schinagl
  2024-03-27 19:24               ` Junio C Hamano
  0 siblings, 1 reply; 14+ messages in thread
From: Olliver Schinagl @ 2024-03-27 18:33 UTC (permalink / raw)
  To: Junio C Hamano, Christian Couder; +Cc: Stefan Haller, git

On 24-03-2024 19:34, Junio C Hamano wrote:
> Christian Couder <christian.couder@gmail.com> writes:
> 
>> Also, how much better would this be compared to tracking  "git bisect
>> run" scripts in the repo, even if they have to be copied somewhere
>> else before they are launched? I wonder about this because writing the
>> conditions that decide whether the current commit is good or bad might
>> not be so easy either. So if the goal is to simplify things for users,
>> then simplifying all the way by providing example scripts with
>> comments about how they could be customized might be even better.
> 
> If we are driving our bisection session via "bisect run" script,
> computing the condition that we need to skip in the script is the
> most natural and obvious thing to do, but the way I guessed (because
> it was not explicitly written down) what the OP wanted was a way for
> bisect_next() called after even a manual "git bisect (good|bad)" to
> automatically skip certain set of commits.

I think you understood what I failed to properly explain :)

To come up with an RFC, I was trying to study the git code, and while 
some things are quite readable, others are a bit complex.

After some poking, I was thinking of using `find_bisection()`, or rather 
`do_find_bisection()` but got lost there.

What made sense initially was, that in `find_bisection()` there is a 
simple for loop that goes over the list of commits to count them. But 
writing this down, I realize the list of commits is already there in 
`struct commit_list *list`, so I should probably go find out where the 
list is being created!

Anyway, want I was thinking of, based a key somewhere in the message 
body (GIT_BISECT_SKIP=1 for example), mark the commit in the list to be 
skipped, as `git bisect skip` would. This so the skipped commit actualyl 
ends up on the list of skipped commits (`refs/bisect/skip-*`).

But being a bit lost, it would be nice to get some directional pointers.

Is this even possible? What is the easiest way to get the message from 
the `struct commit_list *list` item, are there helpers to parse the 
message at all?

Thanks,
Olliver

>  Because there are cases
> where you have to be testing manually and cannot afford to write
> "bisect run" script, giving a manual bisection a way to compute if a
> commit need to be skipped may be worth having, and that was where my
> "git bisect --skip-when <script>" came from.  It would not be
> necessary if you are doing "bisect run", which can dynamically tell
> if the commit is untestable.  And if the user is going to decide
> after manually testing the one that is suggested, it is not useful
> either, as the point would be to avoid even asking to test ones that
> need to be skipped.  So it is likely that the set of commits that
> need skipping is known a-priori before the bisect session even
> begins.  The end user experience may look like:
> 
>   * "git bisect start" takes "--skip-when <script>" and remembers it,
>     together with other options "start" can take (like <good>, <bad>,
>     <terms>, <pathspec>).
> 
>   - If <good> and <bad> are already given upon "start", the command
>     may check out a revision and ask you to test.
> 
>   * Every time the command checks out a revision to be tested by the
>     user, the command guarantees if it satisfies the --skip-when
>     condition (and internally doing "git bisect skip").
> 
>   * Otherwise the interaction between the user and the session is
>     exactly the same as usual.
> 

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

* Re: Allow git bisect to auto-skip
  2024-03-27 18:33             ` Olliver Schinagl
@ 2024-03-27 19:24               ` Junio C Hamano
  2024-03-28  8:01                 ` Olliver Schinagl
  0 siblings, 1 reply; 14+ messages in thread
From: Junio C Hamano @ 2024-03-27 19:24 UTC (permalink / raw)
  To: Olliver Schinagl; +Cc: Christian Couder, Stefan Haller, git

Olliver Schinagl <oliver@schinagl.nl> writes:

> Anyway, want I was thinking of, based a key somewhere in the message
> body (GIT_BISECT_SKIP=1 for example), mark the commit in the list to
> be skipped, as `git bisect skip` would. This so the skipped commit
> actualyl ends up on the list of skipped commits
> (`refs/bisect/skip-*`).

I think it is very unlikely that we'd adopt any hardcoded scheme
that allows only one supported way of marking commits as not to be
tested.  Certainly not the kind that the committer is forced to know
the commit must be skipped during a bisection session before
creating the commit.  So I am not sure how much good it will do to
know that commit_list->item is a commit object on the list, or
calling repo_get_commit_buffer() on it would give the contents of
the commit object.

Knowing that commit_list->item->object.oid is the object name of
such a commit, and calling oid_to_hex() on such an object name
stringifies it, should be sufficient to write a new code that calls
out to a script specified via "git bisect --skip-when=<script>"
using the run_command() API to decide if the commit object should be
skipped, and if you do want GIT_BISECT_SKIP=1 in the log message,
your script given via --skip-when may be a single-liner:

    #!/bin/sh
    # usage: this-script $commit_object_name
    # expected to exit with 0 when the commit should not be tested
    git cat-file commit "$1" | sed -e '1,/^$/d' | grep GIT_BISECT_SKIP=1

Thanks.

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

* Re: Allow git bisect to auto-skip
  2024-03-27 19:24               ` Junio C Hamano
@ 2024-03-28  8:01                 ` Olliver Schinagl
  0 siblings, 0 replies; 14+ messages in thread
From: Olliver Schinagl @ 2024-03-28  8:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Christian Couder, Stefan Haller, git

On 27-03-2024 20:24, Junio C Hamano wrote:
> Olliver Schinagl <oliver@schinagl.nl> writes:
>
>> Anyway, want I was thinking of, based a key somewhere in the message
>> body (GIT_BISECT_SKIP=1 for example), mark the commit in the list to
>> be skipped, as `git bisect skip` would. This so the skipped commit
>> actualyl ends up on the list of skipped commits
>> (`refs/bisect/skip-*`).
> I think it is very unlikely that we'd adopt any hardcoded scheme
> that allows only one supported way of marking commits as not to be
> tested.  Certainly not the kind that the committer is forced to know
> the commit must be skipped during a bisection session before
> creating the commit.

My goal is to make things work as automagically as possible, without 
special tooling or scripts. This to make the barrier for entry as low as 
possible. E.g. if someone says 'could you please bisect this for me', 
things should just work.

Having said that, I also realize that the use-cases where a commit 
message would be enough are limited. There's only a very rare cases 
where it is known before hand a commit will break bisect.

The `GIT_BISECT_SKIP=1` would obviously be configurable, so it could be 
set on a per-repo level, with a sane default, and probably wiser to use 
`git commit --skip-bisect` to make use of said flag.


Having said all that, right now, I'm just exploring how things work, how 
easy it would be, how logical it would be. So for thank you for bearing 
with me :)

>    So I am not sure how much good it will do to
> know that commit_list->item is a commit object on the list, or
> calling repo_get_commit_buffer() on it would give the contents of
> the commit object.
>
> Knowing that commit_list->item->object.oid is the object name of
> such a commit, and calling oid_to_hex() on such an object name
> stringifies it, should be sufficient to write a new code that calls
> out to a script specified via "git bisect --skip-when=<script>"

I had learned about `oid_to_hex()` already, and I am charmed by that 
idea for sure. So much in that I'll look into this first. In the end, 
both solutions could even be used as they are in nearly the same spot 
I'd think.


Four things I lack right now. The first one I hope to have figured out 
before your reply (but don't shy the reply).

1) What is the best way to parse the skip-when argument. I see 
`bisect_start()` does arg parsing, but trying to see what 'the git way' 
is to get the argument of the new flag.

2) Where to store the argument. `bisect_terms` is passed around 
everywhere to hold the good and bad states, but with bisect doesn't pass 
much of anything else around. Though `bisect_run` isn't storing 
`command` either, and you do mention so I'll delve into that and see how 
that maps into the `run_command()` API.

3) Using the `run_command()` API implies to do the check 'on checkout'; 
which was my first idea as well, but a) where does this happen? I have 
not found this yet, though `bisect_run` might be more revealing here,

3) Using this approach, can I still do a 'regular' `git bisect skip` in 
that the skip gets recorded (in refs/bisect/skip-*)? I think it would be 
nice to see what was skipped.


Thanks,

Olliver

> using the run_command() API to decide if the commit object should be
> skipped, and if you do want GIT_BISECT_SKIP=1 in the log message,
> your script given via --skip-when may be a single-liner:
>
>      #!/bin/sh
>      # usage: this-script $commit_object_name
>      # expected to exit with 0 when the commit should not be tested
>      git cat-file commit "$1" | sed -e '1,/^$/d' | grep GIT_BISECT_SKIP=1
>
> Thanks.



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

end of thread, other threads:[~2024-03-28  8:01 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-22 22:18 Allow git bisect to auto-skip Olliver Schinagl
2024-03-22 22:31 ` Junio C Hamano
2024-03-23  1:59   ` Olliver Schinagl
2024-03-23 13:51   ` Stefan Haller
2024-03-23 18:43     ` Junio C Hamano
2024-03-23 20:51       ` Olliver Schinagl
2024-03-24  7:47         ` Olliver Schinagl
2024-03-24 10:16       ` Stefan Haller
2024-03-24 14:29         ` Christian Couder
2024-03-24 16:04           ` rsbecker
2024-03-24 18:34           ` Junio C Hamano
2024-03-27 18:33             ` Olliver Schinagl
2024-03-27 19:24               ` Junio C Hamano
2024-03-28  8:01                 ` Olliver Schinagl

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.