fix(publish): Avoid errors when files are ignored by git

[fictivecreations]

Change git-add command to use the '.' syntax so as only to add files that aren't in the .gitignore that have been changed.

Description

Instead of adding files by their specific names (which would cause git to complain if one of those files was included in the .gitignore file), we use the git add . syntax to smartly grab any files that have been changed.

Motivation and Context

This fixes issue #2151 , where if you have projects that you are entirely generating and want to be able to publish without commiting to your github repo, you are now able to do so.

How Has This Been Tested?

This has been run through the lerna tests successfully and it was tested in practice against my repo with a similar use-case to the one listed above with successful results.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

I don't think this needs new tests since the git-add tests still cover the functionality that it's supposed to accomplish. And while it might need documentation I wouldn't be sure where it should be placed so if anyone has input on those two, I'd be happy to update accordingly.

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• All new and existing tests passed.

[evocateur]

You need to add tests. Merely passing existing ones simply exposes how shoddy the existing test coverage is.

[fictivecreations]

@evocateur
Okay, I've added some unit tests to be able to cover the .gitignore scenario.

I've also added a change and tests to the git-checkout command because we found that was also erroring out for the same reason on the githead edits. So that should be clearing out correctly now.

[ediabal]

Would really love to see this added.

[evocateur]

Thanks for adding the test!

[alasdairhurst]

Why wasn't this marked as a breaking change? Definitely broke our process because we only expected lerna to commit the changes that it made and no others.

Files were generated/copied around as part of our build process and specifically weren't committed, and they have now been published and pushed back to source after this change.

Previously these files were just ignored because they weren't explicitly added, but now we've only just noticed that we have extra CI files floating around in our git repo and published packages for the past few days.

Now the reason why our process is like that is questionable in itself :) We shouldn't be copying files into packages like that but we also didn't expect the behavior to change in a significant way like this, or at least be able to opt-into the breaking change and handle it accordingly by properly gitignoring the files.

I just hope that someone doesn't have something with credentials or secrets checked into their packages/public repos now.

[grxy]

I just hope that someone doesn't have something with credentials or secrets checked into their packages/public repos now.

😬 This happened to me. My .npmrc (only created in CI to auto-publish updates) was committed by lerna when renovate merged an update to my master branch. Fortunately, NPM caught this before my tokens could be compromised, but I'm certain there are many more cases like this with higher risk potential than my own personal repo.

[jmgaya]

I just hope that someone doesn't have something with credentials or secrets checked into their packages/public repos now.

Same problem here 😞

Adding files outside "Lerna scope" seems a bit dangerous...

Maybe parsing the .gitignore and then filtering the files array fits better given the original bug description?

I could take care of it if you agree @evocateur 👍

[evocateur]

I did not deem this a major because I thought people with secrets on their filesystem would be prudent enough to add them to their .gitignore. Clearly I was foolish, in this regard.

Guess I'll stick it behind an option, now. Struggling to name it, though...

[evocateur]

think i'm gonna go with --no-granular-pathspec, a boolean option which would be configured in lerna.json as "granularPathspec": false

(the default will be flipped in the next major, preserving pre-3.22.0 behavior for the rest of the 3.x range)

[evocateur]

(definition of pathspec, for those interested in git terminology)

[jmgaya]

I did not deem this a major because I thought people with secrets on their filesystem would be prudent enough to add them to their .gitignore. Clearly I was foolish, in this regard.

I won't personally stretch those cases to secrets only. Someone with local changes who's not aware of this behavior could execute lerna version command without removing those changes. Moreover, in some scenarios, you could edit something just for manual testing purposes locally before releasing a new version, in order to verify a feature accomplishes its proposal.

Thanks for your time and patience🤘 !

[aprilandjan]

@evocateur although there is currently the optiongranularPathspec to decide pathspec, is it still necessary to exclude all ignored files of all changed files passed to gitAdd function to avoid error(#2522 for example), especially when people do not use --no-granular-pathspec?