Skip to content
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

stg repair can throw away commits #163

Closed
smoofra opened this issue Nov 17, 2021 · 3 comments
Closed

stg repair can throw away commits #163

smoofra opened this issue Nov 17, 2021 · 3 comments

Comments

@smoofra
Copy link

smoofra commented Nov 17, 2021

To Reproduce:

$ git co -b xxx origin/main
$ stg init
$ stg new xxx
$ vi README
$ stg refresh
$ git commit --amend
$ stg repair

Results.

The amended commit has not been turned into a patch, and instead has been silently left behind.

Expected results.

Ideally stg repair would recognize that only the patch message has changed and update that. In any case though it should't abandon the commit, it should patchily the amended commit resulting in two, now conflicting identical unapplied patches in the
series.

Why would I do that anyway?

I use stgit to maintain my queue of ongoing patches while contributing to llvm-project, which uses Phabricator as a code review platform. Phabricators client wants to amend the commit messages to add its own information to them, which makes the workflow with stgit awkward.

@NonLogicalDev
Copy link
Contributor

NonLogicalDev commented Nov 25, 2021

Hey @smoofra, yes unfortunately this is one of the shortcomings of stg at the moment. I too use Phab at work and I can definitely feel your pain. I have been thinking about how to make the repair process better. And if not detect the amends automatically then at least give the user a way to fixup stack state by hand. But alas that is the current limitation.

Working with Phab I have come up with a workaround for the trivial case of "oops I (or **cough** arc diff **cough** made an amend of the topmost commit":

This will get you out of the pickle, but only if the amend touched the topmost commit. That will not save you from interactive rebase.

$ git co -b xxx origin/main
$ stg init
$ stg new xxx
$ vi README
$ stg refresh
$ git commit --amend

$ stg-repair-top
# your stack should be fixed and changes to your topmost commit should be preserved

@jpgrayson Do you have any workarounds if the user accidentally done git rebase -i and made some changes? I am happy we have stg repair but most of the time I am too scared to use it.

@jpgrayson
Copy link
Collaborator

A thing I'm noticing about the above amend scenario is that it only seems to be a problem when operating on the first applied patch. Consider the following scenario:

$ stg branch --create test-branch
$ stg new -m p0
$ vi README
$ stg refresh
$ stg new -m p1
$ vi README
$ stg refresh
$ git commit --amend -m "p1 amended"
$ stg repair
Creating 1 new patch ... 
  Creating patch p1-amended from commit a4cba3c316942cb99267baa9f83e85bd6d27a1aa
done
Checking patch appliedness ... 
  p1-amended is now applied
  p1 is now unapplied
done
Now at patch "p1-amended"
$ stg series
+ p0
> p1-amended
- p1

In this scenario, I think we get satisfactory behavior from stg repair. The amended commit is identified and patchified and the former patch remains in the series, but unapplied.

So it seems like stg repair could be repaired to recognize the amended commit's relationship to the stack's base and do its normal thing from there. I'll try to look at this in more detail in the near future.

jpgrayson added a commit that referenced this issue Dec 8, 2021
When `stg repair` searches for commits to "patchify", it looks for
commits that are children of known patch commits. This left an unhandled
edge case where if only one patch was applied, and that patch was
amended with `git commit --amend`, the amended commit would not be
patchified and, worse, that commit would become orphaned.

`stg repair` is modified to look for the last known stack base commit in
addition to known patch commits such that children of the base commit
will now be patchified.

An added benefit of identifying the stack's base commit is that it
signals the point beyond which no patches will be found and thus allows
searching to stop and a potentially large amount of work avoided. This
can have a significant impact on the performance of running `stg repair`
in a repositories with long histories.

Repairs #163

Signed-off-by: Peter Grayson <pete@jpgrayson.net>
@jpgrayson
Copy link
Collaborator

I've modified stg repair in 879b5c2 to handle the reported scenario. It was just a matter of having repair comprehend the stack base and proceed from there.

I have not attempted to resolve any repair scenarios involving git rebase -i (@NonLogicalDev), but if there is a use case there that stg repair doesn't handle well, I'd love to have a new issue opened for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants