Skip to content

Match Bazel env attribute behaviour#52

Merged
Silic0nS0ldier merged 2 commits intomainfrom
jordan-mele_run-env-info-provider
Mar 13, 2024
Merged

Match Bazel env attribute behaviour#52
Silic0nS0ldier merged 2 commits intomainfrom
jordan-mele_run-env-info-provider

Conversation

@Silic0nS0ldier
Copy link
Copy Markdown
Contributor

@Silic0nS0ldier Silic0nS0ldier commented Mar 13, 2024

This PR;

Since environment variables are now passed along via a provider, they won't be an input for building nodejs_binary and nodejs_test targets.

TODO

  • Test under build.
    Does not work in genrule.
    According to Bazel docs, env was never supposed to apply to builds. This is a bug in Rules NodeJS v3.8.
  • Test under test.
  • Test under run.
  • Test under run --script_path

@Silic0nS0ldier Silic0nS0ldier marked this pull request as ready for review March 13, 2024 01:53
@Silic0nS0ldier Silic0nS0ldier merged commit 04d179e into main Mar 13, 2024
Comment thread internal/node/node.bzl
# since this will be called from a nodejs_test, where the entrypoint is going to be the test file
# we shouldn't add the entrypoint as a attribute to collect here
coverage_common.instrumented_files_info(ctx, dependency_attributes = ["data"], extensions = ["js", "ts"]),
RunEnvironmentInfo(expanded_env),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This means it doesn't get the right env when run directly in bazel-bin right? Or when run as a data dependency in the runfiles of something else?

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.

3 participants