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

Track dev/non-dev use_extension calls #18829

Closed
wants to merge 2 commits into from

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Jul 2, 2023

By always tracking whether a given extension usage by a module has use_extension calls with and/or without dev_dependency = True instead of just for isolated extension usages, we obtain the following advantages:

  • Module extensions can use module_ctx.is_dev_dependency to learn whether the root module contains only use_extension calls with dev_dependency = True for them. This is necessary to decide whether repositories that do not directly correspond to tags (e.g. hub repos) should be marked as dev or non-dev dependencies in module_ctx.extension_metadata.
  • ModuleExtensionMetadata consistency checks of the type "no dev/non-dev imports without dev/non-dev usage" are generalized from isolated to all extensions.
  • Prepares for the removal of isDevUsage from IsolationKey in a follow-up change which will instead use the exported name of the (only) usage proxy of an isolated usage as the key.

@fmeum fmeum force-pushed the is-dev-usage branch 3 times, most recently from e6a5f5f to e2c2ddb Compare July 2, 2023 21:30
@fmeum fmeum marked this pull request as ready for review July 2, 2023 21:32
@github-actions github-actions bot added awaiting-review PR is awaiting review from an assigned reviewer team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. labels Jul 2, 2023
@fmeum fmeum force-pushed the is-dev-usage branch 3 times, most recently from a82dbc0 to d4fd4b9 Compare July 3, 2023 13:20
fmeum added 2 commits July 11, 2023 11:33
By always tracking whether a given extension usage by a module has
`use_extension` calls with and/or without `dev_dependency = True`
instead of just for isolated extension usages, we obtain the following
advantages:

* Module extensions can use `module_ctx.is_dev_dependency` to learn
  whether the root module contains only `use_extension` calls with
  `dev_dependency = True` for them. This is necessary to decide
  whether repositories that do not directly correspond to tags (e.g. hub
  repos) should be marked as dev or non-dev dependencies in
  `module_ctx.extension_metadata`.
* `ModuleExtensionMetadata` consistency checks of the type "no
  dev/non-dev imports without dev/non-dev usage" are generalized from
  isolated to all extensions.
* Prepares for the removal of `isDevUsage` from `IsolationKey` in a
  follow-up change which will instead use the exported name of the
  (only) usage proxy of an isolated usage as the key.
@fmeum fmeum requested a review from Wyverald July 11, 2023 10:09
@Wyverald Wyverald added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Jul 11, 2023
@fmeum
Copy link
Collaborator Author

fmeum commented Jul 11, 2023

@bazel-io flag

@bazel-io bazel-io added the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Jul 11, 2023
@keertk
Copy link
Member

keertk commented Jul 12, 2023

@bazel-io fork 6.3.0

@bazel-io bazel-io removed the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Jul 12, 2023
@fmeum fmeum deleted the is-dev-usage branch July 12, 2023 16:30
@github-actions github-actions bot removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Jul 12, 2023
iancha1992 pushed a commit to iancha1992/bazel that referenced this pull request Jul 12, 2023
By always tracking whether a given extension usage by a module has `use_extension` calls with and/or without `dev_dependency = True` instead of just for isolated extension usages, we obtain the following advantages:

* Module extensions can use `module_ctx.is_dev_dependency` to learn whether the root module contains only `use_extension` calls with `dev_dependency = True` for them. This is necessary to decide whether repositories that do not directly correspond to tags (e.g. hub repos) should be marked as dev or non-dev dependencies in `module_ctx.extension_metadata`.
* `ModuleExtensionMetadata` consistency checks of the type "no dev/non-dev imports without dev/non-dev usage" are generalized from isolated to all extensions.
* Prepares for the removal of `isDevUsage` from `IsolationKey` in a follow-up change which will instead use the exported name of the (only) usage proxy of an isolated usage as the key.

Closes bazelbuild#18829.

PiperOrigin-RevId: 547517851
Change-Id: I1290e53adf735a16d62e2c103f3776ecbd5b1a18
iancha1992 added a commit that referenced this pull request Jul 12, 2023
By always tracking whether a given extension usage by a module has `use_extension` calls with and/or without `dev_dependency = True` instead of just for isolated extension usages, we obtain the following advantages:

* Module extensions can use `module_ctx.is_dev_dependency` to learn whether the root module contains only `use_extension` calls with `dev_dependency = True` for them. This is necessary to decide whether repositories that do not directly correspond to tags (e.g. hub repos) should be marked as dev or non-dev dependencies in `module_ctx.extension_metadata`.
* `ModuleExtensionMetadata` consistency checks of the type "no dev/non-dev imports without dev/non-dev usage" are generalized from isolated to all extensions.
* Prepares for the removal of `isDevUsage` from `IsolationKey` in a follow-up change which will instead use the exported name of the (only) usage proxy of an isolated usage as the key.

Closes #18829.

PiperOrigin-RevId: 547517851
Change-Id: I1290e53adf735a16d62e2c103f3776ecbd5b1a18

Co-authored-by: Fabian Meumertzheim <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants