search

cgrindel / rules_swift_package_manager

Collection of utilities and Bazel rules to aid in the development and maintenance of Swift repositories using Bazel.
Apache License 2.0
68 stars 22 forks source link

fix: specify all defines as local using `copts` #1094

Closed cgrindel closed 1 month ago

cgrindel commented 1 month ago

In an attempt to localize defines to specific targets, we specify defines/macros for all target types (e.g., swift_library, objc_library, and cc_library) targets using copts.

Fixes #1093.

brentleyjones commented 1 month ago

We can emulate local_defines with copts though. We should do so on both swift_library and objc_library.

cgrindel commented 1 month ago

We can emulate local_defines with copts though. We should do so on both swift_library and objc_library.

@brentleyjones Can you elaborate on that? Do you mean specifying -DSWIFT_PACKAGE=1 directly?

brentleyjones commented 1 month ago

Yes :+1:. But without =1 for swift_library.

cgrindel commented 1 month ago

So, do we want to keep all defines local or just the SWIFT_PACKAGE? We apply other define values, as well. They can be specified as build settings. I am checking the source to see what it does, because the doc is not clear.

cgrindel commented 1 month ago

I don't see any code that propagates the define values. So, I will update the code to set all defines as local.

@brentleyjones Is there any background why the defines for swift_library propagate? Is it just matching the cc_library behavior?

brentleyjones commented 1 month ago

Yes. It would make more sense to behave that way if you could have a source-less swift_library, but at least it's consistent with cc_library.

cgrindel commented 1 month ago

@brentleyjones @luispadron This PR is ready for review. It now uses copts for defines.