From b3da937a61199ac71ed44b184ed36ff131237165 Mon Sep 17 00:00:00 2001 From: Daniel Stockman <5605+evocateur@users.noreply.github.com> Date: Mon, 8 Jun 2020 18:23:15 -0700 Subject: [PATCH] fix: Move #2445 behind `--no-granular-pathspec` option Fixes #2598 --- __fixtures__/root-manifest-only/.gitignore | 3 +- commands/publish/README.md | 18 ++++++++ .../publish/__tests__/git-checkout.test.js | 18 ++++---- .../publish/__tests__/publish-command.test.js | 41 +++++++++++++++++++ commands/publish/command.js | 11 +++++ commands/publish/index.js | 5 ++- commands/publish/lib/git-checkout.js | 6 ++- commands/version/README.md | 18 ++++++++ commands/version/__tests__/git-add.test.js | 36 +++++++++------- .../version/__tests__/version-command.test.js | 35 ++++++++++++++++ commands/version/command.js | 11 +++++ commands/version/index.js | 4 +- commands/version/lib/git-add.js | 11 ++++- 13 files changed, 189 insertions(+), 28 deletions(-) diff --git a/__fixtures__/root-manifest-only/.gitignore b/__fixtures__/root-manifest-only/.gitignore index 397b4a7624..600e8dfde2 100644 --- a/__fixtures__/root-manifest-only/.gitignore +++ b/__fixtures__/root-manifest-only/.gitignore @@ -1 +1,2 @@ -*.log +# simulate a "dynamic", intentionally unversioned package by gitignoring it +packages/dynamic-* diff --git a/commands/publish/README.md b/commands/publish/README.md index 6b92b51878..82a6c08323 100644 --- a/commands/publish/README.md +++ b/commands/publish/README.md @@ -56,6 +56,7 @@ This is useful when a previous `lerna publish` failed to publish all packages to - [`--ignore-prepublish`](#--ignore-prepublish) - [`--legacy-auth`](#--legacy-auth) - [`--no-git-reset`](#--no-git-reset) +- [`--no-granular-pathspec`](#--no-granular-pathspec) - [`--no-verify-access`](#--no-verify-access) - [`--otp`](#--otp) - [`--preid`](#--preid) @@ -179,6 +180,23 @@ To avoid this, pass `--no-git-reset`. This can be especially useful when used as lerna publish --no-git-reset ``` +### `--no-granular-pathspec` + +By default, `lerna publish` will attempt (if enabled) to `git checkout` _only_ the leaf package manifests that are temporarily modified during the publishing process. This yields the equivalent of `git checkout -- packages/*/package.json`, but tailored to _exactly_ what changed. + +If you **know** you need different behavior, you'll understand: Pass `--no-granular-pathspec` to make the git command _literally_ `git checkout -- .`. By opting into this [pathspec](https://git-scm.com/docs/gitglossary#Documentation/gitglossary.txt-aiddefpathspecapathspec), you must have all intentionally unversioned content properly ignored. + +This option makes the most sense configured in lerna.json, as you really don't want to mess it up: + +```json +{ + "version": "independent", + "granularPathspec": false +} +``` + +The root-level configuration is intentional, as this also covers the [identically-named option in `lerna version`](https://github.com/lerna/lerna/tree/master/commands/version#--no-granular-pathspec). + ### `--no-verify-access` By default, `lerna` will verify the logged-in npm user's access to the packages about to be published. Passing this flag will disable that check. diff --git a/commands/publish/__tests__/git-checkout.test.js b/commands/publish/__tests__/git-checkout.test.js index 2a775150a9..873d586d1c 100644 --- a/commands/publish/__tests__/git-checkout.test.js +++ b/commands/publish/__tests__/git-checkout.test.js @@ -11,7 +11,7 @@ test("gitCheckout files", async () => { const files = ["package-1", "package-2"].map(name => path.join("packages", name, "package.json")); await Promise.all(files.map(fp => fs.writeJSON(path.join(cwd, fp), { foo: "bar" }))); - await gitCheckout(files, { cwd }); + await gitCheckout(files, { granularPathspec: true }, { cwd }); const modified = await execa.stdout("git", ["ls-files", "--modified"], { cwd }); expect(modified).toBe(""); @@ -19,12 +19,16 @@ test("gitCheckout files", async () => { test("gitCheckout files with .gitignored files", async () => { const cwd = await initFixture("no-interdependencies"); - const files = ["package-1", "package-2"].map(name => path.join("packages", name, "package.json")); - files.push("packages/package-1/index.log"); + const files = ["package-1", "package-2", "package-3"].map(name => + path.join("packages", name, "package.json") + ); - await Promise.all(files.map(fp => fs.writeJSON(path.join(cwd, fp), { foo: "bar" }))); - await gitCheckout(files, { cwd }); + // simulate a "dynamic", intentionally unversioned package by gitignoring it + await fs.writeFile(path.join(cwd, ".gitignore"), "packages/package-3/*", "utf8"); - const modified = await execa.stdout("git", ["ls-files", "--modified"], { cwd }); - expect(modified).toBe(""); + await Promise.all(files.map(fp => fs.outputJSON(path.join(cwd, fp), { foo: "bar" }))); + await gitCheckout(files, { granularPathspec: false }, { cwd }); + + const modified = await execa.stdout("git", ["ls-files", "--others"], { cwd }); + expect(modified).toBe("packages/package-3/package.json"); }); diff --git a/commands/publish/__tests__/publish-command.test.js b/commands/publish/__tests__/publish-command.test.js index 4daa569189..1c0b8269ed 100644 --- a/commands/publish/__tests__/publish-command.test.js +++ b/commands/publish/__tests__/publish-command.test.js @@ -32,6 +32,7 @@ const commitChangeToPackage = require("@lerna-test/commit-change-to-package"); const loggingOutput = require("@lerna-test/logging-output"); const initFixture = require("@lerna-test/init-fixture")(__dirname); const path = require("path"); +const fs = require("fs-extra"); // file under test const lernaPublish = require("@lerna-test/command-runner")(require("../command")); @@ -144,6 +145,13 @@ Map { // extra insurance that @lerna/npm-conf is defaulting things correctly expect.figgyPudding({ otp: undefined }) ); + + expect(gitCheckout).toHaveBeenCalledWith( + // the list of changed files has been asserted many times already + expect.any(Array), + { granularPathspec: true }, + { cwd: testDir } + ); }); it("publishes changed independent packages", async () => { @@ -332,6 +340,39 @@ Map { }); }); + // TODO: (major) make --no-granular-pathspec the default + describe("--no-granular-pathspec", () => { + it("resets staged changes globally", async () => { + const cwd = await initFixture("normal"); + + await lernaPublish(cwd)("--no-granular-pathspec"); + + expect(gitCheckout).toHaveBeenCalledWith( + // the list of changed files has been asserted many times already + expect.any(Array), + { granularPathspec: false }, + { cwd } + ); + }); + + it("consumes configuration from lerna.json", async () => { + const cwd = await initFixture("normal"); + + await fs.outputJSON(path.join(cwd, "lerna.json"), { + version: "1.0.0", + granularPathspec: false, + }); + await lernaPublish(cwd)(); + + expect(gitCheckout).toHaveBeenCalledWith( + // the list of changed files has been asserted many times already + expect.any(Array), + { granularPathspec: false }, + { cwd } + ); + }); + }); + describe("--contents", () => { it("allows you to do fancy angular crap", async () => { const cwd = await initFixture("lifecycle"); diff --git a/commands/publish/command.js b/commands/publish/command.js index 8d63cbc905..814efaad82 100644 --- a/commands/publish/command.js +++ b/commands/publish/command.js @@ -63,6 +63,17 @@ exports.builder = yargs => { describe: "Disable all lifecycle scripts", type: "boolean", }, + // TODO: (major) make --no-granular-pathspec the default + "no-granular-pathspec": { + describe: "Do not reset changes file-by-file, but globally.", + type: "boolean", + }, + "granular-pathspec": { + // proxy for --no-granular-pathspec + hidden: true, + // describe: "Reset changes file-by-file, not globally.", + type: "boolean", + }, otp: { describe: "Supply a one-time password for publishing with two-factor authentication.", type: "string", diff --git a/commands/publish/index.js b/commands/publish/index.js index 09e88a9f09..078012845c 100644 --- a/commands/publish/index.js +++ b/commands/publish/index.js @@ -562,11 +562,14 @@ class PublishCommand extends Command { // the package.json files are changed (by gitHead if not --canary) // and we should always __attempt_ to leave the working tree clean const { cwd } = this.execOpts; + const gitOpts = { + granularPathspec: this.options.granularPathspec !== false, + }; const dirtyManifests = [this.project.manifest] .concat(this.packagesToPublish) .map(pkg => path.relative(cwd, pkg.manifestLocation)); - return gitCheckout(dirtyManifests, this.execOpts).catch(err => { + return gitCheckout(dirtyManifests, gitOpts, this.execOpts).catch(err => { this.logger.silly("EGITCHECKOUT", err.message); this.logger.notice("FYI", "Unable to reset working tree changes, this probably isn't a git repo."); }); diff --git a/commands/publish/lib/git-checkout.js b/commands/publish/lib/git-checkout.js index 7726c38054..81c83c1ff9 100644 --- a/commands/publish/lib/git-checkout.js +++ b/commands/publish/lib/git-checkout.js @@ -5,8 +5,10 @@ const childProcess = require("@lerna/child-process"); module.exports = gitCheckout; -function gitCheckout(files, opts) { +function gitCheckout(stagedFiles, gitOpts, execOpts) { + const files = gitOpts.granularPathspec ? stagedFiles : "."; + log.silly("gitCheckout", files); - return childProcess.exec("git", ["checkout", "--", "."], opts); + return childProcess.exec("git", ["checkout", "--"].concat(files), execOpts); } diff --git a/commands/version/README.md b/commands/version/README.md index e720488e2d..6bdccc100a 100644 --- a/commands/version/README.md +++ b/commands/version/README.md @@ -60,6 +60,7 @@ Running `lerna version --conventional-commits` without the above flags will rele - [`--no-changelog`](#--no-changelog) - [`--no-commit-hooks`](#--no-commit-hooks) - [`--no-git-tag-version`](#--no-git-tag-version) +- [`--no-granular-pathspec`](#--no-granular-pathspec) - [`--no-private`](#--no-private) - [`--no-push`](#--no-push) - [`--preid`](#--preid) @@ -338,6 +339,23 @@ Pass `--no-git-tag-version` to disable the behavior. This option is analogous to the `npm version` option [`--git-tag-version`](https://docs.npmjs.com/misc/config#git-tag-version), just inverted. +### `--no-granular-pathspec` + +By default, `lerna version` will `git add` _only_ the leaf package manifests (and possibly changelogs) that have changed during the versioning process. This yields the equivalent of `git add -- packages/*/package.json`, but tailored to _exactly_ what changed. + +If you **know** you need different behavior, you'll understand: Pass `--no-granular-pathspec` to make the git command _literally_ `git add -- .`. By opting into this [pathspec](https://git-scm.com/docs/gitglossary#Documentation/gitglossary.txt-aiddefpathspecapathspec), you **MUST HAVE _ALL_ SECRETS AND BUILD OUTPUT PROPERLY IGNORED, _OR IT WILL BE COMMITTED AND PUSHED_**. + +This option makes the most sense configured in lerna.json, as you really don't want to mess it up: + +```json +{ + "version": "independent", + "granularPathspec": false +} +``` + +The root-level configuration is intentional, as this also covers the [identically-named option in `lerna publish`](https://github.com/lerna/lerna/tree/master/commands/publish#--no-granular-pathspec). + ### `--no-private` By default, `lerna version` will include private packages when choosing versions, making commits, and tagging releases. diff --git a/commands/version/__tests__/git-add.test.js b/commands/version/__tests__/git-add.test.js index 4562d19d2d..d0f24f88d8 100644 --- a/commands/version/__tests__/git-add.test.js +++ b/commands/version/__tests__/git-add.test.js @@ -12,7 +12,7 @@ test("relative files", async () => { const file = path.join("packages", "pkg-1", "index.js"); await fs.outputFile(path.join(cwd, file), "hello"); - await gitAdd([file], { cwd }); + await gitAdd([file], { granularPathspec: true }, { cwd }); const list = await execa.stdout("git", ["diff", "--cached", "--name-only"], { cwd }); expect(slash(list)).toBe("packages/pkg-1/index.js"); @@ -23,7 +23,7 @@ test("absolute files", async () => { const file = path.join(cwd, "packages", "pkg-2", "index.js"); await fs.outputFile(file, "hello"); - await gitAdd([file], { cwd }); + await gitAdd([file], { granularPathspec: true }, { cwd }); const list = await execa.stdout("git", ["diff", "--cached", "--name-only"], { cwd }); expect(slash(list)).toBe("packages/pkg-2/index.js"); @@ -31,26 +31,34 @@ test("absolute files", async () => { test(".gitignore", async () => { const cwd = await initFixture("root-manifest-only"); - const file = path.join(cwd, "packages", "pkg-2", "index.log"); - const file2 = path.join(cwd, "packages", "pkg-2", "index.js"); + const file3 = path.join(cwd, "packages/version-3/package.json"); + const file4 = path.join(cwd, "packages/dynamic-4/package.json"); - await fs.outputFile(file, "hello"); - await fs.outputFile(file2, "hello2"); - await gitAdd([file, file2], { cwd }); + await Promise.all([ + // a "dynamic" package is intentionally unversioned, yet still published + fs.outputJSON(file3, { three: true }), + fs.outputJSON(file4, { four: true }), + ]); + + await gitAdd([file3, file4], { granularPathspec: false }, { cwd }); const list = await execa.stdout("git", ["diff", "--cached", "--name-only"], { cwd }); - expect(slash(list)).toBe("packages/pkg-2/index.js"); + expect(slash(list)).toBe("packages/version-3/package.json"); }); test(".gitignore without naming files", async () => { const cwd = await initFixture("root-manifest-only"); - const file = path.join(cwd, "packages", "pkg-2", "index.log"); - const file2 = path.join(cwd, "packages", "pkg-2", "index.js"); + const file5 = path.join(cwd, "packages/version-5/package.json"); + const file6 = path.join(cwd, "packages/dynamic-6/package.json"); - await fs.outputFile(file, "hello"); - await fs.outputFile(file2, "hello2"); - await gitAdd([], { cwd }); + await Promise.all([ + // a "dynamic" package is intentionally unversioned, yet still published + fs.outputJSON(file5, { five: true }), + fs.outputJSON(file6, { six: true }), + ]); + + await gitAdd([], { granularPathspec: false }, { cwd }); const list = await execa.stdout("git", ["diff", "--cached", "--name-only"], { cwd }); - expect(slash(list)).toBe("packages/pkg-2/index.js"); + expect(slash(list)).toBe("packages/version-5/package.json"); }); diff --git a/commands/version/__tests__/version-command.test.js b/commands/version/__tests__/version-command.test.js index 2ccabb2830..ee2d67848f 100644 --- a/commands/version/__tests__/version-command.test.js +++ b/commands/version/__tests__/version-command.test.js @@ -296,6 +296,41 @@ describe("VersionCommand", () => { }); }); + // TODO: (major) make --no-granular-pathspec the default + describe("--no-granular-pathspec", () => { + it("adds changed files globally", async () => { + const cwd = await initFixture("normal"); + await fs.outputFile(path.join(cwd, ".gitignore"), "packages/dynamic"); + await fs.outputJSON(path.join(cwd, "packages/dynamic/package.json"), { + name: "dynamic", + version: "1.0.0", + }); + // a "dynamic", intentionally unversioned package must _always_ be forced + await lernaVersion(cwd)("--force-publish=dynamic", "--no-granular-pathspec"); + + const leftover = await execa.stdout("git", ["ls-files", "--others"], { cwd }); + expect(leftover).toBe("packages/dynamic/package.json"); + }); + + it("consumes configuration from lerna.json", async () => { + const cwd = await initFixture("normal"); + await fs.outputFile(path.join(cwd, ".gitignore"), "packages/dynamic"); + await fs.outputJSON(path.join(cwd, "packages/dynamic/package.json"), { + name: "dynamic", + version: "1.0.0", + }); + await fs.outputJSON(path.join(cwd, "lerna.json"), { + version: "1.0.0", + granularPathspec: false, + }); + // a "dynamic", intentionally unversioned package must _always_ be forced + await lernaVersion(cwd)("--force-publish=dynamic"); + + const leftover = await execa.stdout("git", ["ls-files", "--others"], { cwd }); + expect(leftover).toBe("packages/dynamic/package.json"); + }); + }); + // TODO: (major) make --no-private the default describe("--no-private", () => { it("does not universally version private packages", async () => { diff --git a/commands/version/command.js b/commands/version/command.js index f2e8ea0185..974ce66687 100644 --- a/commands/version/command.js +++ b/commands/version/command.js @@ -105,6 +105,17 @@ exports.builder = (yargs, composed) => { hidden: true, type: "boolean", }, + // TODO: (major) make --no-granular-pathspec the default + "no-granular-pathspec": { + describe: "Do not stage changes file-by-file, but globally.", + type: "boolean", + }, + "granular-pathspec": { + // proxy for --no-granular-pathspec + hidden: true, + // describe: "Stage changes file-by-file, not globally.", + type: "boolean", + }, // TODO: (major) make --no-private the default "no-private": { describe: "Do not version private packages.", diff --git a/commands/version/index.js b/commands/version/index.js index 5707a9b4c4..30836fec41 100644 --- a/commands/version/index.js +++ b/commands/version/index.js @@ -64,6 +64,7 @@ class VersionCommand extends Command { commitHooks = true, gitRemote = "origin", gitTagVersion = true, + granularPathspec = true, push = true, signGitCommit, signGitTag, @@ -91,6 +92,7 @@ class VersionCommand extends Command { this.gitOpts = { amend, commitHooks, + granularPathspec, signGitCommit, signGitTag, forceGitTag, @@ -614,7 +616,7 @@ class VersionCommand extends Command { } if (this.commitAndTag) { - chain = chain.then(() => gitAdd(Array.from(changedFiles), this.execOpts)); + chain = chain.then(() => gitAdd(Array.from(changedFiles), this.gitOpts, this.execOpts)); } return chain; diff --git a/commands/version/lib/git-add.js b/commands/version/lib/git-add.js index df2d451bea..ecb6454821 100644 --- a/commands/version/lib/git-add.js +++ b/commands/version/lib/git-add.js @@ -1,12 +1,19 @@ "use strict"; const log = require("npmlog"); +const path = require("path"); +const slash = require("slash"); const childProcess = require("@lerna/child-process"); module.exports = gitAdd; -function gitAdd(files, opts) { +function gitAdd(changedFiles, gitOpts, execOpts) { + // granular pathspecs should be relative to the git root, but that isn't necessarily where lerna lives + const files = gitOpts.granularPathspec + ? changedFiles.map(file => slash(path.relative(execOpts.cwd, path.resolve(execOpts.cwd, file)))) + : "."; + log.silly("gitAdd", files); - return childProcess.exec("git", ["add", "--", "."], opts); + return childProcess.exec("git", ["add", "--", ...files], execOpts); }