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

feat(builtin): add environment attribute to yarn_install & npm_install #1596

Merged
merged 1 commit into from
Feb 5, 2020

Conversation

gregmagolan
Copy link
Collaborator

@gregmagolan gregmagolan commented Jan 31, 2020

This allows you to specify a dict of environment variables for Bazel to set before calling yarn and npm in the yarn_install and npm_install repository rules respectively.

Also set BAZEL_YARN_INSTALL to "1" and BAZEL_NPM_INSTALL to "1" respectively (unless they are set to another value by the environment attribute).

Inspired by this thread in slack: https://bazelbuild.slack.com/archives/CEZUUKQ6P/p1579828677017300

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature (please, look at the "Scope of the project" section in the README.md file)
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

This allows you to specify a dict of environment variables for Bazel to set before calling yarn and npm in the yarn_install and npm_install repository rules respectively.

Also set BAZEL_YARN_INSTALL to "1" and BAZEL_NPM_INSTALL to "1" respectively (unless they are set to another value by the environment attribute).
@gregmagolan
Copy link
Collaborator Author

gregmagolan commented Feb 1, 2020

Long slack thread linked there but the short answer is that one use case for this is when yarn is run manually outside of bazel a postinstall script would update an untracked file in the repo that would be an input to the yarn_install repository rule. This way if you run yarn outside of bazel then it will force yarn_install to run yarn again on the next bazel build.

So if your contents of node_modules become out of sync with the lock file (because you changed branches, ran yarn manually & then changed back) it will get cleared up.

@alexeagle
Copy link
Collaborator

What about using frozen-lockfile instead? It seems like we should be able to rely on the node_modules matching the lockfile - but I guess with postinstall this is not true, the lockfile is unchanged yet the node_modules doesn't match what you expect

@gregmagolan
Copy link
Collaborator Author

gregmagolan commented Feb 4, 2020

The trouble this user was having was coming up as their devs often switch between branches and run yarn manually outside of bazel. You can easily get into a situation where the node_modules are corresponding to another branch but the current package.json & yarn.lock file match up with the last time the yarn_install repo rule ran.

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

Successfully merging this pull request may close these issues.

3 participants