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

npx runner is preferred over Swift even if the Swift runner is configured #667

Closed
jmccance opened this issue Mar 4, 2024 · 3 comments · Fixed by #668
Closed

npx runner is preferred over Swift even if the Swift runner is configured #667

jmccance opened this issue Mar 4, 2024 · 3 comments · Fixed by #668
Labels
bug Something isn't working

Comments

@jmccance
Copy link

jmccance commented Mar 4, 2024

🔧 Summary

When npx is available on the path, the hook scripts will never use the Swift runner.

This can be a problem if a developer has an incorrectly configured Node environment and is trying to commit to a Swift project configured with the SPM plugin.

Lefthook version

1.6.4

Steps to reproduce

  1. Set up a project with a Package.swift file and the Swift lefthook-plugin.
  2. Ensure that lefthook is not available through any of the fallbacks that come before npx.
  3. Ensure that an executable called npx is available in your path. (For testing purposes, can do alias npx=false.)
  4. Install a pre-commit hook.
  5. Attempt to commit.

Expected results

The commit should complete successfully using the Swift Lefthook plugin.

Actual results

npx is used to try to run Lefthook.

Possible Solution

Re-order the fallbacks so the Swift test comes before the check for the npx command.

Logs / Screenshots

Not much to show here, since Lefthook never actually runs. What one of our engineers ran into, though, was that their broken Node environment resulted in this error when trying to commit to our Swift repo with Lefthook enabled.

CleanShot 2024-03-04 at 16 13 05

@jmccance jmccance added the bug Something isn't working label Mar 4, 2024
@jmccance
Copy link
Author

jmccance commented Mar 4, 2024

Looks like #653 would also be a possible solution if re-ordering the fallbacks isn't desired.

If it re-ordering is acceptable, let me know and I can open a PR.

@mrexox
Copy link
Member

mrexox commented Mar 5, 2024

Thank you for creating an issue. I will release the fix this week

@jmccance
Copy link
Author

jmccance commented Mar 6, 2024

Excellent, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants