fix: Move #2445 behind --no-granular-pathspec option

Fixes #2598

__fixtures__/root-manifest-only/.gitignore

@@ -1 +1,2 @@
-*.log
+# simulate a "dynamic", intentionally unversioned package by gitignoring it
+packages/dynamic-*

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.

commands/publish/__tests__/git-checkout.test.js

@@ -11,20 +11,24 @@ 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("");
 });
 
 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");
 });

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");

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",

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.");
     });

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);
 }

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.

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,34 +23,42 @@ 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");
 });
 
 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");
 });

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 () => {

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.",

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;

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);
 }