Re: A note on the 5.12-rc1 tag

Re: A note on the 5.12-rc1 tag

[Josh Triplett]

[CCing the git list]

On Wed, Mar 03, 2021 at 12:53:18PM -0800, Linus Torvalds wrote:
> Hey peeps - some of you may have already noticed that in my public git
> tree, the "v5.12-rc1" tag has magically been renamed to
> "v5.12-rc1-dontuse". It's still the same object, it still says
> "v5.12-rc1" internally, and it is still is signed by me, but the
> user-visible name of the tag has changed.
> 
> The reason is fairly straightforward: this merge window, we had a very
> innocuous code cleanup and simplification that raised no red flags at
> all, but had a subtle and very nasty bug in it: swap files stopped
> working right.  And they stopped working in a particularly bad way:
> the offset of the start of the swap file was lost.
> 
> Swapping still happened, but it happened to the wrong part of the
> filesystem, with the obvious catastrophic end results.
[...]
> One additional reason for this note is that I want to not just warn
> people to not run this if you have a swapfile - even if you are
> personally not impacted (like I am, and probably most people are -
> swap partitions all around) - I want to make sure that nobody starts
> new topic branches using that 5.12-rc1 tag. I know a few developers
> tend to go "Ok, rc1 is out, I got all my development work into this
> merge window, I will now fast-forward to rc1 and use that as a base
> for the next release". Don't do it this time. It may work perfectly
> well for you because you have the common partition setup, but it can
> end up being a horrible base for anybody else that might end up
> bisecting into that area.

Even if people avoid basing their topic branches on 5.12-rc1, it's still
possible for a future bisect to end up wandering to one of the existing
dangerous commits, if someone's trying to find a historical bug and git
happens to choose that as a halfway point. And if they happen to be
using a swap file, they could end up with serious data loss, years from
now when "5.12-rc1 is broken" isn't on the top of their mind or even
something they heard about originally.

Would it make sense to add a feature to git that allows defining a
"dangerous" region for bisect? Rough sketch:
- Add a `/.git-bisect-dangerous` file to the repository, containing a
  list of of commit range expressions (contains commit X, doesn't
  contain commit Y) and associated messages ("Do not use these kernels
  if you have a swap file; if you need to bisect into here, disable swap
  files first").
- git-bisect, as it navigates commits, always checks that file for any
  commit it processes, and adds any new entries it sees into
  `.git/bisect-dangerous`; it never removes entries from there.
- git-bisect avoids choosing bisection points anywhere in that range
  until it absolutely has to (because it's narrowed an issue to that
  range). This can use something similar to the existing `git bisect
  skip` machinery. Manual bisections print the message at that point.
  Automated bisections (`git bisect run`) stop and print the range
  without narrowing further, unless the user passes something like
  `--dangerous-ok=commit-range`.

(git notes would be nice for this, but they're hard to share reliably;
the above mechanism to accumulate entries from a file in the repo seems
simpler. I can imagine other possibilities.)

Does something like this seem potentially reasonable, and worth doing to
help people avoid future catastrophic data loss?


- Josh Triplett

Re: A note on the 5.12-rc1 tag

[Christian Couder]

On Fri, Mar 5, 2021 at 1:58 AM Josh Triplett <josh@joshtriplett.org> wrote:
> On Wed, Mar 03, 2021 at 12:53:18PM -0800, Linus Torvalds wrote:

> > One additional reason for this note is that I want to not just warn
> > people to not run this if you have a swapfile - even if you are
> > personally not impacted (like I am, and probably most people are -
> > swap partitions all around) - I want to make sure that nobody starts
> > new topic branches using that 5.12-rc1 tag. I know a few developers
> > tend to go "Ok, rc1 is out, I got all my development work into this
> > merge window, I will now fast-forward to rc1 and use that as a base
> > for the next release". Don't do it this time. It may work perfectly
> > well for you because you have the common partition setup, but it can
> > end up being a horrible base for anybody else that might end up
> > bisecting into that area.
>
> Even if people avoid basing their topic branches on 5.12-rc1, it's still
> possible for a future bisect to end up wandering to one of the existing
> dangerous commits, if someone's trying to find a historical bug and git
> happens to choose that as a halfway point. And if they happen to be
> using a swap file, they could end up with serious data loss, years from
> now when "5.12-rc1 is broken" isn't on the top of their mind or even
> something they heard about originally.
>
> Would it make sense to add a feature to git that allows defining a
> "dangerous" region for bisect? Rough sketch:
> - Add a `/.git-bisect-dangerous` file to the repository, containing a
>   list of of commit range expressions (contains commit X, doesn't
>   contain commit Y) and associated messages ("Do not use these kernels
>   if you have a swap file; if you need to bisect into here, disable swap
>   files first").
> - git-bisect, as it navigates commits, always checks that file for any
>   commit it processes, and adds any new entries it sees into
>   `.git/bisect-dangerous`; it never removes entries from there.

The `git bisect skip` machinery uses `refs/bisect/skip-<commit>` refs
instead of such a file, so I wonder if such a file is needed. It could
be used to store a map between skipped commits and the associated
messages though. Or git notes could be used for that purpose.

By the way I wonder what should happen if a commit is associated with
a message by a `/.git-bisect-dangerous` file, but in another branch
such file associates it with a different message. I guess all the
different messages should be stored, and then displayed.

> - git-bisect avoids choosing bisection points anywhere in that range
>   until it absolutely has to (because it's narrowed an issue to that
>   range). This can use something similar to the existing `git bisect
>   skip` machinery. Manual bisections print the message at that point.
>   Automated bisections (`git bisect run`) stop and print the range
>   without narrowing further, unless the user passes something like
>   `--dangerous-ok=commit-range`.

Yeah, using the `git bisect skip` machinery looks like a good idea.
Instead of `/.git-bisect-dangerous`, the file could actually be called
`/.git-bisect-skip` and could also store ranges where the code doesn't
compile, or completely misbehave, without necessarily being dangerous.
The dangerous status would only be conveyed by the associated messages
then.

Another way could be to directly share some special refs similar to
the existing `refs/bisect/skip-<commit>` refs, instead of a
`/.git-bisect-dangerous` file. This would likely raise some issues
about how to create and share these refs and the associated messages
though.

> (git notes would be nice for this, but they're hard to share reliably;
> the above mechanism to accumulate entries from a file in the repo seems
> simpler. I can imagine other possibilities.)

If the notes are created automatically from the `/.git-bisect-skip`
files and stored in `refs/notes/skip`, then they might not need to be
shared. If people already share notes, they would just need to stop
sharing those in `refs/notes/skip`.

> Does something like this seem potentially reasonable, and worth doing to
> help people avoid future catastrophic data loss?

It seems reasonable as part of the skip mechanism.

Re: A note on the 5.12-rc1 tag

[Junio C Hamano]

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

>> (git notes would be nice for this, but they're hard to share reliably;
>> the above mechanism to accumulate entries from a file in the repo seems
>> simpler. I can imagine other possibilities.)
>
> If the notes are created automatically from the `/.git-bisect-skip`
> files and stored in `refs/notes/skip`, then they might not need to be
> shared. If people already share notes, they would just need to stop
> sharing those in `refs/notes/skip`.

Ehh, doesn't Josh _want_ to share them, though?  I do not know if a
single "refs/notes/bisect-skip" notes would do, or you'd need one
notes tree per the kind of bisection (iow, people may be chasing
different set of bugs, and the commits that need to be skipped while
chasing one bug may be OK to test while chasing another one), but I
would imagine that for this particular use case of marking "these
commits are dangerous to check out and build on", it does not depend
on what you are bisecting to find at all, so sharing would be a
sensible thing to do.

It is trivial for you to fetch the refs/notes/do-not--checkout notes
tree from me and merge it into your refs/notes/do-not--checkout
notes tree, I would think; "git notes merge" may have room for
improvement, but essentially it would just want a union of two
sets, no?

Re: A note on the 5.12-rc1 tag

[Josh Triplett]

On Fri, Mar 05, 2021 at 10:10:05AM -0800, Junio C Hamano wrote:
> Christian Couder <christian.couder@gmail.com> writes:
> 
> >> (git notes would be nice for this, but they're hard to share reliably;
> >> the above mechanism to accumulate entries from a file in the repo seems
> >> simpler. I can imagine other possibilities.)
> >
> > If the notes are created automatically from the `/.git-bisect-skip`
> > files and stored in `refs/notes/skip`, then they might not need to be
> > shared. If people already share notes, they would just need to stop
> > sharing those in `refs/notes/skip`.
> 
> Ehh, doesn't Josh _want_ to share them, though?  I do not know if a
> single "refs/notes/bisect-skip" notes would do, or you'd need one
> notes tree per the kind of bisection (iow, people may be chasing
> different set of bugs, and the commits that need to be skipped while
> chasing one bug may be OK to test while chasing another one), but I
> would imagine that for this particular use case of marking "these
> commits are dangerous to check out and build on", it does not depend
> on what you are bisecting to find at all, so sharing would be a
> sensible thing to do.
> 
> It is trivial for you to fetch the refs/notes/do-not--checkout notes
> tree from me and merge it into your refs/notes/do-not--checkout
> notes tree, I would think; "git notes merge" may have room for
> improvement, but essentially it would just want a union of two
> sets, no?

My primary concern about notes is that they require manual
action/configuration in order to share. I was looking for a solution
that would automatically protect anyone who pulled from linux.git
(directly or indirectly), without them having to specifically take a
separate step to sync this information.

Re: A note on the 5.12-rc1 tag

[Junio C Hamano]

Josh Triplett <josh@joshtriplett.org> writes:

>> It is trivial for you to fetch the refs/notes/do-not--checkout notes
>> tree from me and merge it into your refs/notes/do-not--checkout
>> notes tree, I would think; "git notes merge" may have room for
>> improvement, but essentially it would just want a union of two
>> sets, no?
>
> My primary concern about notes is that they require manual
> action/configuration in order to share. I was looking for a solution
> that would automatically protect anyone who pulled from linux.git
> (directly or indirectly), without them having to specifically take a
> separate step to sync this information.

If "without any configuration" is a hard requirement, then by
definition you'd need to live with what you get from "git clone" and
"git pull" alone, so be it the notes or any other mechanism, there
currently is nothing that lets you do the "skip this part while
bisection".