-
Notifications
You must be signed in to change notification settings - Fork 168
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.
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
Cocoapods subspec consumption: Introduce script to filter out unused resources. #492
Cocoapods subspec consumption: Introduce script to filter out unused resources. #492
Conversation
…he xcodebuild command.
try FileManager.default.removeItem(at: directoryURL) | ||
} | ||
|
||
print(" - \(directoryEntry) (\(shouldBeKept ? "kept" : "removed"))") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
while this is great should we print all the time tho?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it'll be easier to correlate logs of our cloud builds to understand which assets were included in the bundle.
What is your main concern with printing all the time? Log size? Noise?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
log size and noise. can we put #ifdef?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Showing All Messages ONLY_ACTIVE_ARCH=YES requested with multiple ARCHS and no run destination to provide an active architecture; building for all applicable architectures (in target 'MicrosoftFluentUI-FluentUIResources-ios' from project 'Pods')
…nd the script during linting. Enclosing variables in double quotes to prevent parameters from being interpreted incorrectly when they have spaces.
…resources. (#492) * First version of the resource cleanup script based on .resources.xcfilelist files. * Fixing script phase commands and preparing for another test (tag 0.0.13_Test). * Fixing log messages and setting additional environment variables in the xcodebuild command. * Bumping version to 0.0.14 * Reverting podspec version and linting script file. * Fixing indentation of string extension. * Adding flags to set verbose level based on Debug/Release configurations. * Setting the ARCHS variable to prevent the build warning in debug mode: Showing All Messages ONLY_ACTIVE_ARCH=YES requested with multiple ARCHS and no run destination to provide an active architecture; building for all applicable architectures (in target 'MicrosoftFluentUI-FluentUIResources-ios' from project 'Pods') * Fixing pod lib lint issues by using $PODS_TARGET_SRCROOT so it can find the script during linting. Enclosing variables in double quotes to prevent parameters from being interpreted incorrectly when they have spaces.
Platforms Impacted
Context / Problem statement
Currently the Coccoapods consumption story allows client apps to consume individual subspecs which are granular building blocks of the entire FluentUI library. It's a great approach to save on the binary size footprint of FluentUI in the client app by importing only the controls needed instead of the whole library.
While this works with the compiled code, resources are now being consumed as a single monolithic block. This means that if the client app wants to import only the Label subspec (for example), it will get all the resources for all the controls/subspecs (which won't be used) as part of the resource bundle.
The Core_ios subspec defines the resource bundle including the FluentUI-ios.xcassets as well as the shared (with FluentUI Mac) xcassets as part of its resource bundle:
The initially planned approach was based on an assumption that Cococapods would merge all the contents defined with the same resource bundle name ("FluentUIResources-ios") in the resource_bundle properties of the subspecs.
Unfortunately that is not currently supported by Cocoapods. What ends up happening in this case is that the most basic spec in the dependency hierarchy wins and only its defined resources are part of the bundle.
Some issues created in the Cocoapods GitHub repo illustrate the problem:
Solution
Given the context above, the approach taken to solve the problem is to inject a script on the Core_ios subspec (the basic subspec) via script_phases so that we can remove the assets that are not currently being used.
The script will delete assets based on the .resources.xcfilelist files which will contain a list of all resources used by that subspec. The .resource.xcfilelist will only be included in the project folder if the subspec is being consumed by the client app.
Because Cocoapods does not have support for a build script phase in the resource bundle target (so the assets could be removed before the target builds), we need to rebuild that specific target as part of the FluentUI framework/static lib target (which takes dependency on the resource bundle target).
Verification
Published the contents of this PR as a private Cococapods repo (https://github.com/rdeassis/cocoapods-repo) and tested importing the pod from a client test app:
1. Using FluentUI as a framework:
2. Using FluentUI as a static library:
- Imported the entire FluentUI pod
- Imported only the CommandBar_ios, Drawer_ios and Popupmenu_ios subspecs
The table below shows the size of a resource bundle of an app targeted to iOS 14 consuming different variations of the FluentUI subspecs:
Build log sample for reference:
Pull request checklist
This PR has considered:
Microsoft Reviewers: Open in CodeFlow