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

fix: npm pack marks the wrong files as executable #409

Conversation

kchindam-infy
Copy link

@kchindam-infy kchindam-infy commented Oct 16, 2024

This PR introduces an optional parameter to isPackageBin that allows for more flexible handling of the file path prefixes. By calling isPackageBin with replace = false in tar-create-options.js we avoid stripping the initial directory component (such as 'package/') from the file paths. This approach maintains existing functionality , pasess all the current tests, provides a clearer, more direct path-to-bin comparison without altering the established test suite behavior.

References

Related to [https://github.com/npm/cli/issues/7762]
Depends on #0
Blocked by #0
Fixes [https://github.com/npm/cli/issues/7762]
Closes [(https://github.com/npm/cli/issues/7762)]

@kchindam-infy kchindam-infy requested a review from a team as a code owner October 16, 2024 19:38
lib/util/is-package-bin.js Outdated Show resolved Hide resolved
@kchindam-infy kchindam-infy marked this pull request as draft October 22, 2024 19:00
@kchindam-infy kchindam-infy marked this pull request as ready for review November 27, 2024 18:18
@kchindam-infy
Copy link
Author

After reviewing the discussions and feedback it appears the root issue lies with npm pac incorrectly setting the chmod on the wrong file. While npm extract works as expected, the problem stems from the packing step which is outside the scope of this PR. This behavior needs further investigation to determine why npm pack modifying the chmod of the incorrect file. I am closing this PR since the underlying the issue is in the npm pack with chmod which is in place but currently broken.

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

Successfully merging this pull request may close these issues.

2 participants