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

TODO: Attempt to rebase human commits instead of failing immediately #10

Open
balsoft opened this issue Sep 29, 2021 · 8 comments · May be fixed by #15
Open

TODO: Attempt to rebase human commits instead of failing immediately #10

balsoft opened this issue Sep 29, 2021 · 8 comments · May be fixed by #15
Assignees

Comments

@balsoft
Copy link
Member

balsoft commented Sep 29, 2021

update-daemon should ideally try to rebase "human" commits on top of the automatic update commit. It should work in most cases (when humans didn't change the flake.lock file), and would make the workflow much smoother.

@gromakovsky
Copy link
Member

@notgne2
Copy link
Contributor

notgne2 commented Jun 2, 2022

We used to have some form of rebasing before this commit 91d1f66 - which removed it as iirc it was very broken, but old implementation probably still worth referencing

@sancho20021 sancho20021 self-assigned this Jun 16, 2022
@sancho20021
Copy link
Contributor

We used to have some form of rebasing before this commit 91d1f66 - which removed it as iirc it was very broken, but old implementation probably still worth referencing

What were the main reasons why old implementation was broken, does anybody remember?)

@balsoft
Copy link
Member Author

balsoft commented Jun 16, 2022

It didn't handle conflicts at all, resulting in broken state.

@sancho20021
Copy link
Contributor

Ok, thanks.

I'm trying to understand the current behavior of update procedure, am I right that:

  1. If the latest commit in remote update branch is done by someone else, updating fails;
  2. If there were changes in default (master) branch and remote update branch exists, update will just reset local repository to remote update and won't try to synchronize with master, which will cause conflicts?

So, if the stated above is correct,

  • in the first situation I just need to reset to the latest human commit?
  • in the second I need to rebase update branch on top of master?

@sancho20021
Copy link
Contributor

Btw what is the logic for aborting update in case of human commits in the update branch? What errors will happen if we remove this check?

@balsoft
Copy link
Member Author

balsoft commented Jun 22, 2022

If the latest commit in remote update branch is done by someone else, updating fails;

Yes

If there were changes in default (master) branch and remote update branch exists, update will just reset local repository to remote update and won't try to synchronize with master, which will cause conflicts?

It resets the update branch to master every time, regardless of changes in master. I.e. it disregards the remote update branch state. It's basically

git checkout master
git pull
git checkout -B $UPDATE_BRANCH
nix flake update
git push --set-upstream origin $UPDATE_BRANCH --force-with-lease

@balsoft
Copy link
Member Author

balsoft commented Jun 22, 2022

What errors will happen if we remove this check?

No errors will happen, but human commits may potentially be orphaned, and we presume that human commits are imporant and worth keeping, even at the cost of failing to update.

I guess it might kinda make sense to simply remove this check and let update-daemon force-push over human commits, and then let humans do the rebasing manually, but it would be annoying.

sancho20021 added a commit that referenced this issue Jun 30, 2022
Problem: Currently when there are human commits in update branch,
update bot reports an error.

Solution: Rebase current changes in remote update branch on top
of remote default branch if possible. Otherwise report an error.
@sancho20021 sancho20021 linked a pull request Jun 30, 2022 that will close this issue
sancho20021 added a commit that referenced this issue Jul 20, 2022
Problem: bot doesn't checkout to a sensible branch after a rebase fail.

Solution: checkout to default branch. Note: currently an error
"rebase patch already applied" occurs. Needs to be fixed.
lierdakil added a commit that referenced this issue Jul 21, 2023
Problem: if the default branch is updated while an update branch exists,
the update branch won't be automatically rebased.

Solution: in lieu of a proper rebase, which is hard, reset the update
branch to the default, as we'll have to force-push anyway, and there's
not much to rebase if there are no human commits to begin with.
lierdakil added a commit that referenced this issue Jul 21, 2023
Problem: currently, when there are human commits in the update branch,
we just bail. There has to be a more robust strategy.

Solution: complicated. The first decision point is whether the update branch
is behind the default branch.

- If it isn't, the bot pushes new commits on top of the branch, or
  amends the last commit if it wasn't made by a human and it's not also
  on the default branch.
- If it is, the update branch is hard-reset to the default one, and any
  human commits are cherry-picked on top.
- If it doesn't exist, it's initialized to the default branch.

This feels like a reasonable balance between force-pushing all the time
and trying to preserve history to an extent. However it is all rather
convoluted.
lierdakil added a commit that referenced this issue Jul 26, 2023
…commits

[#10] Reset to default branch if update branch is behind
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

Successfully merging a pull request may close this issue.

4 participants