fix: (part 1?) only auto-apply in the end if it's possible w/o explod…

…ing (regression introduced in 0657970)

currently `canAndShouldBeApplying` is only in that part of the logic.

to be fair, the problem arises only when we try to apply
before the rebase was finished.

but, it very well could be that the user (even tho unlikely) that user
runs `git-stacked-rebase` while a regular `git-rebase` is in progress,
thus causing the same issue.
TODO so we need to handle this everywhere, and the approach might be
different as well.

Signed-off-by: Kipras Melnikovas <[email protected]>

apply.ts

@@ -106,8 +106,13 @@ const getPathOfFilenameOfNeedsToApply = (pathToStackedRebaseDirInsideDotGit: str
 
 export const unmarkThatNeedsToApply = (
 	pathToStackedRebaseDirInsideDotGit: string,
-	mark = getPathOfFilenameOfNeedsToApply(pathToStackedRebaseDirInsideDotGit)
-): void => (fs.existsSync(mark) ? fs.unlinkSync(mark) : void 0);
+	mark = getPathOfFilenameOfNeedsToApply(pathToStackedRebaseDirInsideDotGit),
+	rewrittenListFile: string = path.join(pathToStackedRebaseDirInsideDotGit, "rewritten-list")
+): void => (
+	fs.existsSync(mark) && fs.unlinkSync(mark),
+	fs.existsSync(rewrittenListFile) && fs.renameSync(rewrittenListFile, rewrittenListFile + ".applied"),
+	void 0
+);
 
 export async function applyIfNeedsToApply({
 	repo,

branchSequencer.ts

@@ -52,6 +52,7 @@ export type BranchSequencerArgs = BranchSequencerArgsBase & {
 	actionInsideEachCheckedOutBranch: ActionInsideEachCheckedOutBranch;
 	delayMsBetweenCheckouts?: number;
 	callbackAfterDone?: CallbackAfterDone;
+	rewrittenListFile?: "rewritten-list" | "rewritten-list.applied";
 };
 
 export type BranchSequencerBase = (args: BranchSequencerArgsBase) => Promise<EitherExitFinal>;
@@ -67,12 +68,13 @@ export const branchSequencer: BranchSequencer = async ({
 	// callbackBeforeBegin,
 	actionInsideEachCheckedOutBranch,
 	callbackAfterDone = (): void => {},
+	rewrittenListFile = "rewritten-list",
 }) => {
 	if (!fs.existsSync(pathToStackedRebaseDirInsideDotGit)) {
 		return fail(`\n\nno stacked-rebase in progress? (nothing to ${rootLevelCommandName})\n\n`);
 	}
 
-	const [exit, stackedRebaseCommandsNew] = parseNewGoodCommands(repo, pathToStackedRebaseTodoFile);
+	const [exit, stackedRebaseCommandsNew] = parseNewGoodCommands(repo, pathToStackedRebaseTodoFile, rewrittenListFile);
 	if (!stackedRebaseCommandsNew) return fail(exit);
 
 	// const remotes: Git.Remote[] = await repo.getRemotes();

forcePush.ts

@@ -38,4 +38,5 @@ export const forcePush: BranchSequencerBase = (argsBase) =>
 			execSyncInRepo(`${argsBase.gitCmd} push --force`);
 		},
 		delayMsBetweenCheckouts: 0,
+		rewrittenListFile: "rewritten-list.applied",
 	});

git-stacked-rebase.ts

@@ -17,6 +17,7 @@ import { createExecSyncInRepo } from "./util/execSyncInRepo";
 import { noop } from "./util/noop";
 import { parseTodoOfStackedRebase } from "./parse-todo-of-stacked-rebase/parseTodoOfStackedRebase";
 import { processWriteAndOrExit, fail, EitherExitFinal } from "./util/Exitable";
+import { namesOfRebaseCommandsThatMakeRebaseExitToPause } from "./parse-todo-of-stacked-rebase/validator";
 
 // console.log = () => {};
 
