-
Notifications
You must be signed in to change notification settings - Fork 122
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
west update: generalize -k / -r to include commits, not just branches #381
Comments
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 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 |
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. |
Well, west could try and Without dreaming and digressing that far, |
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.
Why is that necessary if we rebase regardless of whether HEAD is detached? |
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. 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.
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.
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". |
Yes, individual rebases can fail. The entire 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.
I'm still confused, sorry. Perhaps because I think it's easier to just rebase detached heads along with non-detached heads.
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? |
Exactly.
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.
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 zephyrproject-rtos/zephyr#31201 |
BTW Mercurial and |
This is a behavioral change originally suggested in #380, so I'm opening an issue to solicit feedback.
Currently,
west update --keep-descendants
andwest 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
The text was updated successfully, but these errors were encountered: