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

feat(merge-lane)!: change no-snap to avoid fast-forward #9159

Merged
merged 2 commits into from
Aug 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions e2e/harmony/lanes/bit-remove-on-lanes.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -324,10 +324,10 @@ describe('bit lane command', function () {
expect(staged).to.include(`${helper.scopes.remote}/comp2`);
});
});
describe('when merging with --no-snap', () => {
describe('when merging with --no-auto-snap', () => {
before(() => {
helper.scopeHelper.getClonedLocalScope(beforeMerge);
output = helper.command.mergeLane('lane-a', '-x --no-snap');
output = helper.command.mergeLane('lane-a', '-x --no-auto-snap');
});
it('bit status should not show the component as soft-removed from remote', () => {
const status = helper.command.statusJson();
Expand Down
77 changes: 68 additions & 9 deletions e2e/harmony/lanes/merge-lanes.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,7 @@ describe('merge lanes', function () {
describe('merge the lane without snapping', () => {
before(() => {
helper.scopeHelper.getClonedLocalScope(beforeMerge);
helper.command.mergeLane('main', '--auto-merge-resolve theirs --no-snap -x');
helper.command.mergeLane('main', '--auto-merge-resolve theirs --no-auto-snap -x');
});
it('should show the during-merge as modified', () => {
const status = helper.command.statusJson();
Expand Down Expand Up @@ -664,12 +664,12 @@ describe('merge lanes', function () {
expect(() => helper.command.export()).to.not.throw();
});
});
describe('bit lane merge with --resolve-unrelated and --no-snap', () => {
describe('bit lane merge with --resolve-unrelated and --no-auto-snap', () => {
before(() => {
helper.scopeHelper.getClonedRemoteScope(remoteScopeAfterExport);
helper.scopeHelper.getClonedLocalScope(afterLaneExport);
helper.command.import();
helper.command.mergeLane('main', '--resolve-unrelated --no-snap');
helper.command.mergeLane('main', '--resolve-unrelated --no-auto-snap');
});
it('bit status should show the component as during-merge and staged and not everywhere else', () => {
helper.command.expectStatusToBeClean(['componentsDuringMergeState', 'stagedComponents']);
Expand Down Expand Up @@ -701,7 +701,7 @@ describe('merge lanes', function () {
helper.scopeHelper.getClonedRemoteScope(remoteScopeAfterExport);
helper.scopeHelper.getClonedLocalScope(afterLaneExport);
helper.command.import();
helper.command.mergeLane('main', '--resolve-unrelated theirs --no-snap');
helper.command.mergeLane('main', '--resolve-unrelated theirs --no-auto-snap');
});
it('bit status should show the component as during-merge and staged and not everywhere else', () => {
helper.command.expectStatusToBeClean(['componentsDuringMergeState', 'stagedComponents']);
Expand Down Expand Up @@ -1063,7 +1063,7 @@ describe('merge lanes', function () {
});
describe('when the lane is merged to main, so currently on the FS the file exits', () => {
before(() => {
helper.command.mergeLane('dev', '--no-squash --no-snap -x');
helper.command.mergeLane('dev', '--no-squash --no-auto-snap -x');
});
// previously the file was removed
it('should not remove the file', () => {
Expand All @@ -1074,7 +1074,7 @@ describe('merge lanes', function () {
before(() => {
helper.scopeHelper.getClonedLocalScope(beforeMerge);
helper.command.switchLocalLane('dev', '-x');
helper.command.mergeLane('main', '--no-snap -x');
helper.command.mergeLane('main', '--no-auto-snap -x');
});
// previously it was in "remain-deleted" state and the file was not created
it('should add the file', () => {
Expand Down Expand Up @@ -1329,7 +1329,7 @@ describe('merge lanes', function () {
helper.command.snapAllComponentsWithoutBuild();
helper.command.export();
helper.command.switchLocalLane('main', '-x');
mergeOutput = helper.command.mergeLane('lane-a', '-x --no-snap');
mergeOutput = helper.command.mergeLane('lane-a', '-x --no-auto-snap');
});
it('should indicate that this file was removed in the output', () => {
expect(mergeOutput).to.have.string('removed foo.js');
Expand All @@ -1353,7 +1353,7 @@ describe('merge lanes', function () {
helper.command.switchLocalLane('main', '-x');
helper.command.tagAllWithoutBuild('--unmodified');
helper.command.export();
mergeOutput = helper.command.mergeLane('lane-a', '-x --no-snap --no-squash');
mergeOutput = helper.command.mergeLane('lane-a', '-x --no-auto-snap --no-squash');
});
it('should indicate that this file was removed in the output', () => {
expect(mergeOutput).to.have.string('removed foo.js');
Expand Down Expand Up @@ -1409,7 +1409,7 @@ describe('merge lanes', function () {
});
// previously it was throwing the "unrelated" error
it('bit-lane-merge should not throw', () => {
expect(() => helper.command.mergeLane('main', '-x --no-snap')).to.not.throw();
expect(() => helper.command.mergeLane('main', '-x --no-auto-snap')).to.not.throw();
});
});
describe('renaming files from uppercase to lowercase', () => {
Expand Down Expand Up @@ -1629,4 +1629,63 @@ describe('merge lanes', function () {
expect(comp1.head).to.equal(oldSnapComp1);
});
});
describe('--no-snap vs --no-auto-snap', () => {
let beforeMerge: string;
let snapLaneA: string;
let snapLaneB: string;
before(() => {
helper.scopeHelper.setNewLocalAndRemoteScopes();
helper.command.createLane('lane-a');
helper.fixtures.populateComponents(1);
helper.command.snapAllComponentsWithoutBuild();
snapLaneA = helper.command.getHeadOfLane('lane-a', 'comp1');
helper.command.export();
helper.command.createLane('lane-b');
helper.fixtures.populateComponents(1, undefined, 'from-lane-b');
helper.command.snapAllComponentsWithoutBuild();
snapLaneB = helper.command.getHeadOfLane('lane-b', 'comp1');
helper.command.export();
helper.command.switchLocalLane('lane-a', '-x');
beforeMerge = helper.scopeHelper.cloneLocalScope();
});
describe('with --no-auto-snap', () => {
before(() => {
helper.command.mergeLane('lane-b', '--no-auto-snap -x');
});
it('should update current lane according to the merged one', () => {
const snap = helper.command.getHeadOfLane('lane-a', 'comp1');
expect(snap).to.equal(snapLaneB);
});
it('should not leave the components as modified', () => {
const status = helper.command.statusJson();
expect(status.modifiedComponents).to.have.lengthOf(0);
});
});
describe('with --no-snap', () => {
before(() => {
helper.scopeHelper.getClonedLocalScope(beforeMerge);
helper.command.mergeLane('lane-b', '--no-snap -x');
});
it('should not update current lane according to the merged one', () => {
const snap = helper.command.getHeadOfLane('lane-a', 'comp1');
expect(snap).to.not.equal(snapLaneB);
expect(snap).to.equal(snapLaneA);
});
it('should leave the components as modified', () => {
const status = helper.command.statusJson();
expect(status.modifiedComponents).to.have.lengthOf(1);
});
describe('after snapping', () => {
before(() => {
helper.command.snapAllComponentsWithoutBuild();
});
it('should save two parents, from the current lane and from the merged lane', () => {
const versionObj = helper.command.catComponent('comp1@latest');
expect(versionObj.parents).to.have.lengthOf(2);
expect(versionObj.parents[0]).to.equal(snapLaneA);
expect(versionObj.parents[1]).to.equal(snapLaneB);
});
});
});
});
});
31 changes: 17 additions & 14 deletions e2e/harmony/merge-config.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,10 @@ describe('merge config scenarios', function () {
helper.scopeHelper.getClonedLocalScope(mainBeforeDiverge);
helper.command.snapAllComponentsWithoutBuild('--unmodified');
helper.command.export();
helper.command.mergeLane(`${helper.scopes.remote}/dev`, '--no-snap --skip-dependency-installation --no-squash');
helper.command.mergeLane(
`${helper.scopes.remote}/dev`,
'--no-auto-snap --skip-dependency-installation --no-squash'
);
});
it('bit status should not show the component with an issue of MergeConfigHasConflict', () => {
helper.command.expectStatusToNotHaveIssue(IssuesClasses.MergeConfigHasConflict.name);
Expand Down Expand Up @@ -163,7 +166,7 @@ describe('merge config scenarios', function () {
helper.scopeHelper.reInitLocalScope();
helper.scopeHelper.addRemoteScope();
helper.command.importLane('dev');
helper.command.mergeLane('main', '--no-snap --skip-dependency-installation');
helper.command.mergeLane('main', '--no-auto-snap --skip-dependency-installation');
beforeConfigResolved = helper.scopeHelper.cloneLocalScope();
});
it('bit status should show the component with an issue of MergeConfigHasConflict', () => {
Expand Down Expand Up @@ -230,7 +233,7 @@ describe('merge config scenarios', function () {
helper.scopeHelper.addRemoteScope();
helper.command.importLane('dev', '--skip-dependency-installation');
beforeMerge = helper.scopeHelper.cloneLocalScope();
helper.command.mergeLane('main', '--no-snap --skip-dependency-installation');
helper.command.mergeLane('main', '--no-auto-snap --skip-dependency-installation');
beforeConfigResolved = helper.scopeHelper.cloneLocalScope();
});
it('bit status should show the component with an issue of MergeConfigHasConflict', () => {
Expand Down Expand Up @@ -267,7 +270,7 @@ describe('merge config scenarios', function () {
describe('merging with --auto-merge-resolve ours', () => {
before(() => {
helper.scopeHelper.getClonedLocalScope(beforeMerge);
helper.command.mergeLane('main', '--no-snap --skip-dependency-installation --auto-merge-resolve=ours');
helper.command.mergeLane('main', '--no-auto-snap --skip-dependency-installation --auto-merge-resolve=ours');
});
it('should not generate the config-merge file', () => {
const configMerge = helper.general.getConfigMergePath();
Expand All @@ -283,7 +286,7 @@ describe('merge config scenarios', function () {
describe('merging with --auto-merge-resolve theirs', () => {
before(() => {
helper.scopeHelper.getClonedLocalScope(beforeMerge);
helper.command.mergeLane('main', '--no-snap --skip-dependency-installation --auto-merge-resolve=theirs');
helper.command.mergeLane('main', '--no-auto-snap --skip-dependency-installation --auto-merge-resolve=theirs');
});
it('should not generate the config-merge file', () => {
const configMerge = helper.general.getConfigMergePath();
Expand Down Expand Up @@ -315,7 +318,7 @@ describe('merge config scenarios', function () {
helper.command.snapAllComponentsWithoutBuild('--unmodified');
helper.command.export();

helper.command.mergeLane('dev', '--no-snap -x --no-squash');
helper.command.mergeLane('dev', '--no-auto-snap -x --no-squash');
});
// previously, it was ignoring the previous config and only adding "ramda".
it('should not remove the packages it had previously via deps set', () => {
Expand Down Expand Up @@ -350,7 +353,7 @@ describe('merge config scenarios', function () {
helper.scopeHelper.reInitLocalScope();
helper.scopeHelper.addRemoteScope();
helper.command.importLane('dev', '--skip-dependency-installation');
helper.command.mergeLane('main', '--no-snap --skip-dependency-installation --ignore-config-changes');
helper.command.mergeLane('main', '--no-auto-snap --skip-dependency-installation --ignore-config-changes');
beforeConfigResolved = helper.scopeHelper.cloneLocalScope();
});
it('bit status should show the component with an issue of MergeConfigHasConflict', () => {
Expand Down Expand Up @@ -417,7 +420,7 @@ describe('merge config scenarios', function () {
helper.scopeHelper.reInitLocalScope();
helper.scopeHelper.addRemoteScope();
helper.command.importLane('dev', '--skip-dependency-installation');
helper.command.mergeLane('main', '--no-snap --skip-dependency-installation');
helper.command.mergeLane('main', '--no-auto-snap --skip-dependency-installation');
beforeConfigResolved = helper.scopeHelper.cloneLocalScope();
});
it('bit status should show the component with an issue of MergeConfigHasConflict', () => {
Expand Down Expand Up @@ -537,7 +540,7 @@ describe('merge config scenarios', function () {
describe('when the dep was updated on the lane only, not on main', () => {
describe('when the dep is in workspace.jsonc', () => {
before(() => {
helper.command.mergeLane(`${helper.scopes.remote}/dev --no-squash --no-snap`);
helper.command.mergeLane(`${helper.scopes.remote}/dev --no-squash --no-auto-snap`);
});
it('should change workspace.jsonc with the updated dependency', () => {
const policy = helper.workspaceJsonc.getPolicyFromDependencyResolver();
Expand All @@ -548,7 +551,7 @@ describe('merge config scenarios', function () {
before(() => {
helper.scopeHelper.getClonedLocalScope(beforeMerges);
helper.workspaceJsonc.addPolicyToDependencyResolver({ dependencies: {} });
helper.command.mergeLane(`${helper.scopes.remote}/dev --no-squash --no-snap`);
helper.command.mergeLane(`${helper.scopes.remote}/dev --no-squash --no-auto-snap`);
});
it('should auto-update the dependency according to the lane, because only there it was changed', () => {
const deps = helper.command.getCompDepsIdsFromData('comp1');
Expand Down Expand Up @@ -583,7 +586,7 @@ describe('merge config scenarios', function () {
});
describe('when the dep is in workspace.jsonc', () => {
before(() => {
helper.command.mergeLane(`${helper.scopes.remote}/dev --no-squash --no-snap`);
helper.command.mergeLane(`${helper.scopes.remote}/dev --no-squash --no-auto-snap`);
});
it('should not write config-merge file', () => {
const conflictFile = helper.general.getConfigMergePath();
Expand All @@ -605,7 +608,7 @@ describe('merge config scenarios', function () {
before(() => {
helper.scopeHelper.getClonedLocalScope(afterExport);
helper.workspaceJsonc.addPolicyToDependencyResolver({ dependencies: {} });
helper.command.mergeLane(`${helper.scopes.remote}/dev --no-squash --no-snap`);
helper.command.mergeLane(`${helper.scopes.remote}/dev --no-squash --no-auto-snap`);
});
it('bit status should not show a workspace issue', () => {
const status = helper.command.statusJson();
Expand Down Expand Up @@ -681,7 +684,7 @@ describe('merge config scenarios', function () {
let mergeOutput: string;
before(() => {
helper.scopeHelper.getClonedLocalScope(laneWs);
mergeOutput = helper.command.mergeLane(`main --no-snap -x`);
mergeOutput = helper.command.mergeLane(`main --no-auto-snap -x`);
});
it('should not update workspace.jsonc', () => {
const policy = helper.workspaceJsonc.getPolicyFromDependencyResolver();
Expand Down Expand Up @@ -723,7 +726,7 @@ describe('merge config scenarios', function () {
helper.scopeHelper.reInitLocalScope();
helper.scopeHelper.addRemoteScope();
helper.command.importLane('dev', '--skip-dependency-installation');
helper.command.mergeLane('main', '--no-snap --skip-dependency-installation --ignore-config-changes');
helper.command.mergeLane('main', '--no-auto-snap --skip-dependency-installation --ignore-config-changes');
});
it('should not delete the previously deps set', () => {
const deps = helper.command.getCompDepsIdsFromData('comp1');
Expand Down
2 changes: 1 addition & 1 deletion e2e/harmony/updates-from-main-and-lane.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ describe('updates from main and lane', function () {
before(() => {
mergeOutput = helper.command.mergeLane(
'main',
'--skip-dependency-installation --no-snap --include-non-lane-comps'
'--skip-dependency-installation --no-auto-snap --include-non-lane-comps'
);
});
it('should update not only components belong to the main but also components that are available on the workspace and have updates from main', () => {
Expand Down
11 changes: 8 additions & 3 deletions scopes/component/merging/merge-status-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export type MergeStatusProviderOptions = {
mergeStrategy: MergeStrategy;
ignoreConfigChanges?: boolean;
shouldSquash?: boolean;
handleTargetAheadAsDiverged?: boolean;
};

export const compIsAlreadyMergedMsg = 'component is already merged';
Expand Down Expand Up @@ -305,7 +306,12 @@ other: ${otherLaneHead.toString()}`);
// component is ahead nothing to merge.
return this.returnUnmerged(id, compIsAlreadyMergedMsg, true);
}
if (divergeData.isTargetAhead()) {
if (!divergeData.isTargetAhead()) {
// we know that localHead and remoteHead are set, so if none of them is ahead they must be equal
return this.returnUnmerged(id, compIsAlreadyMergedMsg, true);
}
// target is ahead.
if (!this.options.handleTargetAheadAsDiverged) {
// just override with the model data
return {
...componentStatus,
Expand All @@ -314,8 +320,7 @@ other: ${otherLaneHead.toString()}`);
divergeData,
};
}
// we know that localHead and remoteHead are set, so if none of them is ahead they must be equal
return this.returnUnmerged(id, compIsAlreadyMergedMsg, true);
// target is ahead and we want to treat it as diverged, continue.
}

// it's diverged and needs merge operation
Expand Down
12 changes: 7 additions & 5 deletions scopes/component/merging/merging.main.runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ export class MergingMain {
mergeStrategy: MergeStrategy,
abort: boolean,
resolve: boolean,
noSnap: boolean,
noAutoSnap: boolean,
message: string,
build: boolean,
skipDependencyInstallation: boolean
Expand All @@ -148,7 +148,7 @@ export class MergingMain {
consumer,
bitIds,
mergeStrategy,
noSnap,
noAutoSnap,
message,
build,
skipDependencyInstallation
Expand All @@ -166,7 +166,7 @@ export class MergingMain {
consumer: Consumer,
bitIds: ComponentID[],
mergeStrategy: MergeStrategy,
noSnap: boolean,
noAutoSnap: boolean,
snapMessage: string,
build: boolean,
skipDependencyInstallation: boolean
Expand Down Expand Up @@ -195,7 +195,7 @@ export class MergingMain {
allComponentsStatus,
otherLaneId: currentLaneId,
currentLane: currentLaneObject,
noSnap,
noAutoSnap: noAutoSnap,
snapMessage,
build,
skipDependencyInstallation,
Expand All @@ -210,6 +210,7 @@ export class MergingMain {
allComponentsStatus,
otherLaneId,
currentLane,
noAutoSnap,
noSnap,
tag,
snapMessage,
Expand All @@ -220,6 +221,7 @@ export class MergingMain {
allComponentsStatus: ComponentMergeStatus[];
otherLaneId: LaneId;
currentLane?: Lane;
noAutoSnap?: boolean;
noSnap?: boolean;
tag?: boolean;
snapMessage?: string;
Expand Down Expand Up @@ -309,7 +311,7 @@ export class MergingMain {
const getSnapOrTagResults = async (): Promise<MergeSnapResults> => {
// if one of the component has conflict, don't snap-merge. otherwise, some of the components would be snap-merged
// and some not. besides the fact that it could by mistake tag dependent, it's a confusing state. better not snap.
if (noSnap || leftUnresolvedConflicts || componentsHasConfigMergeConflicts) {
if (noAutoSnap || noSnap || leftUnresolvedConflicts || componentsHasConfigMergeConflicts) {
return null;
}
if (tag) {
Expand Down
Loading