Skip to content

Commit

Permalink
fix missing head history when on a lane (#6549)
Browse files Browse the repository at this point in the history 
In some cases, due to a regression introduced in #6542, the history of main is missing when on a lane. (see the e2e-test to get the exact scenario).
  • Loading branch information
@davidfirst
davidfirst authored Oct 18, 2022
1 parent 92f61e3 commit 3198b12
Show file tree
Hide file tree
Showing 8 changed files with 49 additions and 93 deletions.
28 changes: 28 additions & 0 deletions e2e/harmony/lanes/bit-import-on-lanes.e2e.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,5 +185,33 @@ describe('bit lane command', function () {
expect(() => helper.command.import()).to.not.throw();
});
});
describe('when the components on the lane have history other than head on main', () => {
before(() => {
helper.scopeHelper.setNewLocalAndRemoteScopes();
helper.bitJsonc.setupDefault();
helper.fixtures.populateComponents(1);
helper.command.tagAllWithoutBuild(); // 0.0.1
helper.command.export();
helper.command.createLane();
helper.command.snapAllComponentsWithoutBuild('--unmodified');
helper.command.export();
helper.command.switchLocalLane('main', '--skip-dependency-installation');
helper.command.tagAllWithoutBuild('--unmodified'); // 0.0.2
helper.command.tagAllWithoutBuild('--unmodified'); // 0.0.3
helper.command.export();
helper.command.switchLocalLane('dev', '--skip-dependency-installation');
helper.fs.deletePath('.bit');
helper.scopeHelper.addRemoteScope();
helper.command.import();
});
// previously, it wasn't importing 0.0.2, because it's not part of the lane history and it's not the head.
it('should bring all history of main', () => {
const comp = helper.command.catComponent(`${helper.scopes.remote}/comp1`);
const v2Hash = comp.versions['0.0.2'];
const v3Hash = comp.versions['0.0.3'];
expect(() => helper.command.catObject(v2Hash)).to.not.throw();
expect(() => helper.command.catObject(v3Hash)).to.not.throw();
});
});
});
});
6 changes: 6 additions & 0 deletions e2e/harmony/lanes/lanes.e2e.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -757,13 +757,15 @@ describe('bit lane command', function () {
});
describe('multiple scopes', () => {
let anotherRemote: string;
let anotherRemotePath: string;
let localScope: string;
let remoteScope: string;
before(() => {
helper.scopeHelper.setNewLocalAndRemoteScopes();
helper.bitJsonc.setupDefault();
const { scopeName, scopePath } = helper.scopeHelper.getNewBareScope();
anotherRemote = scopeName;
anotherRemotePath = scopePath;
helper.scopeHelper.addRemoteScope(scopePath);
helper.scopeHelper.addRemoteScope(scopePath, helper.scopes.remotePath);
helper.scopeHelper.addRemoteScope(helper.scopes.remotePath, scopePath);
Expand Down Expand Up @@ -820,6 +822,7 @@ describe('bit lane command', function () {
helper.command.export();
helper.scopeHelper.reInitLocalScope();
helper.scopeHelper.addRemoteScope();
helper.scopeHelper.addRemoteScope(anotherRemotePath); // needed to fetch the head from the original scope.
// previously, it was throwing an error while trying to fetch these two components, each from its own scope.
helper.command.switchRemoteLane('dev');
});
Expand All @@ -832,11 +835,13 @@ describe('bit lane command', function () {
});
describe('multiple scopes when the components are new', () => {
let anotherRemote: string;
let anotherRemotePath: string;
before(() => {
helper.scopeHelper.setNewLocalAndRemoteScopes();
helper.bitJsonc.setupDefault();
const { scopeName, scopePath } = helper.scopeHelper.getNewBareScope();
anotherRemote = scopeName;
anotherRemotePath = scopePath;
helper.scopeHelper.addRemoteScope(scopePath);
helper.scopeHelper.addRemoteScope(scopePath, helper.scopes.remotePath);
helper.fs.outputFile('bar1/index.js', 'const bar2 = require("../bar2"); console.log(bar2);');
Expand Down Expand Up @@ -872,6 +877,7 @@ describe('bit lane command', function () {
before(() => {
helper.scopeHelper.reInitLocalScope();
helper.scopeHelper.addRemoteScope();
helper.scopeHelper.addRemoteScope(anotherRemotePath); // needed to fetch the head from the original scope.
helper.command.switchRemoteLane('dev');
});
it('should not show the component as staged', () => {
Expand Down
7 changes: 6 additions & 1 deletion e2e/harmony/recovery-after-deletion.e2e.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,11 @@ describe('recovery after component/scope deletion', function () {
helper.command.linkAndCompile();
helper.command.tagAllComponents();
helper.command.export();
helper.command.runCmd(`bit import ${helper.scopes.remote}/* ${remote2Name}/* --objects`);
runFetchMissingDepsAction(helper.scopes.remote, [
`${helper.scopes.remote}/[email protected]`,
`${helper.scopes.remote}/[email protected]`,
]);
// helper.command.runCmd(`bit import ${helper.scopes.remote}/* ${remote2Name}/* --objects`);
localClone = helper.scopeHelper.cloneLocalScope();
helper.scopeHelper.reInitRemoteScope(remote2Path);
remote2Clone = helper.scopeHelper.cloneScope(remote2Path);
Expand Down Expand Up @@ -482,6 +486,7 @@ describe('recovery after component/scope deletion', function () {
helper.command.import(`${helper.scopes.remote}/comp2 ${remote2Name}/comp3`);
helper.command.tagAllComponents('', '0.0.7'); // tag comp2 with the updated comp3 version - 0.0.7
helper.command.export('--origin-directly');
runFetchMissingDepsAction(helper.scopes.remote, [`${helper.scopes.remote}/[email protected]`]);
helper.command.runCmd(`bit import ${helper.scopes.remote}/* ${remote2Name}/* --objects`);
});
it('comp3: should save 0.0.1 of in the orphanedVersions prop on the remote', () => {
Expand Down
2 changes: 1 addition & 1 deletion scopes/lanes/lanes/lanes.main.runtime.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ export class LanesMain {
reset: false,
all: false,
};
return new LaneSwitcher(this.workspace, this.logger, switchProps, checkoutProps).switch();
return new LaneSwitcher(this.workspace, this.logger, switchProps, checkoutProps, this).switch();
}

/**
Expand Down
28 changes: 4 additions & 24 deletions scopes/lanes/lanes/switch-lanes.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import mapSeries from 'p-map-series';
import { Consumer } from '@teambit/legacy/dist/consumer';
import GeneralError from '@teambit/legacy/dist/error/general-error';
import { LaneId, DEFAULT_LANE } from '@teambit/lane-id';
import ScopeComponentsImporter from '@teambit/legacy/dist/scope/component-ops/scope-components-importer';
import { BitId } from '@teambit/legacy-bit-id';
import { ComponentWithDependencies } from '@teambit/legacy/dist/scope';
import { Version, Lane } from '@teambit/legacy/dist/scope/models';
Expand All @@ -26,7 +25,7 @@ import threeWayMerge, {
import { Workspace } from '@teambit/workspace';
import { Logger } from '@teambit/logger';
import { BitError } from '@teambit/bit-error';
import { createLane } from './create-lane';
import { LanesMain } from './lanes.main.runtime';

export type SwitchProps = {
laneName: string;
Expand All @@ -45,7 +44,8 @@ export class LaneSwitcher {
private workspace: Workspace,
private logger: Logger,
private switchProps: SwitchProps,
private checkoutProps: CheckoutProps
private checkoutProps: CheckoutProps,
private Lanes: LanesMain
) {
this.consumer = this.workspace.consumer;
}
Expand Down Expand Up @@ -125,17 +125,7 @@ export class LaneSwitcher {
if (this.consumer.getCurrentLaneId().isEqual(remoteLaneId)) {
throw new BitError(`already checked out to "${remoteLaneId.toString()}"`);
}
// fetch the remote to update all heads
const scopeComponentImporter = ScopeComponentsImporter.getInstance(this.consumer.scope);
const remoteLaneObjects = await scopeComponentImporter.importFromLanes([remoteLaneId]);
if (remoteLaneObjects.length === 0) {
throw new BitError(`error: the lane ${this.switchProps.laneName} doesn't exist.`);
}
if (remoteLaneObjects.length > 1) {
const allLanes = remoteLaneObjects.map((l) => l.id()).join(', ');
throw new BitError(`switching to multiple lanes is not supported. got: ${allLanes}`);
}
const remoteLane = remoteLaneObjects[0];
const remoteLane = await this.Lanes.fetchLaneWithItsComponents(remoteLaneId);
this.switchProps.laneName = remoteLaneId.name;
this.switchProps.ids = remoteLane.components.map((l) => l.id.changeVersion(l.head.toString()));
this.switchProps.localTrackedLane = this.consumer.scope.lanes.getAliasByLaneId(remoteLaneId) || undefined;
Expand Down Expand Up @@ -177,18 +167,8 @@ export class LaneSwitcher {
}

private async saveLanesData() {
const throwIfLaneExists = async () => {
const allLanes = await this.consumer.scope.listLanes();
if (allLanes.find((l) => l.toLaneId().isEqual(this.laneIdToSwitch))) {
throw new BitError(`unable to checkout to lane "${this.laneIdToSwitch.toString()}".
the lane already exists. please switch to the lane and merge`);
}
};

const localLaneName = this.switchProps.alias || this.laneIdToSwitch.name;
if (this.switchProps.remoteLane) {
await throwIfLaneExists();
await createLane(this.consumer, this.laneIdToSwitch.name, this.laneIdToSwitch.scope, this.switchProps.remoteLane);
if (!this.switchProps.localTrackedLane) {
this.consumer.scope.lanes.trackLane({
localLane: localLaneName,
Expand Down
4 changes: 3 additions & 1 deletion scopes/scope/importer/import-components.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,9 @@ export default class ImportComponents {
const bitIds: BitIds = await this.getBitIds();
const beforeImportVersions = await this._getCurrentVersions(bitIds);
await this._throwForPotentialIssues(bitIds);
const componentsWithDependencies = await this._importObjectsDisregardLocalCache(bitIds, this.laneObjects);
const componentsWithDependencies = await this._importComponentsObjects(bitIds, {
lane: this.laneObjects?.[0],
});
if (this.laneObjects && this.options.objectsOnly) {
// merge the lane objects
const mergeAllLanesResults = await pMapSeries(this.laneObjects, (laneObject) =>
Expand Down
63 changes: 0 additions & 63 deletions src/scope/component-ops/scope-components-importer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,30 +121,10 @@ export default class ScopeComponentsImporter {
// we don't care about the VersionDeps returned here as it may belong to the dependencies
await this.getExternalMany(uniqExternals, remotes, throwForDependencyNotFound, lanes, throwForSeederNotFound);
const versionDeps = await this.bitIdsToVersionDeps(idsToImport, throwForSeederNotFound);
await this.temporarilyFetchHeadsIfNeeded(ids, versionDeps, lanes);
logger.debug('importMany, completed!');
return versionDeps;
}

/**
* a temporarily fetch of the heads until the all remotes have a version >= 0.0.881
* in this version 0.0.881, the remote-fetch already send the head in the first fetch.
*
* this is only needed when users ask importMany for ids with specific version.
* otherwise, the fetch sends the head even with the old version.
*/
private async temporarilyFetchHeadsIfNeeded(ids: BitIds, versionDeps: VersionDependencies[], lanes?: Lane[]) {
if (ids.every((id) => !id.hasVersion())) return; // importMany already got the head.
const idsWithHead = versionDeps.map((v) => {
const head = v.component.component.head;
if (!head) return null;
if (head.isEqual(v.version.hash())) return null;
return v.component.id.changeVersion(head.toString());
});
const bitIds = BitIds.fromArray(compact(idsWithHead));
await this.importWithoutDeps(bitIds, true, lanes);
}

/**
* as opposed to importMany, which imports from dependents only.
* needed mostly for cases when importMany doesn't work due to corrupted cache or the cache
Expand Down Expand Up @@ -205,43 +185,8 @@ export default class ScopeComponentsImporter {
return [...compact(componentVersionArr), ...externalDeps];
}

// // probably not needed. leaving it here temporarily as it was slightly changed before removal.
// async importManyWithAllVersions(
// ids: BitIds,
// cache = true,
// lanes: Lane[] = []
// ): Promise<VersionDependencies[]> {
// logger.debug(`scope.getManyWithAllVersions, Ids: ${ids.join(', ')}`);
// Analytics.addBreadCrumb('getManyWithAllVersions', `scope.getManyWithAllVersions, Ids: ${Analytics.hashData(ids)}`);
// const idsWithoutNils = removeNils(ids);
// if (R.isEmpty(idsWithoutNils)) return Promise.resolve([]);
// const versionDependenciesArr: VersionDependencies[] = await this.importMany({
// ids: idsWithoutNils,
// cache,
// lanes,
// });
// const allIdsWithAllVersions = new BitIds();
// versionDependenciesArr.forEach((versionDependencies) => {
// const versions = versionDependencies.component.component.listVersions();
// const versionId = versionDependencies.component.id;
// const idsWithAllVersions = versions.map((version) => {
// if (version === versionDependencies.component.version) return null; // imported already
// return versionId.changeVersion(version);
// });
// allIdsWithAllVersions.push(...removeNils(idsWithAllVersions));
// const head = versionDependencies.component.component.getHead();
// if (head) {
// allIdsWithAllVersions.push(versionId.changeVersion(head.toString()));
// }
// });
// await this.importWithoutDeps(allIdsWithAllVersions, undefined, lanes);

// return versionDependenciesArr;
// }

/**
* delta between the local head and the remote head. mainly to improve performance
* not applicable and won't work for legacy. for legacy, refer to importManyWithAllVersions
*/
async importManyDeltaWithoutDeps(
ids: BitIds,
Expand Down Expand Up @@ -296,14 +241,6 @@ export default class ScopeComponentsImporter {
).fetchFromRemoteAndWrite();
}

async importFromLanes(remoteLaneIds: LaneId[]): Promise<Lane[]> {
const lanes = await this.importLanes(remoteLaneIds);
const ids = lanes.map((lane) => lane.toBitIds());
const bitIds = BitIds.uniqFromArray(R.flatten(ids));
await this.importMany({ ids: bitIds, cache: false, lanes });
return lanes;
}

async importLanes(remoteLaneIds: LaneId[]): Promise<Lane[]> {
const remotes = await getScopeRemotes(this.scope);
const objectsStreamPerRemote = await remotes.fetch(groupByScopeName(remoteLaneIds), this.scope, { type: 'lane' });
Expand Down
4 changes: 1 addition & 3 deletions src/scope/objects/objects-readable-generator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,9 @@ export class ObjectsReadableGenerator {
stopAt: collectParentsUntil ? [collectParentsUntil] : undefined,
});
const missingParentsHashes = allParentsHashes.filter((h) => !h.isEqual(version.hash()));
if (component.head && !component.head.isEqual(version.hash())) {
missingParentsHashes.push(component.head); // always add the head
}
const parentVersions = await pMapSeries(missingParentsHashes, (parentHash) => parentHash.load(this.repo));
allVersions.push(...(parentVersions as Version[]));
// note: don't bring the head. otherwise, component-delta of the head won't bring all history of this comp.
}
await pMapSeries(allVersions, async (ver) => {
const versionObjects = await collectVersionObjects(ver);
Expand Down

0 comments on commit 3198b12

Please sign in to comment.