Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

build: npm package for v1.1.14-v1.1.16 contains .git folder #11684

Closed
Noglen opened this issue Mar 19, 2019 · 20 comments · Fixed by #11695
Closed

build: npm package for v1.1.14-v1.1.16 contains .git folder #11684

Noglen opened this issue Mar 19, 2019 · 20 comments · Fixed by #11695
Assignees
Labels
has: Pull Request A PR has been created to address this issue P2: required Issues that must be fixed. resolution: fixed severity: regression This issue is related to a regression type: bug
Milestone

Comments

@Noglen
Copy link
Contributor

Noglen commented Mar 19, 2019

Bug, enhancement request, or proposal:

Bug

Detailed Reproduction Steps:

  1. npm install angular-material@1.1.14
  2. check the node_modules\angular-material directory

What is the expected behavior?

there is no .git folder

What is the current behavior?

there is a .git folder

What is the use-case or motivation for changing an existing behavior?

npm doesn't like having sub modules or repos in the node_modules folder, and complains with the following error

npm ERR! git {myproject}\node_modules\angular-material: Appears to be a git repo or submodule.
npm ERR! git {myproject}\node_modules\angular-material
npm ERR! git Refusing to remove it. Update manually,
npm ERR! git or move it out of the way first.

Which versions of AngularJS, Material, OS, and browsers are affected?

  • AngularJS Material: 1.1.14

Is there anything else we should know? Stack Traces, Screenshots, etc.

@Splaktar Splaktar self-assigned this Mar 19, 2019
@Splaktar Splaktar added this to the 1.1.15 milestone Mar 19, 2019
@Splaktar Splaktar added type: bug needs: investigation The cause of this issue is not well understood and needs to be investigated by the team or community severity: regression This issue is related to a regression P1: urgent Urgent issues that should be addressed in the next minor or patch release. labels Mar 19, 2019
@Splaktar Splaktar changed the title npm package for v1.1.14 contains .git folder build: npm package for v1.1.14 contains .git folder Mar 19, 2019
@Splaktar
Copy link
Member

Screen Shot 2019-03-19 at 01 26 21

I can confirm that I am seeing this as well and that this wasn't the case in 1.1.13. I also saw the same error message about refusing to remove it when trying to install 1.1.13 to test.

@Splaktar
Copy link
Member

@Splaktar
Copy link
Member

We don't use .npmignore in https://github.com/angular/bower-material (where npm publish is run from), but this seems like a bug in some version of NPM that we have never published with before.

@mmalerba can you please let us know what version of NPM was used for npm publish yesterday?

@Splaktar
Copy link
Member

node 8.15.0 and npm 6.9.0 were used for the 1.1.14 release. It's not sure what was used previously, but I'll check with someone else who recently published to see what they are using.

@Splaktar
Copy link
Member

Splaktar commented Mar 19, 2019

The last 4 releases were created by @mmalerba and @jelbourn. The 1.1.12 release by @mmalerba in January did not have a .git directory in the publish, but he said that he has re-imaged his machine since then. @jelbourn is currently out on vacation.

There is an abandoned npm bug where npm publish on Linux can include the .git directory since npm 5.6.0.

However, this npm blog post on npm publish clearly states that

npm will always ignore these file name patterns:

  • .git, hg , .svn, CVS version control directories

The NPM blog post also suggests that we not use .npmignore together with .gitignore since using .npmignore means that your .gitignore is no longer consulted when doing npm publish.

This clearly looks like a bug in NPM. @mmalerba since you are the only one who can investigate this further or report it, can you please report this to NPM?

We'll also need to get another release out soon that doesn't include this .git directory as it breaks people's projects, especially when upgrading to the next version of AngularJS Material.

@Splaktar Splaktar removed their assignment Mar 19, 2019
@Splaktar
Copy link
Member

Before we do the release to fix this, we should use the --dry-run for npm publish to verify that the .git folder and contents are not in the list of published files.

@middiu
Copy link

middiu commented Mar 23, 2019

same problem here, new version contains .git folder

@Splaktar
Copy link
Member

We're planning to put out the 1.1.15 release next week to resolve this. I wanted to do it today, but I didn't hear back on a couple of presubmits and ended up getting busy with other things.

@Livshitz
Copy link

Same here, 1.1.14 has .git in it's npm package. Causing future npm install to fail.

Downgrading to 1.1.13 by setting this config in package.json: "angular-material": "1.1.13" (note to remove the ^ to avoid getting automatically the latest version, e.g .14).

@marosoft
Copy link
Contributor

@Splaktar in addition to what you mention, these are some extra details:

If there’s no .npmignore file, but there is a .gitignore file, then npm will ignore the stuff matched by the .gitignore file. If you want to include something that is excluded by your .gitignore file, you can create an empty .npmignore file to override it. Like git, npm looks for .npmignore and .gitignore files in all subdirectories of your package, not only the root directory.

https://docs.npmjs.com/misc/developers#keeping-files-out-of-your-package

I could not see any .npmignore file in the repo. At the same time two files in the git ignored dist folder are published into npm.
What about using whitelisting approach by utilising file attribute in the package.json file? Are you using ! to negate the patterns in the npmignore file?

@Splaktar
Copy link
Member

@marosoft it just looks to be a bug with NPM on Linux. It's worked every other time. Changing our config adds risk of another bad release. I think that we'll just try to release from macOS until the NPM issue on Linux gets resolved.

@Splaktar Splaktar removed the needs: investigation The cause of this issue is not well understood and needs to be investigated by the team or community label Mar 26, 2019
@MrWong99
Copy link

MrWong99 commented Mar 26, 2019

Maybe this is related?
npm/npm#20213

Or this:
https://npm.community/t/npm-pack-includes-git-directory/1805

@Splaktar Splaktar self-assigned this Mar 27, 2019
@Splaktar
Copy link
Member

We've decided to investigate a change to the release script so that we never publish from the root of the repo (which we do now).

@whereisaaron
Copy link

Had the sample problem installing angular-material 1.1.14 on Windows with npm 6.4.1. Reverting to 1.1.13 fixed it.

@Splaktar
Copy link
Member

1.1.15 was just released, from a Linux machine again, and we have successfully reproduced this problem ☹️

@Splaktar
Copy link
Member

Hopefully a fixed 1.1.16 coming soon...

@Splaktar
Copy link
Member

1.1.16 released, this time from macOS, and we've also reproduced the problem there now too... we really haven't changed anything that we know of other than publishing with the latest version of npm (6.9.0). 😩

@Splaktar
Copy link
Member

Splaktar commented Mar 30, 2019

1.1.17 has been released and it does not include the .git/ directory.

It looks like it was publishing the .git/ directory from bower-material/.git/ which we clone and cd into as part of the release process. I don't know if NPM 6.9.0 adds some changed support for .git/ directories or not, but that appears to be the core issue. That directory has this .gitignore and no .npmignore. Based on that, there doesn't appear to be any kind of misconfiguration.

Possible workarounds for the future

  1. add .git/ to the bower-material/.gitignore file
  2. have the release script delete bower-material/.git/ as part of the release process

@whereisaaron
Copy link

Thanks @Splaktar, that seems to work for me. I was able to update from 1.1.13 -> 1.1.17 without the dreaded .git appearing. 👏

@Splaktar
Copy link
Member

Splaktar commented Mar 30, 2019

After looking into this some more, and editing my previous comment, it looks like solution 2 is the most likely to succeed and avoid any side effects.

@Splaktar Splaktar added the has: Pull Request A PR has been created to address this issue label Mar 30, 2019
@Splaktar Splaktar changed the title build: npm package for v1.1.14 contains .git folder build: npm package for v1.1.14-v1.1.16 contains .git folder Mar 30, 2019
@Splaktar Splaktar added P2: required Issues that must be fixed. and removed g3: sync P1: urgent Urgent issues that should be addressed in the next minor or patch release. labels Mar 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
has: Pull Request A PR has been created to address this issue P2: required Issues that must be fixed. resolution: fixed severity: regression This issue is related to a regression type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants