[rules_ios] Rebase onto upstream 4.8.2, squash patches, and fix / cleanup custom patches #16
Conversation
This file contains 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
nataliejameson
force-pushed
the
nmj/upgrade-rules-ios-2
branch
from
202747f to
dbd3acd
Compare
nataliejameson
added a commit
that referenced
this pull request
Aug 13, 2024
Just rebasing this to 4.8.2 so that #16 has a cleaner looking merge. This undoes all of the changes we had, but since we rebased, it has different commit hashes. See tags/upstream-4.8.2 for the upstream commit. --------- Signed-off-by: Matt Robinson <[email protected]> Co-authored-by: Matt Robinson <[email protected]> Co-authored-by: Cong Shi <[email protected]> Co-authored-by: Jerry Marino <[email protected]> Co-authored-by: Derek Ostrander (derko) <[email protected]> Co-authored-by: Luis Padron <[email protected]> Co-authored-by: Justin Martin <[email protected]> Co-authored-by: mccorkill1 <[email protected]> Co-authored-by: Thiago Cruz <[email protected]> Co-authored-by: Matt Robinson <[email protected]> Co-authored-by: Qing Yang <[email protected]> Co-authored-by: Qing Yang <[email protected]> Co-authored-by: Steven Hepting <[email protected]> Co-authored-by: John Szumski <[email protected]> Co-authored-by: Su Xiaofeng <[email protected]> Co-authored-by: Angela Guardia <[email protected]> Co-authored-by: John Szumski <[email protected]> Co-authored-by: Karim Alweheshy <[email protected]> Co-authored-by: Karim Alweheshy <[email protected]> Co-authored-by: Cody Vandermyn <[email protected]> Co-authored-by: Cody Vandermyn <[email protected]> Co-authored-by: tymurmustafaiev <[email protected]> Co-authored-by: Timur Mustafaev <[email protected]>
Add an `apple_static_library` that lets us map source files to arbitrary relative include paths by creating a link tree, and adding that link tree to the deps of a main library. This also uses apple_library instead of apple_framework as the framework is frankly unnecessary for what we're trying to build for third party deps. Test Plan: Ported fmt locally to use this, and ran `clyde ios build` Tasks: https://app.asana.com/0/1203960884932166/1204190072048691 https://app.asana.com/0/1203960884932166/1204190072362093 [transitions] Do not remove cpus from configured target graph (#2) Upstream converts ios_multi_cpus to a single value. rules_pods does not. So when rules_pods objc_library rules depend on rules_ios rules, they are considered different configurations, and we end up splitting the configured target graph into two configurations. When swift, objc, etc try to link in all objc_libraries later in the build, they then try to link the same library twice, because the configured nodes are duplicated, and we run into duplicate symbol, module redefinition, etc errors. In theory, this could be reverted once the rules_ios migration is complete, but for our use case, it should not hurt anything being present. This logic, also, only affects our opt builds that build for multiple cpus. The development builds are unaffected. [rules_ios] Re-enable cpp rules (#3) This was accidentally disabled. It is required for glog, so turn this back on so that .cc/.cpp sources and their .a files are properly propagated. Tested by doing a build with this change off of main [rules_ios] Use -idirafter instead of -I (#4) NOTE: that this may cause issues for libraries that anticipate the `-I.` behavior and do something like `#include "Foo.h"` when there are multiple "Foo.h" in libs in its dependencies. In those cases, `-I.` can be added to its copts explicitly. There are some places where using -I can actually cause problems with system header collisions. Notably, fmt has a "locale.h" file. Without this change, you can end up with a chain that is: "fmt/locale.h" -> <locale> -> <clocale> -> <locale.h> -> failure, because -I causes fmt/locale.h to be searched (I believe due to the entries in the headermap being -I) before system paths. See https://gcc.gnu.org/onlinedocs/gcc/Directory-Options.html for resolution order [rules_ios] Allow rewriting header paths for frameworks and libraries (#5) This allows us to pick a different relative include path for header files. This is needed because some third party dependencies have e.g. colliding base filenames (folly has multiple Core.h, e.g.), or included files do relative includes (kv-storage includes "kv-storage/api/Core.h"), and with the current approach taken by apple_framework of flattening the headers out, that path doesn't actually exist in the Headers or PrivateHeaders directories of the .framework file. This patch does a few things: - Adds an extra argument "header_mapping". This is how users define where public/private headers should be accessible. If not specified, or a header is not in this mapping, then the basename of the header is used. (e.g. kv-storage.framework/Headers/Core.h) - Updates the hmapbuild binary so that we can specify these destinations in the .hmap files - Updates the umbrella header to use these destinations - Updates apple_framework/apple_library to thread these mappings through. Tested on a WIP branch (annie/native-versions-ios), and we seemed to get past the #include failures, and onto errors in their code itself. Will also test this with the flipper-folly migration locally. https://app.asana.com/0/1203960884932166/1204190072362095 https://app.asana.com/0/1203960884932166/1204190072362093 [rules_ios] Make modulemaps have '[system]', fix static_library deps (#6) - There was an ordering issue with apple_static_library where it didn't include the symlink map in the deps for the apple_library call. Make sure to route it through. - When changing over to cocoapods bazel, we modularize a lot of things, which causes a lot of extra warnings to show up from third party packages. We're not really in charge of maintaining their code, so just add the "[system]" tag to the modulemap file. This makes the headers get included as if they were included through -isystem, not -I [rules_ios] Export as system_includes (#7) For system / 3rd party libraries, export the symlink dir as -isystem, not -I. [rules_ios] Add objc_public_headers (#9) Add an `objc_public_headers` attribute to apple_* rules. This changes the umbrella header, if set, such that it includes all public_headers headers when compiled against c++, and only a subset of headers (objc_public_headers) when compiled against objective c. The main problem this is solving is that we have a main module map for each library. When trying to import *anything* from the framework, it loads the entire module map and every include in the umbrella header. If you put c++-only headers in it, it will try to interpret those without c++ support if you try to include the .h from a .m file. This allows us to have headers that mix (obj)c++ and objc in a single framework. It should also allow us to drop some of the -objc rules we've had to clunkily build out. [rules_ios] Expose cc_headers_symlinks, add a helper function for headers (#10) Expose cc_headers_symlinks so that we can use it in a refactor of React-Core and ReactCommon. Also expose a helper function for use by ReactCommon [rules_ios] Add more entitlement errors (#12) If there are keys that are in the .entitlements file that are missing from the provisioning profile, this normally results in an error. However, this is a manually specified list. These storekit ones bit us recently, so add the storekit ones, and then just pull in the list from main (https://github.com/bazelbuild/rules_apple/blob/cc1e5b17d0bf16a74c74f9fb3e2940d9c97c12da/tools/plisttool/plisttool.py#L295-L314) [rules_ios] Add more configurability to headers_mapping (#11) When this just takes a dictionary, it requires us to do a lot more boilerplate work to first construct that dictionary. Add a couple of helpers for the common use cases that are in repo. [rules_apple] add upstream filesystem patches (#13) this adds [this commit](bazelbuild/rules_apple@306e300) to address this error when copying results files across filesystem boundaries. [rules_apple] Add support for xcprivacy manifests in extensions (#14) Bringing a change in from upstream of rules_apple to support adding privacy manifests for our extensions bazelbuild/rules_apple@e9159fa
…nerate a swift module map
nataliejameson
force-pushed
the
nmj/upgrade-rules-ios-2
branch
from
dbd3acd to
80ccb86
Compare
nataliejameson
added a commit
that referenced
this pull request
Jan 6, 2025
Rebasing our changes onto 5.3.0. Minimal changes needed.
nataliejameson
added a commit
that referenced
this pull request
Jan 7, 2025
Does a few things: - Rebases to 5.3.0 - Updates the rules_apple version to get bazelbuild/rules_apple#2552 - Fix a bug in static_library that was exposed by bazel 8 update
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Re-applying #15
This does a few things:
At the end, the major changes we have vs upstream are:
The original commits were:
2e4ed21 (origin/discord, origin/HEAD, discord) [rules_apple] Add support for xcprivacy manifests in extensions (#14)
86b8d2d [rules_apple] add upstream filesystem patches (#13)
0add154 [rules_ios] Add more configurability to headers_mapping (#11)
a6c59d1 [rules_ios] Add more entitlement errors (#12)
2699bd3 [rules_ios] Expose cc_headers_symlinks, add a helper function for headers (#10)
c494ff2 [rules_ios] Add objc_public_headers (#9)
7d8db25 [rules_ios] Export as system_includes (#7)
28ed123 [rules_ios] Make modulemaps have '[system]', fix static_library deps (#6)
4340c91 [rules_ios] Allow rewriting header paths for frameworks and libraries (#5)
1fc624a [rules_ios] Use -idirafter instead of -I (#4)
01a939d [rules_ios] Re-enable cpp rules (#3)
238256a [transitions] Do not remove cpus from configured target graph (#2)
429e3f0 [rules_ios] Add static library variant to allow rewriting headers (#1)