Skip to content

Prevent most side effects of yanked modules #18698

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.

Sign up for GitHub

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

Jump to bottom
Closed
fmeum wants to merge 2 commits into from
Closed

Prevent most side effects of yanked modules #18698

fmeum wants to merge 2 commits into from

Conversation

@fmeum
Copy link
Collaborator

@fmeum fmeum commented Jun 15, 2023 •
edited
Loading

Yanked module versions no longer contribute dependency requirements or emit DEBUG messages for print() statements.

Since the module files of yanked modules are still evaluated to learn their compatibility levels, they can still fail to execute.

@fmeum fmeum force-pushed the ignore-yanked-module-files branch 7 times, most recently from 008e619 to 60cd9f9 Compare June 20, 2023 21:09
@fmeum
Prevent most side effects of yanked modules
7e98fbc 
Yanked module versions no longer contribute dependency requirements or
emit `DEBUG` messages for `print()` statements.

Since the module files of yanked modules are still evaluated to learn
their compatibility levels, they can still fail to execute.
@fmeum fmeum force-pushed the ignore-yanked-module-files branch from ca0a1d1 to 7e98fbc Compare June 21, 2023 17:44
@fmeum fmeum changed the title Do not evaluate module files of yanked modules Prevent most side effects of yanked modules Jun 21, 2023
@fmeum fmeum marked this pull request as ready for review June 22, 2023 15:40
@fmeum fmeum requested review from a team, Wyverald and meteorcloudy as code owners June 22, 2023 15:40
@fmeum fmeum requested review from sdtwigg and removed request for a team and sdtwigg June 22, 2023 15:40
@github-actions github-actions bot added awaiting-review PR is awaiting review from an assigned reviewer team-Configurability platforms, toolchains, cquery, select(), config transitions team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. labels Jun 22, 2023
@fmeum fmeum mentioned this pull request Jun 22, 2023
Open
@fmeum
Copy link
Collaborator Author

fmeum commented Jun 22, 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 Jun 22, 2023
@iancha1992
Copy link
Member

iancha1992 commented Jun 22, 2023

@bazel-io fork 6.3.0

@bazel-io bazel-io mentioned this pull request Jun 22, 2023
Closed
@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 Jun 22, 2023
Wyverald
Wyverald reviewed Jun 23, 2023
View reviewed changes
@fmeum fmeum requested a review from Wyverald June 25, 2023 19:11
@aiuto aiuto removed the team-Configurability platforms, toolchains, cquery, select(), config transitions label Jun 26, 2023
@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 Jun 27, 2023
@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 Jun 28, 2023
@iancha1992
Copy link
Member

iancha1992 commented Jun 28, 2023

@Wyverald @fmeum Looks like in order to cherry-pick this to release-6.3.0, we'll need another commit/PR ("src/test/java/com/google/devtools/build/lib/rules/starlarkdocextract/StarlarkDocExtractTest.java" is missing). Could you please direct me to a commit/PR that helps? Thanks

@fmeum
Copy link
Collaborator Author

fmeum commented Jun 28, 2023

@iancha1992 Pretty sure that particular part of the diff can just be dropped as that file isn't part of 6.3.0.

@iancha1992
Copy link
Member

iancha1992 commented Jun 28, 2023

@fmeum Sorry there's also one more conflict from

  • "src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunction.java" (conflicts caused by "compute" function which throws SkyFunctionException in the release-6.3.0 branch, but throws BazelModuleResolutionFunctionException exception in the master branch. Also "allowedYankedVersionsFromEnv" variable is no longer there in the master branch)

@fmeum fmeum deleted the ignore-yanked-module-files branch June 29, 2023 05:35
Wyverald pushed a commit that referenced this pull request Jul 11, 2023
@fmeum @Wyverald
Prevent most side effects of yanked modules
3de8049 
Yanked module versions no longer contribute dependency requirements or emit `DEBUG` messages for `print()` statements.

Since the module files of yanked modules are still evaluated to learn their compatibility levels, they can still fail to execute.

Closes #18698.

PiperOrigin-RevId: 544059396
Change-Id: I8a37d5c7975947cd717f6e56d97cce467f22178e
@Wyverald Wyverald mentioned this pull request Jul 11, 2023
Merged
iancha1992 pushed a commit that referenced this pull request Jul 11, 2023
@Wyverald @fmeum
Prevent most side effects of yanked modules (#18908)
da329e3 
Yanked module versions no longer contribute dependency requirements or emit `DEBUG` messages for `print()` statements.

Since the module files of yanked modules are still evaluated to learn their compatibility levels, they can still fail to execute.

Closes #18698.

PiperOrigin-RevId: 544059396
Change-Id: I8a37d5c7975947cd717f6e56d97cce467f22178e

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

Reviewers

@Wyverald Wyverald Wyverald approved these changes

@meteorcloudy meteorcloudy Awaiting requested review from meteorcloudy meteorcloudy is a code owner

Assignees

No one assigned

Labels

team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@fmeum @iancha1992 @Wyverald @aiuto @bazel-io