Fixed the salt.modules.npm module to check the npm version correctly on the Windows platform #51813
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ction name that's likely not to change during refactoring...
…or a function name that's likely not to change during refactoring..." This reverts commit 7c277b7.
…to npm in order to check its version.
arizvisa
added a commit
to arizvisa/lolfuzz3
that referenced
this pull request
Feb 25, 2019
…that fixes the version detection in the npm module. Wtf dude...
twangboy
approved these changes
Feb 26, 2019
garethgreenaway
added a commit
to garethgreenaway/salt
that referenced
this pull request
Sep 19, 2019
dwoz
added a commit
that referenced
this pull request
Dec 19, 2019
[master] Porting #51813 to master
arizvisa
added a commit
to arizvisa/lolfuzz3
that referenced
this pull request
May 24, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
On the Windows platform, the
salt.modules.npmmodule does not check the npm version correctly. This is likely because thenpmscript that is installed is actuallynpm.CMD. When checking the version, thesalt.modules.npmmodule attempts to call justnpm.This PR locates
npmby usingsalt.utils.path.which()to detect the path before checking the version. The version check usessalt.modules.cmdmod.runwhereas the rest of the module usessalt.modules.cmdmod.run_all. It was not determined whysalt.modules.cmdmod.runacts different fromsalt.modules.cmdmod.run_allwhen running a cmd-script on the Windows platform.What issues does this PR fix or reference?
This closes issue #51811.
Previous Behavior
When performing the version check, the module is unable to locate the npm binary (or script, really) and thus the version check fails.
New Behavior
Due to checking for the full path of
npm.cmd, the version is correctly detected and the rest of the module works. (npm.install,npm.uninstall, andnpm.listwas checked).Tests written?
No
Commits signed with GPG?
No