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

regression: 3.5.0 no longer writes license text in pnpm monorepo #1751

Closed
yume-chan opened this issue Jun 23, 2024 · 5 comments
Closed

regression: 3.5.0 no longer writes license text in pnpm monorepo #1751

yume-chan opened this issue Jun 23, 2024 · 5 comments

Comments

@yume-chan
Copy link

After upgrading to 3.5.0, I noticed that it no longer writes license text from dependencies.

Here is a minimal repro: https://github.com/yume-chan/license-test

It currently has [email protected], if you run pnpm i then pnpm build in the foo folder, the output lib/license.txt is:

Name: rollup-plugin-license
Version: 3.4.1
License: MIT
Private: false
Description: Rollup plugin to add license banner to the final bundle and output third party licenses
Repository: https://github.com/mjeanroy/rollup-plugin-license
Homepage: https://github.com/mjeanroy/rollup-plugin-license
Author: Mickael Jeanroy <[email protected]>
License Copyright:
===

The MIT License (MIT)

Copyright (c) 2016-2023 Mickael Jeanroy

Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal
in the Software without restriction, including without limitation the rights
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
copies of the Software, and to permit persons to whom the Software is
furnished to do so, subject to the following conditions:

The above copyright notice and this permission notice shall be included in all
copies or substantial portions of the Software.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
SOFTWARE.

But if I upgrade rollup-plugin-license to 3.5.0, the output becomes:

Name: rollup-plugin-license
Version: 3.5.0
License: MIT
Private: false
Description: Rollup plugin to add license banner to the final bundle and output third party licenses
Repository: https://github.com/mjeanroy/rollup-plugin-license
Homepage: https://github.com/mjeanroy/rollup-plugin-license
Author: Mickael Jeanroy <[email protected]>

In my simple investigation, fdir added in #1742 didn't resolve the symlink in node_modules folder.

I tried something like

diff --git a/src/read-file.js b/src/read-file.js
index 5852b70..d251acb 100644
--- a/src/read-file.js
+++ b/src/read-file.js
@@ -52,19 +52,17 @@ export function readFile(dir, cwd, names) {
   for (let i = 0; i < inputs.length; ++i) {
     const input = inputs[i];
     const absolutePath = path.join(dir, input);
-    const relativeToCwd = path.relative(cwd, absolutePath);
 
     const findings = finder
-      .withRelativePaths()
-      .filter(pathsMatch(relativeToCwd))
+      .withSymlinks()
+      .filter(pathsMatch(absolutePath))
       .crawl(cwd)
       .sync();
 
     const firstPath = findings[0];
 
     if (firstPath) {
-      const file = path.join(cwd, firstPath);
-      return fs.readFileSync(file, 'utf-8');
+      return fs.readFileSync(firstPath, 'utf-8');
     }
   }

It works in the minimal repro, but it's so slow in a real project that it never finishes after several minutes.

I don't really understand the code. It seems like you want to search for license/licence files in dir, but why did you do a deep search starting from cwd?

@mjeanroy
Copy link
Owner

Hi @yume-chan,
Many thanks for the report!
I just published v3.5.1 which should solved the issue.

I don't really understand the code. It seems like you want to search for license/licence files in dir, but why did you do a deep search starting from cwd?

You're right, it was unnecessarily complicated, I took this fix as an opportunity to simplify (see here).

Let me know if something does not work as expected, I'll publish a patch as quickly as possible!

@yume-chan
Copy link
Author

Thanks for your super quick response!

I have verified v3.5.1 works in my project and has good performance!

@Keavon
Copy link

Keavon commented Sep 22, 2024

@mjeanroy would you mind also looking into why it's not generating the license text for https://github.com/jakearchibald/idb-keyval? I did some digging but couldn't figure out why. Thanks :)

@mjeanroy
Copy link
Owner

Hi @Keavon,

Thank you very much for your report.

It appears files named LICENCE (LICENSE files were ok) were not detected anymore because the same fdir was re-used inappropriately, I'm very sorry for that.

It should be fixed in v3.5.3, please let me know if it's not the case.

@Keavon
Copy link

Keavon commented Sep 23, 2024

Thank you for the very prompt fix, @mjeanroy! Yes, I can confirm this solves the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants