Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

rebase-tool: Allow 'continue' and 'finish' to work in exec step #291

Merged
merged 1 commit into from
Jan 13, 2025

Conversation

ptomato
Copy link
Contributor

@ptomato ptomato commented Jan 11, 2025

I went to do some rebasing and found the rebase tool didn't really work with --exec — whenever the tests would fail in the exec step, it was impossible to run trt continue or trt finish; the tool would just think it was all done, and exit immediately. Here's a fix.

Note that to move on after an exec failure, you now have to have run trt exec to mark the last commit as having succeeded. Ideally you wouldn't have to do this and would just be able to run trt continue, but I didn't immediately see a way to implement that without a large refactor.

If using the --exec argument, the rebase may stop during an exec step if,
for example, the tests failed. When stopped during an exec step, there's
no .git/rebase-merge/stopped-sha file, so the tool cannot pick the rebase
back up nor finish early.

Instead, get the SHA from .git/rebase-merge/done, if the last entry in the
file is an 'edit' step. If the last entry is instead an 'exec' step,
handle that separately.
@ptomato ptomato requested a review from 12wrigja January 11, 2025 01:26
Copy link
Contributor

@justingrant justingrant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great!

Copy link
Contributor

@12wrigja 12wrigja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks - I never realized that the done list also contained the active step. Now you mention it, I'm not really sure what it was I did to continue rebasing if the automated exec failed. Maybe just manually continued the rebase, and then once that inevitably failed on a non-exec step retain trt continue? Regardless, this seems cleaner.

@ptomato ptomato merged commit 69c2077 into main Jan 13, 2025
19 checks passed
@ptomato ptomato deleted the fix-rebase-tool-exec branch January 13, 2025 18:00
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants