Skip to content

Commit 9c5d1e2

Browse files
committed
fix: switch back from native rebase to custom for finding commits, again
actually no need to use native rebase for __finding__ the commits. for actually rebasing, we're already doing it w/ the native rebase. for finding commits, using the native rebase had various issues, it involved having to do other hacks & work-arounds, and in general didn't represent what we were actually doing, so e.g. could cause unwanted side-effects, e.g. by calling the post-rewrite hook, when it's not actually meant to. Signed-off-by: Kipras Melnikovas <kipras@kipras.org>
1 parent 35889ae commit 9c5d1e2

File tree

1 file changed

+13
-121
lines changed

1 file changed

+13
-121
lines changed

git-stacked-rebase.ts

Lines changed: 13 additions & 121 deletions
Original file line numberDiff line numberDiff line change
@@ -101,115 +101,6 @@ function areOptionsIncompetible(
101101
return reasons.length > 0;
102102
}
103103

104-
/**
105-
* some notes:
106-
*
107-
* 1. re: "the different ways to launch git rebase":
108-
*
109-
* 1.1 initially, we started probably the hardest way:
110-
* we tried to replicate the behavior of git rebase, by:
111-
*
112-
* 1) finding which commits we need (manually by ourselves),
113-
* 2) letting the user edit our own git-rebase-todo file
114-
* (already containing branch boundaries generated by us),
115-
* 3) creating & writing to files inside `.git/rebase-{merge|apply}/`,
116-
* just like git rebase would, to represent the current state [1].
117-
* 4) and then exec'ing a `git rebase --continue`,
118-
* so that the user gets launched into the rebase.
119-
*
120-
*
121-
* 1.2 later, we started using 2 rebases - 1st for finding the commits, 2nd for the actual rebase.
122-
*
123-
* important switch that i didn't think had significance until 1.3 --
124-
* using `editorScript`s (providing an env var - a string of a path to a bash script
125-
* that will move the user-edited & our-processed git-rebase-todo file
126-
* into the proper place (.git/rebase-{merge|apply}/git-rebase-todo)
127-
* right when the rebase starts, thus no longer having to `--continue` (!),
128-
* which is what broke things that were discovered
129-
* and fixed in 1.3 by (fabricately) introducing `--continue` back)
130-
*
131-
*
132-
* 1.3 but soon after, things started breaking.
133-
*
134-
* i didn't understand why.
135-
*
136-
* but after playing w/ it for a while, realized that it's stuff like `reword`
137-
* that breaks, as long as we launch the rebase with an `editorScript`.
138-
* e.g. git would ask to identify yourself - config wouldn't be detected,
139-
* and our `--apply` was completely broken as well [2].
140-
*
141-
* thus we started manually adding a first command `break`
142-
* into the proper `git-rebase-todo` file [3]
143-
* so that we can launch `--continue` again,
144-
* after the `editorScript` had finished,
145-
* and that somehow fixed everything & we're back to normal.
146-
*
147-
*
148-
*
149-
* i am still not sure what the best approach is,
150-
* though i think i know which one's aren't.
151-
*
152-
* 1.1 seems bad because imitating the full behavior is hard,
153-
*
154-
* e.g. respecting commit.gpgSign required creating a file `gpg_sign_opt`
155-
* with content `-S` - there's probably a ton of stuff like this i didn't even realize
156-
* that git has, and it's possible more will be added in the future.
157-
* you can obviously see the problems that stem from this.
158-
*
159-
* 1.2 seems bad because it, apart from being broken until 1.3,
160-
* also used 2 rebases instead of 1, and that is kinda hacky.
161-
* stuff like git hooks exists, e.g. post-write,
162-
* that even we ourselves utilize,
163-
* and launching 2 rebases means producing side-effects
164-
* like this, and we're potentially breaking end users' workflows.
165-
*
166-
* thus, 1.3 seems like a clear winner, at least for now.
167-
* especially up until we investigate the break + continue thingie -
168-
* ideally we wouldn't need it.
169-
*
170-
*
171-
*
172-
* ---
173-
*
174-
* [1]
175-
* on representing the _state_ -- git rebase,
176-
* specifically the --interactive one (which is what we have here as well),
177-
* is _not_ a continuous command that goes through and is 100% done when it exits.
178-
*
179-
* there are some cases where it intentionally exits, to allow the user
180-
* to do further interactions, and later --continue the rebase
181-
* (e.g. `edit`, `break` etc.).
182-
*
183-
* thus, some state needs to be saved, so that the user, once they're done,
184-
* can tell git to --continue.
185-
*
186-
* turns out, git does this very simply - by saving files inside `.git/`,
187-
* and `.git/rebase-{merge|apply}/` folders.
188-
* this simple design is a big part in allowing tools like us,
189-
* git-stacked-rebase, to work, or rather - to expand upon the existing stuff,
190-
* w/o having to re-implement everything ourselves.
191-
*
192-
* [2]
193-
* on the --apply being broken - it's the logic of `parseNewGoodCommands` that seems broken.
194-
*
195-
* right before realizing 1.3 and writing this comment,
196-
* i wrote a lengthy comment there as well, with thoughts of what's likely broken.
197-
*
198-
* but now, after discovering 1.3, i do not understand yet how the
199-
* `--continue` exec fixes things, and am not sure if i want to mess
200-
* with the `parseNewGoodCommands` logic before i do.
201-
*
202-
* [3]
203-
* the user would never see the `break` command, since just like in 1.1 2),
204-
* we give the user to edit our own git-rebase-todo file, which contains branch boundaries,
205-
* and then extracting the normal git rebase commands and creating
206-
* the git-rebase-todo file for git rebase to run on.
207-
*
208-
* ---
209-
*
210-
*
211-
*
212-
*/
213104
export const gitStackedRebase = async (
214105
nameOfInitialBranch: string,
215106
specifiedOptions: SomeOptionsForGitStackedRebase = {}
@@ -440,18 +331,18 @@ export const gitStackedRebase = async (
440331
initialBranch,
441332
currentBranch,
442333
// __default__pathToStackedRebaseTodoFile
443-
pathToStackedRebaseTodoFile,
444-
() =>
445-
getWantedCommitsWithBranchBoundariesUsingNativeGitRebase({
446-
gitCmd: options.gitCmd,
447-
repo,
448-
initialBranch,
449-
currentBranch,
450-
dotGitDirPath,
451-
pathToRegularRebaseTodoFile,
452-
pathToStackedRebaseTodoFile,
453-
pathToRegularRebaseDirInsideDotGit,
454-
})
334+
pathToStackedRebaseTodoFile
335+
// () =>
336+
// getWantedCommitsWithBranchBoundariesUsingNativeGitRebase({
337+
// gitCmd: options.gitCmd,
338+
// repo,
339+
// initialBranch,
340+
// currentBranch,
341+
// dotGitDirPath,
342+
// pathToRegularRebaseTodoFile,
343+
// pathToStackedRebaseTodoFile,
344+
// pathToRegularRebaseDirInsideDotGit,
345+
// })
455346
);
456347

457348
if (!wasRegularRebaseInProgress || options.viewTodoOnly) {
@@ -1372,6 +1263,7 @@ export async function getWantedCommitsWithBranchBoundariesOurCustomImpl(
13721263
return extendCommitsWithBranchEnds(repo, bb, wantedCommits);
13731264
}
13741265

1266+
noop(getWantedCommitsWithBranchBoundariesUsingNativeGitRebase);
13751267
async function getWantedCommitsWithBranchBoundariesUsingNativeGitRebase({
13761268
gitCmd,
13771269
repo,

0 commit comments

Comments
 (0)