@@ -514,15 +515,57 @@ cat "$REWRITTEN_LIST_FILE_PATH" > "$REWRITTEN_LIST_BACKUP_FILE_PATH"
 		 * before proceeding w/ other commands way above.
 		 */
 		if (configValues.autoApplyIfNeeded) {
-			await applyIfNeedsToApply({
-				repo,
-				pathToStackedRebaseTodoFile,
-				pathToStackedRebaseDirInsideDotGit, //
-				rootLevelCommandName: "--apply",
-				gitCmd: options.gitCmd,
-				autoApplyIfNeeded: configValues.autoApplyIfNeeded,
-				config,
-			});
+			/**
+			 * since we're invoking `git rebase --continue` directly (above),
+			 * we do not have the control over it.
+			 *
+			 * meaning that in case it's the very first rebase,
+			 * the `rewritten-list` in `.git/rebase-merge/`
+			 * (the actual git-rebase, not ours)
+			 * file is not generated yet,
+			 *
+			 * and since we depend on the `git rebase --continue` (the regular rebase)
+			 * to generate the `rewritten-list` file,
+			 * we explode trying to read the file if we try to --apply below.
+			 *
+			 * ---
+			 *
+			 * edit: oh wait nvm, it's potentially any rebase that has
+			 * `break` or `edit` or similar right??
+			 *
+			 * because if the git-rebase-todo file has `break` or `edit`
+			 * or similar commands that make `git rebase --continue` exit
+			 * before it's fully completed, (my theory now is that) our code here proceeds
+			 * and tries to --apply, but again the rewritten-list file
+			 * doesn't exist yet, so we blow up.
+			 *
+			 * ---
+			 *
+			 * let's try to account for only the 1st scenario first.
+			 * TODO implement directly in `--apply`
+			 * (e.g. if user calls `gitStackedRebase` again, while still in a rebase)
+			 *
+			 * upd: ok let's also do the 2nd one because it's useless otherwise
+			 *
+			 */
+			const canAndShouldBeApplying: boolean =
+				/** part 1 */ fs.existsSync(path.join(pathToStackedRebaseDirInsideDotGit, "rewritten-list")) &&
+				/** part 2 (incomplete?) */ !fs.existsSync(pathToRegularRebaseDirInsideDotGit) &&
+				/** part 2 (complete?) (is this even needed?) */ goodCommands.every(
+					(cmd) => !namesOfRebaseCommandsThatMakeRebaseExitToPause.includes(cmd.commandName)
+				);
+
+			if (canAndShouldBeApplying) {
+				await applyIfNeedsToApply({
+					repo,
+					pathToStackedRebaseTodoFile,
+					pathToStackedRebaseDirInsideDotGit, //
+					rootLevelCommandName: "--apply",
+					gitCmd: options.gitCmd,
+					autoApplyIfNeeded: configValues.autoApplyIfNeeded,
+					config,
+				});
+			}
 		}
 
 		return;

parse-todo-of-stacked-rebase/parseNewGoodCommands.ts

@@ -14,14 +14,15 @@ import { GoodCommand, stackedRebaseCommands } from "./validator";
 
 export function parseNewGoodCommands(
 	repo: Git.Repository,
-	pathToStackedRebaseTodoFile: string //
+	pathToStackedRebaseTodoFile: string, //
+	rewrittenListFile: "rewritten-list" | "rewritten-list.applied"
 ): EitherExit<GoodCommand[]> {
 	const [exit, goodCommands] = parseTodoOfStackedRebase(pathToStackedRebaseTodoFile);
 	if (!goodCommands) return fail(exit);
 
 	logGoodCmds(goodCommands);
 
-	const pathOfRewrittenList: string = path.join(repo.workdir(), ".git", "stacked-rebase", "rewritten-list");
+	const pathOfRewrittenList: string = path.join(repo.workdir(), ".git", "stacked-rebase", rewrittenListFile);
 	const rewrittenList: string = fs.readFileSync(pathOfRewrittenList, { encoding: "utf-8" });
 	const rewrittenListLines: string[] = rewrittenList.split("\n").filter((line) => !!line);
 
@@ -36,7 +37,7 @@ export function parseNewGoodCommands(
 		const fromToSHA = line.split(" ");
 		assert(
 			fromToSHA.length === 2,
-			"from and to SHAs, coming from rewritten-list, are written properly (1 space total)."
+			`from and to SHAs, coming from ${rewrittenListFile}, are written properly (1 space total).`
 		);
 
 		const [oldSHA, newSHA] = fromToSHA;

parse-todo-of-stacked-rebase/validator.ts

@@ -23,13 +23,16 @@ type Command = {
 	maxUseCount: number;
 	isRestValid: Validator;
 	// alwaysHasSha: boolean; // TODO: parseTarget (everyone has except `break`)
-	nameOrAlias: EitherRebaseEitherCommandOrAlias;
+	nameButNeverAlias: EitherRebaseCommand;
 	parseTargets: ParseTargets;
+
+	makesGitRebaseExitToPause: boolean;
 };
 
 const createCommand = (
-	nameOrAlias: string,
+	nameButNeverAlias: string,
 	{
+		makesGitRebaseExitToPause,
 		parseTargets = ({ split }) => {
 			assert(
 				split.length >= 2,
@@ -41,29 +44,48 @@ const createCommand = (
 		maxUseCount = Infinity,
 		isRestValid = () => true,
 	}: {
+		/**
+		 * such exit is usually when a user wants to do manual action,
+		 * i.e. something that needs to be followed up by
+		 * `git rebase --continue`,
+		 * e.g. `break`, `edit`, etc.?, but not `pick`, etc.
+		 *
+		 * TODO think if cases such as dropping a commit implicitly
+		 * (removing the line instead of using the `drop` command)
+		 * would have impact for us here, etc.
+		 *
+		 */
+		makesGitRebaseExitToPause: boolean;
+
 		// nameOrAlias: EitherRebaseEitherCommandOrAlias, //
 		parseTargets?: Command["parseTargets"];
 		maxUseCount?: number;
 		isRestValid?: Validator;
-	} = {}
+	}
 ): Command => ({
 	maxUseCount,
 	isRestValid,
-	// nameOrAlias: nameOrAlias,
-	nameOrAlias: nameOrAlias as EitherRebaseEitherCommandOrAlias, // TODO: TS
+	nameButNeverAlias: nameButNeverAlias as EitherRebaseCommand, // TODO: TS
 	parseTargets,
+	makesGitRebaseExitToPause,
 });
 
 export const regularRebaseCommands = {
-	pick: createCommand("pick"),
+	pick: createCommand("pick", { makesGitRebaseExitToPause: false }),
 	// p: standardCommand,
-	reword: createCommand("reword"),
+	reword: createCommand("reword", {
+		makesGitRebaseExitToPause: false /** opens editor & then continues, w/o exiting in between */,
+	}),
 	// r: standardCommand,
-	edit: createCommand("edit"),
+	edit: createCommand("edit", { makesGitRebaseExitToPause: true }),
 	// e: standardCommand,
-	squash: createCommand("squash"),
+	squash: createCommand("squash", {
+		makesGitRebaseExitToPause: false /** opens editor & then continues, w/o exiting in between */,
+	}),
 	// s: standardCommand,
 	fixup: createCommand("fixup", {
+		makesGitRebaseExitToPause: false /** opens editor & then continues, w/o exiting in between */,
+
 		parseTargets: ({ split }) => {
 			/**
 			 * TODO: add metadata about -C|-c
@@ -78,17 +100,23 @@ export const regularRebaseCommands = {
 		},
 	}),
 	// f: standardCommand,
-	exec: createCommand("exec", { parseTargets: ({ rest }) => [rest] }),
+	exec: createCommand("exec", {
+		makesGitRebaseExitToPause: false, //
+
+		parseTargets: ({ rest }) => [rest],
+	}),
 	// x: standardCommand,
-	break: createCommand("break", { parseTargets: () => null }),
+	break: createCommand("break", { makesGitRebaseExitToPause: true, parseTargets: () => null }),
 	// b: standardCommand,
-	drop: createCommand("drop"),
+	drop: createCommand("drop", { makesGitRebaseExitToPause: false }),
 	// d: standardCommand,
-	label: createCommand("label"),
+	label: createCommand("label", { makesGitRebaseExitToPause: false /** TODO VERIFY */ }),
 	// l: standardCommand,
-	reset: createCommand("reset"),
+	reset: createCommand("reset", { makesGitRebaseExitToPause: false /** TODO VERIFY */ }),
 	// t: standardCommand,
 	merge: createCommand("merge", {
+		makesGitRebaseExitToPause: false /** TODO VERIFY */,
+
 		parseTargets: ({ split }) => {
 			if (["-C", "-c"].includes(split[1])) {
 				assert(split.length >= 4 /** not sure if 5 */);
@@ -156,23 +184,35 @@ const branchValidator: Validator = ({ rest, reasonsIfBad: reasonsWhyInvalid }) =
 	return !(reasonsWhyInvalid.length - origLen);
 };
 
+/**
+ * we'll never (?) have the `makesGitRebaseExitToPause` as `true` here
+ * because these commands do not end up in the regular git rebase's todo file.
+ */
 export const stackedRebaseCommands = {
 	"branch-end": createCommand("branch-end", {
+		makesGitRebaseExitToPause: false,
+
 		maxUseCount: Infinity,
 		isRestValid: branchValidator,
 		parseTargets: ({ rest }) => [rest],
 	}),
 	"branch-end-new": createCommand("branch-end-new", {
+		makesGitRebaseExitToPause: false,
+
 		maxUseCount: Infinity,
 		isRestValid: branchValidator,
 		parseTargets: ({ rest }) => [rest],
 	}),
 	"branch-end-initial": createCommand("branch-end-initial", {
+		makesGitRebaseExitToPause: false,
+
 		maxUseCount: 1,
 		isRestValid: branchValidator,
 		parseTargets: ({ rest }) => [rest],
 	}),
 	"branch-end-last": createCommand("branch-end-last", {
+		makesGitRebaseExitToPause: false,
+
 		maxUseCount: 1,
 		isRestValid: branchValidator,
 		parseTargets: ({ rest }) => [rest],
@@ -212,6 +252,14 @@ const allEitherRebaseCommandAliases = {
 // 	[key in AllowedRebaseCommandAlias]: CommandAlias;
 // };
 
+export const rebaseCommandsThatMakeRebaseExitToPause: Command[] = Object.values(allEitherRebaseCommands).filter(
+	(cmd) => cmd.makesGitRebaseExitToPause
+);
+
+export const namesOfRebaseCommandsThatMakeRebaseExitToPause: EitherRebaseCommand[] = rebaseCommandsThatMakeRebaseExitToPause.map(
+	(cmd) => cmd.nameButNeverAlias
+);
+
 type BadCommand = {
 	command: string;
 	lineNumber: number;