Re: Policy proposal (draft): Don't push knowingly broken or work-in-progress work to dist git

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 08/02/21 16:28 +0100, Kevin Kofler via devel wrote:
Jonathan Wakely wrote:
You've completely misunderstood the suggestion.

No, sorry, I understood it completely. I think that you are the one who
completely misunderstood my objection, and also missed the point of the
original post. Let me explain:

I'm suggesting that everybody pushes to rawhide just like today.
Maintainers and provenpackagers. Nothing changes in that respect. There is
a read-only branch called rawhide-build which contains the
last-known-to-build commit from rawhide. Nobody can push to rawhide-build,
it only gets updated automatically when a build completes in koji.

Yes, I got that.

But how does that address the point of the OP that provenpackagers need the
rawhide branch to be buildable so they can *automatically* bump and commit
to it?

Let's say that I am a provenpackager (which I happen to actually be) and
that I want to do a scripted mass-rebuild (which is unlikely to happen in my
case, but other provenpackagers have that need, so let's please assume it).
What value does a read-only rawhide-build branch give me? It just tells me
what was the last commit that built, an information I can just get directly
from Koji. Having this in git does not help me in any way.

How do you get that directly from koji in a script?

Is it as easy as a single git command to check if rawhide and
rawhide-build are the same ref?

It looks to me like I need to parse the build ID from the output of
"koji latest-build <tag> <name>" and then use "koji buildinfo <id>"
and then parse that for the Source: line. Is there a better way?

If a maintainer wants to fix the broken state they created, they just
fix it locally and push. There's no need to reset or merge in the
revert that happened upstream (because that would be extra work for
them, and non-trivial to do right for devs who aren't comfortable with
Git beyond the basics).

And that is exactly the same as the status quo, no rawhide-build branch
needed.

If a provenpackager needs to bump and rebuild they have choices (but
none of them involve touching rawhide-build as that's read-only):

So who needs rawhide-build then?

1) Do nothing. The package is broken anyway. The maintainer needs to
fix it anyway. It will be rebuilt after they fix it. Why should the
provenpackager fix it? Often they don't need to. This is less work for
the provenpackager, and leaves the rawhide branch in the state where
the maintainer last touched it, so they can easily start fixing it.

That's what the OP explicitly does not want to do. It happens now for lack
of a better solution, but it means that packages that need to be rebuilt for
a broken dependency will be stuck with the broken dependency.

2) Proven packager *manually* does git revert for the commits that are
on rawhide but not rawhide-build (which is an easy way to know which
commits were added since the last successful build)

The whole point of this thread is that doing this manually does not scale to
a scripted mass rebuild.

And there is indeed no way to automate the general case where there can be
not only more than one commit in a linear history, but also merge commits
making the history non-linear. The only effective way (hard reset and force
push) is banned by our Git hooks.

Or soft reset to rawhide-build and commit.

That could work, even automatically, but it breaks the history by
introducing an SVN-style "merge commit" that is not really a git merge or a
git revert, but masquerades as a plain commit. And what should the commit
message be? "Revert to deadbeef1234567890abdeadbeef1234567890ab (rawhide-
build)"?

Still, it is the only way the rawhide-build branch can possibly be actually
used.

This is appropriate when the provenpackager needs the rebuild in a side
tag because other packages that depend on it also need to be rebuilt. This
still makes things harder for the maintainer who wants to fix the package,
but if the provenpackager really needs the new build, this gets them
there.  The history is still [l]inear

For some definition of "linear". It does indeed not introduce branching or
merging, but it does introduce a cumulative revert commit that makes the
history go backwards. I would say the history is indeed linear, but non-
monotonic.

(no force push anywhere)

The force push does not introduce non-linear history either. And doing a
hard reset and force push actually preserves not only the linearity, but
also the monotonicity of the history on the server.

But git is distributed, the server isn't the only copy of the history.

A force push breaks history. I can't believe we're even discussing
this point.

so the maintainer can still re-apply their changes (or revert the revert)
and then fix them.

The maintainer can also trivially re-apply their changes if the server
force-pushed them out of the way.

If your local history is:

ccccccc ← rawhide
  |
bbbbbbb ← origin/rawhide
  |
aaaaaaa

then you push this to the server:

ccccccc ← rawhide, origin/rawhide
  |
bbbbbbb
  |
aaaaaaa

and then the server does the force push:

ccccccc ← rawhide, apparent origin/rawhide
  |
bbbbbbb ← actual origin/rawhide
  |
aaaaaaa

then you fix the build:

ddddddd ← rawhide
  |
ccccccc ← apparent origin/rawhide
  |
bbbbbbb ← actual origin/rawhide
  |
aaaaaaa

and then you push rawhide, git will actually NOT complain, and this is NOT a
force-push, but a standard fast-forward push. bbbbbbb is an ancestor of
ddddddd, so the server will happily accept the push, leading to the
consistent state:

ddddddd ← rawhide, origin/rawhide
  |
ccccccc
  |
bbbbbbb
  |
aaaaaaa

(Alternatively, you could do a fetch to update your local clone's idea of
where origin/rawhide actually is. But it is not actually necessary.)

It is if a provenpackager bumped it in the meanwhile.

Now the maintainer has to resolve the fact that they thought they'd
pushed their work, but what they have locally isn't on the server.

If you are comfortable using Git, that's trivial to do. But requiring
every maintainer to be comfortable with that would be a change.

If they just do a git pull they're likely to get conflicts (to the
%changelog if nothing else) to resolve. Again, many of us will be able
%to resolve those easily. But for somebody who thought they'd pushed
%all their changes to the server already, having to re-apply them
%might not be trivial.


Sometimes option 1) will be appropriate, sometimes option 2). Making
the reset/revert automatic doesn't give anybody a choice. Both options
ensure a linear history, no rewriting with force push.

That is true.

I'm very strongly opposed to having automated force-pushes on the
rawhide branch to reset it to an earlier state. That doesn't just hurt
the maintainer who pushed the broken work, but anybody (including
provenpackagers) who fetched it before the history is altered.

How does it hurt them? See my example above.

Again, I can't believe that "rewriting git history causes issues" even
needs explanation.

I'm opposed to automated 'git revert' on rawhide (but not as strongly
as force pushing to public branches).

On the other hand, I am strongly opposed to introducing a rawhide-build
branch as you suggest. It does not address the problem at hand and I fail to
see what problem it actually addresses.

I don't need to use koji to find out the commit that built, I can just
use git commands.
_______________________________________________
devel mailing list -- devel@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxx
Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: https://lists.fedoraproject.org/archives/list/devel@xxxxxxxxxxxxxxxxxxxxxxx




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Fedora Announce]     [Fedora Users]     [Fedora Kernel]     [Fedora Testing]     [Fedora Formulas]     [Fedora PHP Devel]     [Kernel Development]     [Fedora Legacy]     [Fedora Maintainers]     [Fedora Desktop]     [PAM]     [Red Hat Development]     [Gimp]     [Yosemite News]

  Powered by Linux