stack: add reorder support

Summary:
Use `FileStack.remapRevs` to calculate the file contents after reordering.
It is a bit tricky since there are various of mappings.

Reviewed By: evangrayk

Differential Revision: D45073071

fbshipit-source-id: 8b8a7e14b15d2ff2a1bbf706764be23b9ae968d7
This commit is contained in:
Jun Wu 2023-04-21 15:51:13 -07:00 committed by Facebook GitHub Bot
parent 45c8d5e5f0
commit a86449be3a
2 changed files with 216 additions and 1 deletions

View File

@ -5,6 +5,7 @@
* LICENSE file in the root directory of this source tree.
*/
import type {Rev} from '../fileStackState';
import type {ExportCommit, ExportStack} from 'shared/types/stack';
import {ABSENT_FILE, CommitStackState} from '../commitStackState';
@ -507,4 +508,90 @@ describe('CommitStackState', () => {
});
});
});
describe('reordering commits', () => {
const e = exportCommitDefault;
it('cannot be used for immutable commits', () => {
const stack = new CommitStackState([
{...e, node: 'A', immutable: true},
{...e, node: 'B', parents: ['A'], immutable: true},
{...e, node: 'C', parents: ['B'], immutable: false},
]);
expect(stack.canReorder([0, 2, 1])).toBeFalsy();
expect(stack.canReorder([1, 0, 2])).toBeFalsy();
expect(stack.canReorder([0, 1, 2])).toBeTruthy();
});
it('respects content dependencies', () => {
const stack = new CommitStackState([
{...e, node: 'A', files: {xx: {data: '2\n'}}},
{...e, node: 'B', parents: ['A'], files: {xx: {data: '1\n2\n'}}},
{...e, node: 'C', parents: ['B'], files: {xx: {data: '1\n2\n3\n'}}},
{...e, node: 'D', parents: ['C'], files: {xx: {data: '1\n2\n3\n4\n'}}},
]);
expect(stack.canReorder([0, 2, 3, 1])).toBeTruthy();
expect(stack.canReorder([0, 2, 1, 3])).toBeTruthy();
expect(stack.canReorder([0, 3, 2, 1])).toBeFalsy();
expect(stack.canReorder([0, 3, 1, 2])).toBeFalsy();
});
it('refuses to reorder non-linear stack', () => {
const stack = new CommitStackState([
{...e, node: 'A', files: {xx: {data: '1'}}},
{...e, node: 'B', parents: ['A'], files: {xx: {data: '2'}}},
{...e, node: 'C', parents: ['A'], files: {xx: {data: '3'}}},
{...e, node: 'D', parents: ['C'], files: {xx: {data: '4'}}},
]);
expect(stack.canReorder([0, 2, 3, 1])).toBeFalsy();
expect(stack.canReorder([0, 2, 1, 3])).toBeFalsy();
expect(stack.canReorder([0, 1, 2, 3])).toBeFalsy();
});
it('reorders content changes', () => {
const stack = new CommitStackState([
{...e, node: 'A', files: {xx: {data: '1\n1\n'}}},
{...e, node: 'B', parents: ['A'], files: {xx: {data: '0\n1\n1\n'}}},
{...e, node: 'C', parents: ['B'], files: {yy: {data: '0'}}}, // Does not change 'xx'.
{...e, node: 'D', parents: ['C'], files: {xx: {data: '0\n1\n1\n2\n'}}},
{...e, node: 'E', parents: ['D'], files: {xx: {data: '0\n1\n3\n1\n2\n'}}},
]);
// A-B-C-D-E => A-C-E-B-D.
let order = [0, 2, 4, 1, 3];
expect(stack.canReorder(order)).toBeTruthy();
stack.reorder(order);
const getNode = (r: Rev) => [...stack.stack[r].originalNodes][0];
expect(stack.revs().map(r => getNode(r))).toMatchObject(['A', 'C', 'E', 'B', 'D']);
expect(stack.revs().map(r => stack.stack[r].parents)).toMatchObject([[], [0], [1], [2], [3]]);
expect(stack.revs().map(r => stack.getFile(r, 'xx').data)).toMatchObject([
'1\n1\n',
'1\n1\n', // Not changed by 'C'.
'1\n3\n1\n',
'0\n1\n3\n1\n',
'0\n1\n3\n1\n2\n',
]);
expect(stack.revs().map(r => stack.getFile(r, 'yy').data)).toMatchObject([
'',
'0',
'0',
'0',
'0',
]);
// Reorder back. A-C-E-B-D => A-B-C-D-E.
order = [0, 3, 1, 4, 2];
expect(stack.canReorder(order)).toBeTruthy();
stack.reorder(order);
expect(stack.revs().map(r => getNode(r))).toMatchObject(['A', 'B', 'C', 'D', 'E']);
expect(stack.revs().map(r => stack.stack[r].parents)).toMatchObject([[], [0], [1], [2], [3]]);
expect(stack.revs().map(r => stack.getFile(r, 'xx').data)).toMatchObject([
'1\n1\n',
'0\n1\n1\n',
'0\n1\n1\n',
'0\n1\n1\n2\n',
'0\n1\n3\n1\n2\n',
]);
});
});
});

View File

@ -11,7 +11,8 @@ import type {ExportStack, ExportFile} from 'shared/types/stack';
import {assert} from '../utils';
import {FileStackState} from './fileStackState';
import {generatorContains, unwrap} from 'shared/utils';
import deepEqual from 'fast-deep-equal';
import {generatorContains, unwrap, zip} from 'shared/utils';
/**
* A stack of commits with stack editing features.
@ -394,6 +395,11 @@ export class CommitStackState {
return false;
}
/** Test if the stack is linear. */
isStackLinear(): boolean {
return this.stack.every((commit, rev) => rev === 0 || deepEqual(commit.parents, [rev - 1]));
}
// Histedit-related opeations.
/**
@ -578,6 +584,119 @@ export class CommitStackState {
this.rewriteStackDroppingRev(rev);
}
/**
* Check if reorder is conflict-free.
*
* `order` defines the new order as a "from rev" list.
* For example, when `this.revs()` is `[0, 1, 2, 3]` and `order` is
* `[0, 2, 3, 1]`, it means moving the second (rev 1) commit to the
* stack top.
*
* Reordering in a non-linear stack is not supported and will return
* `false`. This is because it's tricky to describe the desired
* new parent relationships with just `order`.
*
* If `order` is `this.revs()` then no reorder is done.
*/
canReorder(order: Rev[]): boolean {
if (!this.isStackLinear()) {
return false;
}
if (!deepEqual([...order].sort(), this.revs())) {
return false;
}
// "hash" immutable commits cannot be moved.
if (this.stack.some((commit, rev) => commit.immutableKind === 'hash' && order[rev] !== rev)) {
return false;
}
const map = new Map<Rev, Rev>(order.map((fromRev, toRev) => [fromRev, toRev]));
// Check dependencies.
const depMap = this.calculateDepMap();
for (const [rev, depRevs] of depMap) {
const newRev = map.get(rev);
if (newRev == null) {
return false;
}
for (const depRev of depRevs) {
const newDepRev = map.get(depRev);
if (newDepRev == null) {
return false;
}
if (!generatorContains(this.log(newRev), newDepRev)) {
return false;
}
}
}
// Passed checks.
return true;
}
/**
* Reorder stack. Similar to running `histedit`, follwed by reordering
* commits.
*
* See `canReorder` for the meaning of `order`.
* This should only be called when `canReorder(order)` returned `true`.
*/
reorder(order: Rev[]) {
const commitRevMap = new Map<Rev, Rev>(order.map((fromRev, toRev) => [fromRev, toRev]));
// Reorder file contents. This is somewhat tricky involving multiple
// mappings. Here is an example:
//
// Stack: A-B-C-D. Original file contents: [11, 112, 0112, 01312].
// Reorder to: A-D-B-C. Expected result: [11, 131, 1312, 01312].
//
// First, we figure out the file stack, and reorder it. The file stack
// now has the content [11 (A), 131 (B), 1312 (C), 01312 (D)], but the
// commit stack is still in the A-B-C-D order and refers to the file stack
// using **fileRev**s. If we blindly reorder the commit stack to A-D-B-C,
// the resulting files would be [11 (A), 01312 (D), 131 (B), 1312 (C)].
//
// To make it work properly, we apply a reverse mapping (A-D-B-C =>
// A-B-C-D) to the file stack before reordering commits, changing
// [11 (A), 131 (D), 1312 (B), 01312 (C)] to [11 (A), 1312 (B), 01312 (C),
// 131 (D)]. So after the commit remapping it produces the desired
// output.
this.useFileStack();
this.fileStacks.forEach((fileStack, fileIdx) => {
// file revs => commit revs => mapped commit revs => mapped file revs
const fileRevs = fileStack.revs();
const commitRevPaths: [Rev, RepoPath][] = fileRevs.map(fRev =>
unwrap(this.fileToCommit.get(`${fileIdx}:${fRev}`)),
);
const commitRevs: Rev[] = commitRevPaths.map(([rev, _path]) => rev);
const mappedCommitRevs: Rev[] = commitRevs.map(rev => commitRevMap.get(rev) ?? rev);
// commitRevs and mappedCommitRevs might not overlap, although they
// have the same length (fileRevs.length). Turn them into compact
// sequence to reason about.
const fromRevs: Rev[] = compactSequence(commitRevs);
const toRevs: Rev[] = compactSequence(mappedCommitRevs);
// Mapping: zip(original revs, mapped file revs)
const fileRevMap = new Map<Rev, Rev>(zip(fromRevs, toRevs));
fileStack.remapRevs(fileRevMap);
// Apply the reverse mapping. See the above comment for why this is necessary.
const fileTextList = [...fileStack.convertToPlainText()];
fileRevs.forEach(rev => {
const text = fileTextList[toRevs[rev]];
fileStack.editText(rev, text, false /* do not update rest of the stack */);
});
});
// Update this.stack.
this.stack = this.stack.map((_commit, rev) => {
const commit = this.stack[order[rev]];
if (commit.parents.length > 0 && rev > 0) {
commit.parents = [rev - 1];
}
commit.rev = rev;
return commit;
});
this.buildFileStacks();
}
}
function getBottomFilesFromExportStack(stack: ExportStack): Map<RepoPath, FileState> {
@ -734,6 +853,15 @@ function isUtf8(file: FileState): boolean {
return type == 'fileStack';
}
/**
* Turn distinct numbers to a 0..n sequence preserving the order.
* For example, turn [0, 100, 50] into [0, 2, 1].
*/
function compactSequence(revs: Rev[]): Rev[] {
const sortedRevs = [...revs].sort();
return revs.map(rev => sortedRevs.indexOf(rev));
}
const ABSENT_FLAG = 'a';
/**