west
west copied to clipboard
zephyrproject-rtos
west update: generalize -k / -r to include commits, not just branches
This is a behavioral change originally suggested in #380, so I'm opening an issue to solicit feedback.
Currently, west update --keep-descendants and west update --rebase only work if there is a branch checked out. They ignore detached heads and do not try to keep or rebase them.
This issue tracks generalizing their behavior to also work when HEAD is detached. We could have a configuration option to keep the old behavior.
cc: @carlescufi @tejlmand @marc-hb
Having lost detached HEADs many times when using grepo, I couldn't agree more. grepo's "solution" to this problem is (was?) "don't forget to create branches". This is silly: we use computers precisely because they don't forget what we do.
The reason users regularly "forget to create branches" is super obvious: it's simply because plain git uses branches by default whereas grepo and west do not use branches by default. Inconsistent usability - for valid reason(s) I'm sure but inconsistent usability still.
I can think of a possibly simpler/cheaper middle ground between 1. #381 automagically rebasing everything even detached HEADs and 3. #382 just warn when something bad happens: 2. Refuse to west rebase if any detached HEAD is found. This could be opt-in in ~/.westconfig
By the way I wonder whether west more generally supports the concept of a transactional west update. Last time I tried it did not. A plain git rebase or merge is either successful or it's not, I mean it either commits all the files or none. Plain git also has the incredibly useful --abort feature. Does west have the ability to "successfully west rebase all repos or none"? Part of why we have manifests is to control dependencies across repos. #338 is related.
By the way I wonder whether west more generally supports the concept of a transactional west update. Last time I tried it did not.
It does not, and I don't know how to do it. Is there some way to ask git "would this rebase work if I tried it?"
Without that, I'm not sure how such a thing could be done.
Is there some way to ask git "would this rebase work if I tried it?"
Well, west could try and git --rebase abort if it fails! Or it could implement its own git checkpoint system...
Without dreaming and digressing that far, west could perform a few sanity checks like looking for detached HEADs before performing anything with a side effect. Maybe it performs some other per-repo checks already?
Well, west could try and
git --rebase abortif it fails! Or it could implement its own git checkpoint system...
That's getting mighty fancy IMO, and asking for trouble if git rebase fails for unexpected reasons. I think failing the whole process and telling the user which repositories failed is already a pretty decent start. You can west forall -c 'git rebase --abort' (THAT OUTPUT) yourself if you need to.
Without dreaming and digressing that far, west could perform a few sanity checks like looking for detached HEADs before performing anything with a side effect. Maybe it performs some other per-repo checks already?
Why is that necessary if we rebase regardless of whether HEAD is detached?
I think failing the whole process and telling the user which repositories failed is already a pretty decent start.
Just to make sure: "failing the whole process" does not refer here to any "transactional" rebase, right? It does leave some repos successfully rebased and others not?
That's getting mighty fancy IMO,
Yes. This was just to pushing the "transactional" idea to the extreme to try to get the point across. Forget this extreme, but please try to keep the general transactional idea in mind.
Why is that necessary if we rebase regardless of whether HEAD is detached?
It's not necessary. As this issue is open to "solicit feedback", I'm suggesting some non-mutually exclusive alternative. I suspect it might be cheaper and simpler to implement.
Currently, west rebases [...] ignore detached heads and do not try to keep or rebase them.
In other words, west currently _ drops_ detached HEADs silently on a rebase, correct? So you opened this issue to propose "rescuing" them. I'm suggesting instead something like a breathalyzer, opt-in safety feature that stops the rebase trip from even starting. This is for users (like me) who know detached HEADs are bad, never intended to start a rebase trip with any and forgot to attach them to a (new) branch. Or forgot to get rid of them but as a conscious decision and not as something west does while they're not looking. This alternative is not mutually exclusive and "more transactional".
Just to make sure: "failing the whole process" does not refer here to any "transactional" rebase, right? It does leave some repos successfully rebased and others not?
Yes, individual rebases can fail.
The entire west update process exits nonzero if any rebase does fail. A message is printed in red at the end with a list of which ones failed.
I think it's probably asking for trouble to try to do more, like recover. It would be cleaner to just refuse to do any rebases if we can ask git "hypothetically, would this rebase succeed?" for all rebases and it answers "no" for any of them.
It's not necessary. As this issue is open to "solicit feedback", I'm suggesting some non-mutually exclusive alternative. I suspect it might be cheaper and simpler to implement.
I'm still confused, sorry. Perhaps because I think it's easier to just rebase detached heads along with non-detached heads.
In other words, west currently _ drops_ detached HEADs silently on a rebase, correct? So you opened this issue to propose "rescuing" them. I'm suggesting instead something like a breathalyzer, opt-in safety feature that stops the rebase trip from even starting. This is for users (like me) who know detached HEADs are bad, never intended to start a rebase trip with any and forgot to attach them to a (new) branch. Or forgot to get rid of them but as a conscious decision and not as something west does while they're not looking. This alternative is not mutually exclusive and "more transactional".
OK, so this would kind of be a "no, don't try to rebase at all" if there are any detached HEADs (that aren't an ancestor of the new manifest-rev, I guess).
Certainly open to that behavior as an alternative.
@carlescufi, @tejlmand , any thoughts on that last?
OK, so this would kind of be a "no, don't try to rebase at all" if there are any detached HEADs
Exactly.
(that aren't an ancestor of the new manifest-rev, I guess).
I was thinking about something stricter: "any detached HEAD that isn't in the previous version of the manifest" but maybe that's more difficult. Either would stop dropping detached HEADs.
Perhaps because I think it's easier to just rebase detached heads along with non-detached heads.
You know better for sure; ignore the "maybe simpler" part of the pitch.
Kick this alternative way (to solve the same problem) to a different github issue if you prefer to keep this one more focused.
Mere bidirectional link to related https://github.com/zephyrproject-rtos/zephyr/pull/31201 doc: west: add note about west update --keep-descendants option #31201