Skip to content

Commit ab408ab

Browse files
authored
fix(core): filter branch in preparation for nx import (nrwl#27652)
This PR fixes an issue with `nx import` where multiple directories cannot be imported from the same repo. We now use `git filter-repo` (if installed) or `git filter-branch` (fallback) to prepare the repo for import so anything not in the subdirectory will be ignored (i.e. cleaner history). Note: `filter-repo` is much faster but requires the user to install it first via their package manager (e.g. `brew install git-filter-repo` or `apt install git-filter-repo`). We fallback to `git filter-branch` since it comes with git, but it is slower, so the process may take 10+ minutes on really large monorepos (e.g. next.js). Also: - Use `await` before returning a promise to Node can maintain correct stacktrace - Remove logic for `if (relativeSourceDir === '')` since using `filter-branch` moves all the files to the root (`.`) - Default destination project location to be the same as source (e.g. importing `packages/a` will go to `packages/a` unless user types in something else) - Add `--depth` option if users don't want to clone with full history (default is full history) - Fix issues with special characters causing `git ls-files` + `git mv` to since `mv` doesn't work with escaped names <!-- If this is a particularly complex change or feature addition, you can request a dedicated Nx release for this pull request branch. Mention someone from the Nx team or the `@nrwl/nx-pipelines-reviewers` and they will confirm if the PR warrants its own release for testing purposes, and generate it for you if appropriate. --> ## Current Behavior <!-- This is the behavior we have today --> ## Expected Behavior <!-- This is the behavior we should expect with the changes in this PR --> ## Related Issue(s) <!-- Please link the issue being fixed so it gets closed when this is merged. --> Fixes #
1 parent 925672e commit ab408ab

File tree

8 files changed

+343
-268
lines changed

8 files changed

+343
-268
lines changed

e2e/nx/src/import.test.ts

+51-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {
88
updateFile,
99
e2eCwd,
1010
} from '@nx/e2e/utils';
11-
import { mkdirSync, rmdirSync } from 'fs';
11+
import { writeFileSync, mkdirSync, rmdirSync } from 'fs';
1212
import { execSync } from 'node:child_process';
1313
import { join } from 'path';
1414

@@ -86,4 +86,54 @@ describe('Nx Import', () => {
8686
);
8787
runCLI(`vite:build created-vite-app`);
8888
});
89+
90+
it('should be able to import two directories from same repo', () => {
91+
// Setup repo with two packages: a and b
92+
const repoPath = join(tempImportE2ERoot, 'repo');
93+
mkdirSync(repoPath, { recursive: true });
94+
writeFileSync(join(repoPath, 'README.md'), `# Repo`);
95+
execSync(`git init`, {
96+
cwd: repoPath,
97+
});
98+
execSync(`git add .`, {
99+
cwd: repoPath,
100+
});
101+
execSync(`git commit -am "initial commit"`, {
102+
cwd: repoPath,
103+
});
104+
execSync(`git checkout -b main`, {
105+
cwd: repoPath,
106+
});
107+
mkdirSync(join(repoPath, 'packages/a'), { recursive: true });
108+
writeFileSync(join(repoPath, 'packages/a/README.md'), `# A`);
109+
execSync(`git add packages/a`, {
110+
cwd: repoPath,
111+
});
112+
execSync(`git commit -m "add package a"`, {
113+
cwd: repoPath,
114+
});
115+
mkdirSync(join(repoPath, 'packages/b'), { recursive: true });
116+
writeFileSync(join(repoPath, 'packages/b/README.md'), `# B`);
117+
execSync(`git add packages/b`, {
118+
cwd: repoPath,
119+
});
120+
execSync(`git commit -m "add package b"`, {
121+
cwd: repoPath,
122+
});
123+
124+
runCLI(
125+
`import ${repoPath} packages/a --ref main --source packages/a --no-interactive`,
126+
{
127+
verbose: true,
128+
}
129+
);
130+
runCLI(
131+
`import ${repoPath} packages/b --ref main --source packages/b --no-interactive`,
132+
{
133+
verbose: true,
134+
}
135+
);
136+
137+
checkFilesExist('packages/a/README.md', 'packages/b/README.md');
138+
});
89139
});

packages/nx/src/command-line/import/command-object.ts

+5
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,11 @@ export const yargsImportCommand: CommandModule = {
2828
type: 'string',
2929
description: 'The branch from the source repository to import',
3030
})
31+
.option('depth', {
32+
type: 'number',
33+
description:
34+
'The depth to clone the source repository (limit this for faster git clone)',
35+
})
3136
.option('interactive', {
3237
type: 'boolean',
3338
description: 'Interactive mode',

packages/nx/src/command-line/import/import.ts

+173-19
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
1-
import { join, relative, resolve } from 'path';
1+
import { dirname, join, relative, resolve } from 'path';
2+
import { minimatch } from 'minimatch';
3+
import { existsSync, promises as fsp } from 'node:fs';
4+
import * as chalk from 'chalk';
5+
import { load as yamlLoad } from '@zkochan/js-yaml';
26
import { cloneFromUpstream, GitRepository } from '../../utils/git-utils';
37
import { stat, mkdir, rm } from 'node:fs/promises';
48
import { tmpdir } from 'tmp';
@@ -11,6 +15,9 @@ import { workspaceRoot } from '../../utils/workspace-root';
1115
import {
1216
detectPackageManager,
1317
getPackageManagerCommand,
18+
isWorkspacesEnabled,
19+
PackageManager,
20+
PackageManagerCommands,
1421
} from '../../utils/package-manager';
1522
import { resetWorkspaceContext } from '../../utils/workspace-context';
1623
import { runInstall } from '../init/implementation/utils';
@@ -21,6 +28,7 @@ import {
2128
getPackagesInPackageManagerWorkspace,
2229
needsInstall,
2330
} from './utils/needs-install';
31+
import { readPackageJson } from '../../project-graph/file-utils';
2432

2533
const importRemoteName = '__tmp_nx_import__';
2634

@@ -41,6 +49,10 @@ export interface ImportOptions {
4149
* The directory in the destination repo to import into
4250
*/
4351
destination: string;
52+
/**
53+
* The depth to clone the source repository (limit this for faster clone times)
54+
*/
55+
depth: number;
4456

4557
verbose: boolean;
4658
interactive: boolean;
@@ -90,7 +102,7 @@ export async function importHandler(options: ImportOptions) {
90102

91103
const sourceRepoPath = join(tempImportDirectory, 'repo');
92104
const spinner = createSpinner(
93-
`Cloning ${sourceRemoteUrl} into a temporary directory: ${sourceRepoPath}`
105+
`Cloning ${sourceRemoteUrl} into a temporary directory: ${sourceRepoPath} (Use --depth to limit commit history and speed up clone times)`
94106
).start();
95107
try {
96108
await rm(tempImportDirectory, { recursive: true });
@@ -101,6 +113,7 @@ export async function importHandler(options: ImportOptions) {
101113
try {
102114
sourceGitClient = await cloneFromUpstream(sourceRemoteUrl, sourceRepoPath, {
103115
originName: importRemoteName,
116+
depth: options.depth,
104117
});
105118
} catch (e) {
106119
spinner.fail(`Failed to clone ${sourceRemoteUrl} into ${sourceRepoPath}`);
@@ -110,6 +123,9 @@ export async function importHandler(options: ImportOptions) {
110123
}
111124
spinner.succeed(`Cloned into ${sourceRepoPath}`);
112125

126+
// Detecting the package manager before preparing the source repo for import.
127+
const sourcePackageManager = detectPackageManager(sourceGitClient.root);
128+
113129
if (!ref) {
114130
const branchChoices = await sourceGitClient.listBranches();
115131
ref = (
@@ -149,6 +165,7 @@ export async function importHandler(options: ImportOptions) {
149165
name: 'destination',
150166
message: 'Where in this workspace should the code be imported into?',
151167
required: true,
168+
initial: source ? source : undefined,
152169
},
153170
])
154171
).destination;
@@ -157,6 +174,23 @@ export async function importHandler(options: ImportOptions) {
157174
const absSource = join(sourceRepoPath, source);
158175
const absDestination = join(process.cwd(), destination);
159176

177+
const destinationGitClient = new GitRepository(process.cwd());
178+
await assertDestinationEmpty(destinationGitClient, absDestination);
179+
180+
const tempImportBranch = getTempImportBranch(ref);
181+
await sourceGitClient.addFetchRemote(importRemoteName, ref);
182+
await sourceGitClient.fetch(importRemoteName, ref);
183+
spinner.succeed(`Fetched ${ref} from ${sourceRemoteUrl}`);
184+
spinner.start(
185+
`Checking out a temporary branch, ${tempImportBranch} based on ${ref}`
186+
);
187+
await sourceGitClient.checkout(tempImportBranch, {
188+
new: true,
189+
base: `${importRemoteName}/${ref}`,
190+
});
191+
192+
spinner.succeed(`Created a ${tempImportBranch} branch based on ${ref}`);
193+
160194
try {
161195
await stat(absSource);
162196
} catch (e) {
@@ -165,11 +199,6 @@ export async function importHandler(options: ImportOptions) {
165199
);
166200
}
167201

168-
const destinationGitClient = new GitRepository(process.cwd());
169-
await assertDestinationEmpty(destinationGitClient, absDestination);
170-
171-
const tempImportBranch = getTempImportBranch(ref);
172-
173202
const packageManager = detectPackageManager(workspaceRoot);
174203

175204
const originalPackageWorkspaces = await getPackagesInPackageManagerWorkspace(
@@ -186,8 +215,7 @@ export async function importHandler(options: ImportOptions) {
186215
source,
187216
relativeDestination,
188217
tempImportBranch,
189-
sourceRemoteUrl,
190-
importRemoteName
218+
sourceRemoteUrl
191219
);
192220

193221
await createTemporaryRemote(
@@ -220,22 +248,74 @@ export async function importHandler(options: ImportOptions) {
220248
options.interactive
221249
);
222250

223-
if (plugins.length > 0) {
224-
output.log({ title: 'Installing Plugins' });
225-
installPlugins(workspaceRoot, plugins, pmc, updatePackageScripts);
251+
if (packageManager !== sourcePackageManager) {
252+
output.warn({
253+
title: `Mismatched package managers`,
254+
bodyLines: [
255+
`The source repository is using a different package manager (${sourcePackageManager}) than this workspace (${packageManager}).`,
256+
`This could lead to install issues due to discrepancies in "package.json" features.`,
257+
],
258+
});
259+
}
226260

227-
await destinationGitClient.amendCommit();
261+
// If install fails, we should continue since the errors could be resolved later.
262+
let installFailed = false;
263+
if (plugins.length > 0) {
264+
try {
265+
output.log({ title: 'Installing Plugins' });
266+
installPlugins(workspaceRoot, plugins, pmc, updatePackageScripts);
267+
268+
await destinationGitClient.amendCommit();
269+
} catch (e) {
270+
installFailed = true;
271+
output.error({
272+
title: `Install failed: ${e.message || 'Unknown error'}`,
273+
bodyLines: [e.stack],
274+
});
275+
}
228276
} else if (await needsInstall(packageManager, originalPackageWorkspaces)) {
229-
output.log({
230-
title: 'Installing dependencies for imported code',
231-
});
277+
try {
278+
output.log({
279+
title: 'Installing dependencies for imported code',
280+
});
281+
282+
runInstall(workspaceRoot, getPackageManagerCommand(packageManager));
283+
284+
await destinationGitClient.amendCommit();
285+
} catch (e) {
286+
installFailed = true;
287+
output.error({
288+
title: `Install failed: ${e.message || 'Unknown error'}`,
289+
bodyLines: [e.stack],
290+
});
291+
}
292+
}
232293

233-
runInstall(workspaceRoot, getPackageManagerCommand(packageManager));
294+
console.log(await destinationGitClient.showStat());
234295

235-
await destinationGitClient.amendCommit();
296+
if (installFailed) {
297+
const pmc = getPackageManagerCommand(packageManager);
298+
output.warn({
299+
title: `The import was successful, but the install failed`,
300+
bodyLines: [
301+
`You may need to run "${pmc.install}" manually to resolve the issue. The error is logged above.`,
302+
],
303+
});
236304
}
237305

238-
console.log(await destinationGitClient.showStat());
306+
await warnOnMissingWorkspacesEntry(packageManager, pmc, relativeDestination);
307+
308+
// When only a subdirectory is imported, there might be devDependencies in the root package.json file
309+
// that needs to be ported over as well.
310+
if (ref) {
311+
output.log({
312+
title: `Check root dependencies`,
313+
bodyLines: [
314+
`"dependencies" and "devDependencies" are not imported from the source repository (${sourceRemoteUrl}).`,
315+
`You may need to add some of those dependencies to this workspace in order to run tasks successfully.`,
316+
],
317+
});
318+
}
239319

240320
output.log({
241321
title: `Merging these changes into ${getBaseRef(nxJson)}`,
@@ -274,3 +354,77 @@ async function createTemporaryRemote(
274354
await destinationGitClient.addGitRemote(remoteName, sourceRemoteUrl);
275355
await destinationGitClient.fetch(remoteName);
276356
}
357+
358+
// If the user imports a project that isn't in NPM/Yarn/PNPM workspaces, then its dependencies
359+
// will not be installed. We should warn users and provide instructions on how to fix this.
360+
async function warnOnMissingWorkspacesEntry(
361+
pm: PackageManager,
362+
pmc: PackageManagerCommands,
363+
pkgPath: string
364+
) {
365+
if (!isWorkspacesEnabled(pm, workspaceRoot)) {
366+
output.warn({
367+
title: `Missing workspaces in package.json`,
368+
bodyLines:
369+
pm === 'npm'
370+
? [
371+
`We recommend enabling NPM workspaces to install dependencies for the imported project.`,
372+
`Add \`"workspaces": ["${pkgPath}"]\` to package.json and run "${pmc.install}".`,
373+
`See: https://docs.npmjs.com/cli/using-npm/workspaces`,
374+
]
375+
: pm === 'yarn'
376+
? [
377+
`We recommend enabling Yarn workspaces to install dependencies for the imported project.`,
378+
`Add \`"workspaces": ["${pkgPath}"]\` to package.json and run "${pmc.install}".`,
379+
`See: https://yarnpkg.com/features/workspaces`,
380+
]
381+
: pm === 'bun'
382+
? [
383+
`We recommend enabling Bun workspaces to install dependencies for the imported project.`,
384+
`Add \`"workspaces": ["${pkgPath}"]\` to package.json and run "${pmc.install}".`,
385+
`See: https://bun.sh/docs/install/workspaces`,
386+
]
387+
: [
388+
`We recommend enabling PNPM workspaces to install dependencies for the imported project.`,
389+
`Add the following entry to to pnpm-workspace.yaml and run "${pmc.install}":`,
390+
chalk.bold(`packages:\n - '${pkgPath}'`),
391+
`See: https://pnpm.io/workspaces`,
392+
],
393+
});
394+
} else {
395+
// Check if the new package is included in existing workspaces entries. If not, warn the user.
396+
let workspaces: string[] | null = null;
397+
398+
if (pm === 'npm' || pm === 'yarn' || pm === 'bun') {
399+
const packageJson = readPackageJson();
400+
workspaces = packageJson.workspaces;
401+
} else if (pm === 'pnpm') {
402+
const yamlPath = join(workspaceRoot, 'pnpm-workspace.yaml');
403+
if (existsSync(yamlPath)) {
404+
const yamlContent = await fsp.readFile(yamlPath, 'utf-8');
405+
const yaml = yamlLoad(yamlContent);
406+
workspaces = yaml.packages;
407+
}
408+
}
409+
410+
if (workspaces) {
411+
const isPkgIncluded = workspaces.some((w) => minimatch(pkgPath, w));
412+
if (!isPkgIncluded) {
413+
const pkgsDir = dirname(pkgPath);
414+
output.warn({
415+
title: `Project missing in workspaces`,
416+
bodyLines:
417+
pm === 'npm' || pm === 'yarn' || pm === 'bun'
418+
? [
419+
`The imported project (${pkgPath}) is missing the "workspaces" field in package.json.`,
420+
`Add "${pkgsDir}/*" to workspaces run "${pmc.install}".`,
421+
]
422+
: [
423+
`The imported project (${pkgPath}) is missing the "packages" field in pnpm-workspaces.yaml.`,
424+
`Add "${pkgsDir}/*" to packages run "${pmc.install}".`,
425+
],
426+
});
427+
}
428+
}
429+
}
430+
}

0 commit comments

Comments
 (0)