Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] npm pack does not include certain README/LICENSE/NOTICE files in archive #3334

Open
1 task done
hanazuki opened this issue May 30, 2021 · 3 comments
Open
1 task done
Labels
Bug thing that needs fixing Needs Discussion is pending a discussion Priority 2 secondary priority issue Release 7.x work is associated with a specific npm 7 release

Comments

@hanazuki
Copy link

hanazuki commented May 30, 2021

Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

Documentation for files field in package.json says:

Certain files are always included, regardless of settings:
[...]

  • README
  • CHANGES / CHANGELOG / HISTORY
  • LICENSE / LICENCE
  • NOTICE

[...]
README, CHANGES, LICENSE & NOTICE can have any case and extension.

and npm@6 seems to follow these statements.

But,

  1. npm@7 does not include CHANGES/CHANGELOG/HISTORY/NOTICE files.
  2. npm@7 does not include files with these names within sub-directories. Though this behavior is not explicitly stated in the public documentation, this code comment suggests they should be included for compatibility. But actually they aren't.

README/LICENSE/NOTICE files are usually required when redistributing third-party libraries. This change without documentation makes the people doing code vendoring go against OSS licenses without noticing if they depend on the old behavior.

Expected Behavior

npm@7 should include these files in the generated archive as documented and also as npm@6 did. Or, update the documents to reflect this breaking change.

Steps To Reproduce

  1. Create package.json in an empty directory with the following content:
{
    "name": "npm-pack-test",
    "version": "1.0.0",
    "main": "index.js",
    "files": [
        "vendor/foo/index.js"
    ]
}
  1. Place some other files:
% tree
.
├── NOTICE
├── index.js
├── package.json
└── vendor
    └── foo
        ├── LICENSE
        └── index.js

2 directories, 5 files
  1. Run npm pack --dry to see which files are included in the archive.

Following are the npm pack results on my machine using npm@6 and npm@7. NOTICE and LICENSE files are not included when using npm@7.

% nvm use system
Now using system version of node: v14.17.0 (npm v6.14.13)

% npm pack --dry
npm notice
npm notice 📦  npm-pack-test@1.0.0
npm notice === Tarball Contents ===
npm notice 12B  vendor/foo/LICENSE
npm notice 4B   NOTICE
npm notice 22B  index.js
npm notice 8B   vendor/foo/index.js
npm notice 132B package.json
npm notice === Tarball Details ===
npm notice name:          npm-pack-test
npm notice version:       1.0.0
npm notice filename:      npm-pack-test-1.0.0.tgz
npm notice package size:  333 B
npm notice unpacked size: 178 B
npm notice shasum:        0e013b61a543bea4dd7f1f4d54484e8520fb683a
npm notice integrity:     sha512-+eiED46kbx/27[...]TaHU3RgxdlIfg==
npm notice total files:   5
npm notice
npm-pack-test-1.0.0.tgz

% nvm use 16
Now using node v16.1.0 (npm v7.15.0)

% npm pack --dry
npm notice
npm notice 📦  npm-pack-test@1.0.0
npm notice === Tarball Contents ===
npm notice 22B  index.js
npm notice 132B package.json
npm notice 8B   vendor/foo/index.js
npm notice === Tarball Details ===
npm notice name:          npm-pack-test
npm notice version:       1.0.0
npm notice filename:      npm-pack-test-1.0.0.tgz
npm notice package size:  256 B
npm notice unpacked size: 162 B
npm notice shasum:        c81f71e039dc21eb958e565ddb7a081da28c52f3
npm notice integrity:     sha512-QCiLAdr9r+GWd[...]GXSxWD/vIXirg==
npm notice total files:   3
npm notice
npm-pack-test-1.0.0.tgz

Environment

  • OS: Debian sid
  • Node: v16.1.0 (installed using nvm)
  • npm: v7.15.0
@hanazuki hanazuki added Bug thing that needs fixing Needs Triage needs review for next steps Release 7.x work is associated with a specific npm 7 release labels May 30, 2021
@hanazuki
Copy link
Author

hanazuki commented May 30, 2021

I found that not including CHANGES/CHANGELOG/HISTORY/NOTICE is an intended change introduced by npm/npm-packlist#61.
But, I think NOTICE should be treated like COPYING and LICENSE, as it is designated for inclusion by the Apache License, Version 2.0.

@wraithgar wraithgar added Priority 2 secondary priority issue Needs Discussion is pending a discussion and removed Needs Triage needs review for next steps labels Jun 18, 2021
@leilapearson
Copy link

The developers guide still says that CHANGELOG is always included:

https://docs.npmjs.com/cli/v8/using-npm/developers

@leilapearson
Copy link

Also the packlist README.md file still says "If a package.json file is found, and it has a files list, then ignore everything that isn't in files. Always include the readme, license, notice, changes, changelog, and history files, if they exist, and the package.json file itself."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Needs Discussion is pending a discussion Priority 2 secondary priority issue Release 7.x work is associated with a specific npm 7 release
Projects
None yet
Development

No branches or pull requests

3 participants