Skip to content

feat(bzlmod): Use a common constant for detecting bzlmod being enabled#1302

Merged
rickeylev merged 1 commit intobazel-contrib:mainfrom
chrislovecnm:bzlmod-enabled
Jul 10, 2023
Merged

feat(bzlmod): Use a common constant for detecting bzlmod being enabled#1302
rickeylev merged 1 commit intobazel-contrib:mainfrom
chrislovecnm:bzlmod-enabled

Conversation

@chrislovecnm
Copy link
Copy Markdown
Contributor

@chrislovecnm chrislovecnm commented Jul 10, 2023

Various parts of the codebase detect whether bzlmod is enabled or not. Most of them copy/paste the same if @ in Label(..) trick and use a comment to explain what they're doing.

Rather than copy/paste that everywhere, this commit uses a constant defined that does this once and reuses the constant value to determine if bzlmod is enabled.

Closes: #1295

@chrislovecnm
Copy link
Copy Markdown
Contributor Author

@rickeylev Thoughts on me breaking the BZLMOD_ENABLED to a separate file? Otherwise, we have to depend on bazel_skylib in a bunch of new places.

@chrislovecnm chrislovecnm force-pushed the bzlmod-enabled branch 2 times, most recently from 98a1377 to d988458 Compare July 10, 2023 19:29
Various parts of the codebase detect whether bzlmod is enabled or not.
Most of them copy/paste the same if @ in Label(..) trick, and use a
comment to explain what they're doing.

Rather that copy/paste that everywhere, this commit uses a constant defined
that does this once, then code can simply check "if BZLMOD_ENABLED".

This commit also refactors the BZLMOD_ENABLED variable to its own file,
so that we are not loading skylib all of the time.
@rickeylev
Copy link
Copy Markdown
Collaborator

@rickeylev Thoughts on me breaking the BZLMOD_ENABLED to a separate file? Otherwise, we have to depend on bazel_skylib in a bunch of new places.

SGTM. From the error you shared, I think the issue is the skylib dependency becomes newly introduced during the workspace/module-extension phase. Which yeah, if we don't actually need it during that phase, +1 to refactor things such that it doesn't need to be loaded.

@rickeylev rickeylev enabled auto-merge July 10, 2023 19:34
@rickeylev rickeylev added this pull request to the merge queue Jul 10, 2023
Merged via the queue into bazel-contrib:main with commit a068d1b Jul 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: bzlmod bzlmod work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use a common constant for detecting bzlmod being enabled

2 participants