Skip to content

fix copy_dynamic_libraries_to_binary for subdirectories#623

Open
novas0x2a wants to merge 1 commit intobazelbuild:mainfrom
novas0x2a:fix-windows-dlls
Open

fix copy_dynamic_libraries_to_binary for subdirectories#623
novas0x2a wants to merge 1 commit intobazelbuild:mainfrom
novas0x2a:fix-windows-dlls

Conversation

@novas0x2a
Copy link

This is a new instance of a similar problem described in bazelbuild/bazel#12448. In order for windows binaries to run, the (non-system) dlls need to be copied to the same directory. However, the code that determined that was only looking at the package label and the workspace, but not the actual subdirectory. Previously, this was fixed by adding the lib.is_source check, but this only solves it for source files.

An example of code that can trigger this is https://github.com/bazel-contrib/rules_foreign_cc/blob/d8e34f6f9b66a13cd845011200d444b060dc1af1/examples/cmake_hello_world_lib/shared/BUILD.bazel

@novas0x2a novas0x2a force-pushed the fix-windows-dlls branch 2 times, most recently from dbe9bbe to 166637a Compare March 6, 2026 22:16
@novas0x2a
Copy link
Author

Force-pushed over it to retry the CI, which failed with a GOAWAY.

novas0x2a added a commit to bazel-contrib/rules_foreign_cc that referenced this pull request Mar 7, 2026
This fails due to a bug in bazel / in rules_cc (depending on bazel
version). See bazelbuild/rules_cc#623
novas0x2a added a commit to bazel-contrib/rules_foreign_cc that referenced this pull request Mar 7, 2026
This fails due to a bug in bazel / in rules_cc (depending on bazel
version). See bazelbuild/rules_cc#623
novas0x2a added a commit to bazel-contrib/rules_foreign_cc that referenced this pull request Mar 9, 2026
…1475)

This fails due to a bug in bazel / in rules_cc (depending on bazel
version). See bazelbuild/rules_cc#623. With
upstream's approval I plan to roll this fix out everywhere.
Copy link
Collaborator

@armandomontanez armandomontanez left a comment

Choose a reason for hiding this comment

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

This does look more correct, but this issue is subtle enough that I think it deserves a test so it doesn't regress.

@novas0x2a novas0x2a force-pushed the fix-windows-dlls branch 5 times, most recently from 3928445 to 97f3d58 Compare March 10, 2026 22:09
This is a new instance of a similar problem described in
bazelbuild/bazel#12448. In order for windows
binaries to run, the (non-system) dlls need to be copied to the same
directory. However, the code that determined that was only looking at
the package label and the workspace, but not the actual subdirectory.
Previously, this was fixed by adding the lib.is_source check, but this
only solves it for source files.
@armandomontanez armandomontanez added type: bug Something that should be working isn't working P2 We'll consider working on this in future. (Assignee optional) category: core rules labels Mar 10, 2026
Copy link
Collaborator

@armandomontanez armandomontanez left a comment

Choose a reason for hiding this comment

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

Thank you for this!

novas0x2a added a commit to novas0x2a/bazel that referenced this pull request Mar 10, 2026
This is a new instance of a similar problem described in
bazelbuild#12448. In order for windows
binaries to run, the (non-system) dlls need to be copied to the same
directory. However, the code that determined that was only looking at
the package label and the workspace, but not the actual subdirectory.
Previously, this was fixed by adding the lib.is_source check, but this
only solves it for source files.

This is a crossport from rules_cc; See bazelbuild/rules_cc#623
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category: core rules P2 We'll consider working on this in future. (Assignee optional) type: bug Something that should be working isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants