Skip to content

Commit

Permalink
fix(core): filter branch in preparation for nx import (#27652)
Browse files Browse the repository at this point in the history
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 #

(cherry picked from commit ab408ab)
  • Loading branch information
jaysoo authored and FrozenPandaz committed Sep 3, 2024
1 parent d2b333f commit 578d067
Show file tree
Hide file tree
Showing 8 changed files with 343 additions and 268 deletions.
52 changes: 51 additions & 1 deletion e2e/nx/src/import.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
updateFile,
e2eCwd,
} from '@nx/e2e/utils';
import { mkdirSync, rmdirSync } from 'fs';
import { writeFileSync, mkdirSync, rmdirSync } from 'fs';
import { execSync } from 'node:child_process';
import { join } from 'path';

Expand Down Expand Up @@ -86,4 +86,54 @@ describe('Nx Import', () => {
);
runCLI(`vite:build created-vite-app`);
});

it('should be able to import two directories from same repo', () => {
// Setup repo with two packages: a and b
const repoPath = join(tempImportE2ERoot, 'repo');
mkdirSync(repoPath, { recursive: true });
writeFileSync(join(repoPath, 'README.md'), `# Repo`);
execSync(`git init`, {
cwd: repoPath,
});
execSync(`git add .`, {
cwd: repoPath,
});
execSync(`git commit -am "initial commit"`, {
cwd: repoPath,
});
execSync(`git checkout -b main`, {
cwd: repoPath,
});
mkdirSync(join(repoPath, 'packages/a'), { recursive: true });
writeFileSync(join(repoPath, 'packages/a/README.md'), `# A`);
execSync(`git add packages/a`, {
cwd: repoPath,
});
execSync(`git commit -m "add package a"`, {
cwd: repoPath,
});
mkdirSync(join(repoPath, 'packages/b'), { recursive: true });
writeFileSync(join(repoPath, 'packages/b/README.md'), `# B`);
execSync(`git add packages/b`, {
cwd: repoPath,
});
execSync(`git commit -m "add package b"`, {
cwd: repoPath,
});

runCLI(
`import ${repoPath} packages/a --ref main --source packages/a --no-interactive`,
{
verbose: true,
}
);
runCLI(
`import ${repoPath} packages/b --ref main --source packages/b --no-interactive`,
{
verbose: true,
}
);

checkFilesExist('packages/a/README.md', 'packages/b/README.md');
});
});
5 changes: 5 additions & 0 deletions packages/nx/src/command-line/import/command-object.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ export const yargsImportCommand: CommandModule = {
type: 'string',
description: 'The branch from the source repository to import',
})
.option('depth', {
type: 'number',
description:
'The depth to clone the source repository (limit this for faster git clone)',
})
.option('interactive', {
type: 'boolean',
description: 'Interactive mode',
Expand Down
192 changes: 173 additions & 19 deletions packages/nx/src/command-line/import/import.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import { join, relative, resolve } from 'path';
import { dirname, join, relative, resolve } from 'path';
import { minimatch } from 'minimatch';
import { existsSync, promises as fsp } from 'node:fs';
import * as chalk from 'chalk';
import { load as yamlLoad } from '@zkochan/js-yaml';
import { cloneFromUpstream, GitRepository } from '../../utils/git-utils';
import { stat, mkdir, rm } from 'node:fs/promises';
import { tmpdir } from 'tmp';
Expand All @@ -11,6 +15,9 @@ import { workspaceRoot } from '../../utils/workspace-root';
import {
detectPackageManager,
getPackageManagerCommand,
isWorkspacesEnabled,
PackageManager,
PackageManagerCommands,
} from '../../utils/package-manager';
import { resetWorkspaceContext } from '../../utils/workspace-context';
import { runInstall } from '../init/implementation/utils';
Expand All @@ -21,6 +28,7 @@ import {
getPackagesInPackageManagerWorkspace,
needsInstall,
} from './utils/needs-install';
import { readPackageJson } from '../../project-graph/file-utils';

const importRemoteName = '__tmp_nx_import__';

Expand All @@ -41,6 +49,10 @@ export interface ImportOptions {
* The directory in the destination repo to import into
*/
destination: string;
/**
* The depth to clone the source repository (limit this for faster clone times)
*/
depth: number;

verbose: boolean;
interactive: boolean;
Expand Down Expand Up @@ -90,7 +102,7 @@ export async function importHandler(options: ImportOptions) {

const sourceRepoPath = join(tempImportDirectory, 'repo');
const spinner = createSpinner(
`Cloning ${sourceRemoteUrl} into a temporary directory: ${sourceRepoPath}`
`Cloning ${sourceRemoteUrl} into a temporary directory: ${sourceRepoPath} (Use --depth to limit commit history and speed up clone times)`
).start();
try {
await rm(tempImportDirectory, { recursive: true });
Expand All @@ -101,6 +113,7 @@ export async function importHandler(options: ImportOptions) {
try {
sourceGitClient = await cloneFromUpstream(sourceRemoteUrl, sourceRepoPath, {
originName: importRemoteName,
depth: options.depth,
});
} catch (e) {
spinner.fail(`Failed to clone ${sourceRemoteUrl} into ${sourceRepoPath}`);
Expand All @@ -110,6 +123,9 @@ export async function importHandler(options: ImportOptions) {
}
spinner.succeed(`Cloned into ${sourceRepoPath}`);

// Detecting the package manager before preparing the source repo for import.
const sourcePackageManager = detectPackageManager(sourceGitClient.root);

if (!ref) {
const branchChoices = await sourceGitClient.listBranches();
ref = (
Expand Down Expand Up @@ -149,6 +165,7 @@ export async function importHandler(options: ImportOptions) {
name: 'destination',
message: 'Where in this workspace should the code be imported into?',
required: true,
initial: source ? source : undefined,
},
])
).destination;
Expand All @@ -157,6 +174,23 @@ export async function importHandler(options: ImportOptions) {
const absSource = join(sourceRepoPath, source);
const absDestination = join(process.cwd(), destination);

const destinationGitClient = new GitRepository(process.cwd());
await assertDestinationEmpty(destinationGitClient, absDestination);

const tempImportBranch = getTempImportBranch(ref);
await sourceGitClient.addFetchRemote(importRemoteName, ref);
await sourceGitClient.fetch(importRemoteName, ref);
spinner.succeed(`Fetched ${ref} from ${sourceRemoteUrl}`);
spinner.start(
`Checking out a temporary branch, ${tempImportBranch} based on ${ref}`
);
await sourceGitClient.checkout(tempImportBranch, {
new: true,
base: `${importRemoteName}/${ref}`,
});

spinner.succeed(`Created a ${tempImportBranch} branch based on ${ref}`);

try {
await stat(absSource);
} catch (e) {
Expand All @@ -165,11 +199,6 @@ export async function importHandler(options: ImportOptions) {
);
}

const destinationGitClient = new GitRepository(process.cwd());
await assertDestinationEmpty(destinationGitClient, absDestination);

const tempImportBranch = getTempImportBranch(ref);

const packageManager = detectPackageManager(workspaceRoot);

const originalPackageWorkspaces = await getPackagesInPackageManagerWorkspace(
Expand All @@ -186,8 +215,7 @@ export async function importHandler(options: ImportOptions) {
source,
relativeDestination,
tempImportBranch,
sourceRemoteUrl,
importRemoteName
sourceRemoteUrl
);

await createTemporaryRemote(
Expand Down Expand Up @@ -220,22 +248,74 @@ export async function importHandler(options: ImportOptions) {
options.interactive
);

if (plugins.length > 0) {
output.log({ title: 'Installing Plugins' });
installPlugins(workspaceRoot, plugins, pmc, updatePackageScripts);
if (packageManager !== sourcePackageManager) {
output.warn({
title: `Mismatched package managers`,
bodyLines: [
`The source repository is using a different package manager (${sourcePackageManager}) than this workspace (${packageManager}).`,
`This could lead to install issues due to discrepancies in "package.json" features.`,
],
});
}

await destinationGitClient.amendCommit();
// If install fails, we should continue since the errors could be resolved later.
let installFailed = false;
if (plugins.length > 0) {
try {
output.log({ title: 'Installing Plugins' });
installPlugins(workspaceRoot, plugins, pmc, updatePackageScripts);

await destinationGitClient.amendCommit();
} catch (e) {
installFailed = true;
output.error({
title: `Install failed: ${e.message || 'Unknown error'}`,
bodyLines: [e.stack],
});
}
} else if (await needsInstall(packageManager, originalPackageWorkspaces)) {
output.log({
title: 'Installing dependencies for imported code',
});
try {
output.log({
title: 'Installing dependencies for imported code',
});

runInstall(workspaceRoot, getPackageManagerCommand(packageManager));

await destinationGitClient.amendCommit();
} catch (e) {
installFailed = true;
output.error({
title: `Install failed: ${e.message || 'Unknown error'}`,
bodyLines: [e.stack],
});
}
}

runInstall(workspaceRoot, getPackageManagerCommand(packageManager));
console.log(await destinationGitClient.showStat());

await destinationGitClient.amendCommit();
if (installFailed) {
const pmc = getPackageManagerCommand(packageManager);
output.warn({
title: `The import was successful, but the install failed`,
bodyLines: [
`You may need to run "${pmc.install}" manually to resolve the issue. The error is logged above.`,
],
});
}

console.log(await destinationGitClient.showStat());
await warnOnMissingWorkspacesEntry(packageManager, pmc, relativeDestination);

// When only a subdirectory is imported, there might be devDependencies in the root package.json file
// that needs to be ported over as well.
if (ref) {
output.log({
title: `Check root dependencies`,
bodyLines: [
`"dependencies" and "devDependencies" are not imported from the source repository (${sourceRemoteUrl}).`,
`You may need to add some of those dependencies to this workspace in order to run tasks successfully.`,
],
});
}

output.log({
title: `Merging these changes into ${getBaseRef(nxJson)}`,
Expand Down Expand Up @@ -274,3 +354,77 @@ async function createTemporaryRemote(
await destinationGitClient.addGitRemote(remoteName, sourceRemoteUrl);
await destinationGitClient.fetch(remoteName);
}

// If the user imports a project that isn't in NPM/Yarn/PNPM workspaces, then its dependencies
// will not be installed. We should warn users and provide instructions on how to fix this.
async function warnOnMissingWorkspacesEntry(
pm: PackageManager,
pmc: PackageManagerCommands,
pkgPath: string
) {
if (!isWorkspacesEnabled(pm, workspaceRoot)) {
output.warn({
title: `Missing workspaces in package.json`,
bodyLines:
pm === 'npm'
? [
`We recommend enabling NPM workspaces to install dependencies for the imported project.`,
`Add \`"workspaces": ["${pkgPath}"]\` to package.json and run "${pmc.install}".`,
`See: https://docs.npmjs.com/cli/using-npm/workspaces`,
]
: pm === 'yarn'
? [
`We recommend enabling Yarn workspaces to install dependencies for the imported project.`,
`Add \`"workspaces": ["${pkgPath}"]\` to package.json and run "${pmc.install}".`,
`See: https://yarnpkg.com/features/workspaces`,
]
: pm === 'bun'
? [
`We recommend enabling Bun workspaces to install dependencies for the imported project.`,
`Add \`"workspaces": ["${pkgPath}"]\` to package.json and run "${pmc.install}".`,
`See: https://bun.sh/docs/install/workspaces`,
]
: [
`We recommend enabling PNPM workspaces to install dependencies for the imported project.`,
`Add the following entry to to pnpm-workspace.yaml and run "${pmc.install}":`,
chalk.bold(`packages:\n - '${pkgPath}'`),
`See: https://pnpm.io/workspaces`,
],
});
} else {
// Check if the new package is included in existing workspaces entries. If not, warn the user.
let workspaces: string[] | null = null;

if (pm === 'npm' || pm === 'yarn' || pm === 'bun') {
const packageJson = readPackageJson();
workspaces = packageJson.workspaces;
} else if (pm === 'pnpm') {
const yamlPath = join(workspaceRoot, 'pnpm-workspace.yaml');
if (existsSync(yamlPath)) {
const yamlContent = await fsp.readFile(yamlPath, 'utf-8');
const yaml = yamlLoad(yamlContent);
workspaces = yaml.packages;
}
}

if (workspaces) {
const isPkgIncluded = workspaces.some((w) => minimatch(pkgPath, w));
if (!isPkgIncluded) {
const pkgsDir = dirname(pkgPath);
output.warn({
title: `Project missing in workspaces`,
bodyLines:
pm === 'npm' || pm === 'yarn' || pm === 'bun'
? [
`The imported project (${pkgPath}) is missing the "workspaces" field in package.json.`,
`Add "${pkgsDir}/*" to workspaces run "${pmc.install}".`,
]
: [
`The imported project (${pkgPath}) is missing the "packages" field in pnpm-workspaces.yaml.`,
`Add "${pkgsDir}/*" to packages run "${pmc.install}".`,
],
});
}
}
}
}
Loading

0 comments on commit 578d067

Please sign in to comment.