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

Support relative paths for Bin hooks #468

Merged
merged 2 commits into from
Jun 18, 2019

Conversation

charlespierce
Copy link
Contributor

@charlespierce charlespierce commented Jun 11, 2019

Closes #459

Info

  • The Hooks configuration has an option bin that allows you to specify a command to run to resolve the hook.
  • Currently, that command is run relative to the current directory, so if you specify a relative path in hooks.json, it will only work if you are in the correct directory.
  • The easiest way to specify hooks will be with a relative path to the script, so we should support this use-case.

Changes

  • Updated the hooks deserialization to propagate the base directory that the hooks.json file is found in to any Bin hooks.
  • If the command is detected as being a relative path (if it begins with .), then we execute it relative to the base directory found above.
  • Added detection for non-zero exit codes, so that if the hook command returns a failure code, we throw a Volta error instead of continuing as if the command succeeded.
  • Also updated stderr to inherit from the parent process, so that bin hooks can use it to show progress bars or waiting spinners, as well as report errors directly to the user.

Tested

  • Verified that a bin hook with a relative path correctly executes anywhere in a project, as opposed to only from a single directory.

Copy link
Contributor

@chriskrycho chriskrycho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall! Had one small thing which needs a tweak; otherwise 👍

crates/volta-core/src/hook/tool.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@chriskrycho chriskrycho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚡️

@charlespierce charlespierce merged commit b67ed12 into volta-cli:master Jun 18, 2019
@charlespierce charlespierce deleted the relative_path_hooks branch June 18, 2019 22:10
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.

Bin Hooks should allow relative paths
2 participants