bazel-io-cp30043-9.2.0-111558: fix labels broken by src/main/res rename#30184
Closed
rdesgroppes wants to merge 2 commits into
Closed
bazel-io-cp30043-9.2.0-111558: fix labels broken by src/main/res rename#30184rdesgroppes wants to merge 2 commits into
rdesgroppes wants to merge 2 commits into
Conversation
…azelbuild#30043) ### Description This reverts commit f5aeb71, relanding bazelbuild#29822. Additional fix: the change makes `//src/tools/launcher:launcher` depend on a `windows_resources` target, but its embedded `@bazel_tools` copy had no rc toolchain for `toolchain_type`, so the postsubmit `bazel_bootstrap_distfile_test` (which rebuilds `@bazel_tools` from source, unlike presubmit) could not bootstrap Bazel on Windows. The resource-compiler package lived at `src/main/res`, outside `@bazel_tools`, so embedding it there required `@bazel_tools`-qualified labels and hand-staging the package in the lockfile test. Move it to `tools/res` instead, beside the other embedded toolchains, so it is embedded and staged like its siblings and labels resolve relative to the loading repo. Each module must then register its own rc toolchain, so this registers `@local_config_winsdk//:all` and `empty_rc_toolchain` in `src/MODULE.tools` (as in the root `MODULE.bazel`) and drops the now redundant `empty_rc_toolchain` registration from `testBuildLaunchersWithClangClOnWindows`. Verified with `bazel_bootstrap_distfile_test` on Windows: ``` bazel.exe test //src/test/shell/bazel:bazel_bootstrap_distfile_test [...] INFO: Build completed successfully, 2 total actions //src/test/shell/bazel:bazel_bootstrap_distfile_test PASSED in 732.8s Executed 1 out of 1 test: 1 test passes. ``` ### Motivation Same as bazelbuild#29822: > On Windows, manifest-less PE executables whose name contains keywords like "install", "setup", "update", or "patch" trigger the "[UAC installer-detection heuristic](https://www.advancedinstaller.com/user-guide/vista-uac.html)" and demand elevation, even when the target does nothing privileged. > > When running such executables without admin privileges, one indeed face errors resembling: > ``` > FATAL: ExecuteProgram(C:\[...]\install.exe) failed: ERROR: src/main/native/windows/process.cc(189): CreateProcessW("C:\[...]\install.exe"): The requested operation requires elevation. > (error: 740) > ``` > > Declaring `asInvoker` in the launcher stub bypasses the heuristic, similar to rust-lang/cargo#393 (`cargo update` triggered the same heuristic): cargo now ships a `windows.manifest.xml` embedded via its build script. > > Because the manifest lives in the PE resource section, `launcher_maker`'s byte-copy propagates it to every generated binary automatically. Fixes bazelbuild#29819. (for real) ### Build API Changes No ### Checklist - [x] I have added tests for the new use cases (if any). - [ ] I have updated the documentation (if applicable). ### Release Notes RELNOTES[NEW]: Windows launcher stubs now embed an `asInvoker` UAC manifest, preventing "The requested operation requires elevation. (error: 740)" for targets whose name matches the installer-detection heuristic. Closes bazelbuild#30043. PiperOrigin-RevId: 944416012 Change-Id: I37a388ad42ea8192af454a978de39dbe4ff2d100
2 tasks
rdesgroppes
force-pushed
the
bazel-io-cp30043-9.2.0-111558-fixup
branch
2 times, most recently
from
f8e9ed1 to
e72d8bc
Compare
rdesgroppes
changed the title
This is a minimal fixup to unblock bazelbuild#30182. bazelbuild#30182 cherry-picks the src/main/res to tools/res rename from bazelbuild#30043 onto release-9.2.0, but leaves src/test/res/BUILD loading the old //src/main/res:win_res.bzl label, breaking package loading for every bazel query/test invocation on all platforms once the rename lands. master does not hit this because src/test/res was deleted there in dc5e25d ("Delete obsolete and unused Windows resource and rule test rules", closes bazelbuild#27918), before the rename landed. release-9.2.0 predates that deletion. This only repoints the load() to the new tools/res location rather than deleting src/test/res on this branch. resource_compiler_toolchain_test hits the same issue via the now-gone //src/main/res:srcs_for_testing and hardcoded src/main/res paths. Repoint it to //tools/res:srcs (widening that filegroup's visibility) instead of deleting the test too.
rdesgroppes
force-pushed
the
bazel-io-cp30043-9.2.0-111558-fixup
branch
from
e72d8bc to
044efd0
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This is a minimal fixup to unblock #30182.
Motivation
#30182 cherry-picks the src/main/res to tools/res rename from #30043 onto release-9.2.0, but leaves src/test/res/BUILD loading the old //src/main/res:win_res.bzl label, breaking package loading for every bazel query/test invocation on all platforms once the rename lands.
master does not hit this because src/test/res was deleted there in dc5e25d ("Delete obsolete and unused Windows resource and rule test rules", closes #27918), before the rename landed.
release-9.2.0 predates that deletion.
This only repoints the load() to the new tools/res location rather than deleting src/test/res on this branch.
resource_compiler_toolchain_test hits the same issue via the now-gone //src/main/res:srcs_for_testing and hardcoded src/main/res paths.
Repoint it to //tools/res:srcs (widening that filegroup's visibility) instead of deleting the test too.
Build API Changes
No
Checklist
Release Notes
RELNOTES: None