This repository has been archived by the owner on Nov 10, 2023. It is now read-only.
handle bundling resources of static frameworks (Originally by rmaz) #2490
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
add a test for bundling resources of static deps make formatter happy
v-jizhang
reviewed
Jul 1, 2020
Comment on lines
933
to
944
| TargetNodes.castArg(node, PrebuiltAppleFrameworkDescriptionArg.class) | ||
| .ifPresent( | ||
| prebuiltFramework -> { | ||
| // Technically (see Apple Tech Notes 2435), static frameworks are lies. In case | ||
| // a static framework is used, they can escape the incorrect project generation | ||
| // by marking its preferred linkage static (what does preferred linkage even | ||
| // mean for a prebuilt thing? none of this makes sense anyways). | ||
| if (prebuiltFramework.getConstructorArg().getPreferredLinkage() | ||
| != NativeLinkableGroup.Linkage.STATIC) { | ||
| filteredRules.add(node); | ||
| } | ||
| }); |
There was a problem hiding this comment.
According to tech notes https://developer.apple.com/library/archive/technotes/tn2435/_index.html, "apps cannot embed it (static framework) in the app bundle". we already excluded static framework here, why do you want to remove the code?
There was a problem hiding this comment.
I may not be 100% correct in my explanation since the code was originally from @rmaz, but from what I gather we cannot filter the rule here because we need to propagate the dependencies during the project generation
The static frameworks are now only being ignored during the copy files build phase. (Line 3427)
There was a problem hiding this comment.
How about non-static, NativeLinkableGroup.Linkage.SHARED for example? should it be added to filteredRules?
I have tested with and without this change, the test passes in both cases. That means either this change is not needed, or the test does not cover the change.
There was a problem hiding this comment.
I think the code here was made redundant by the other changes, but we can revert this part if you think that's better. All my manual tests work in both cases.
There was a problem hiding this comment.
Reverted, but it looks like a CI test failed for unrelated reasons, can you force a rerun?
There was a problem hiding this comment.
That's unrelated. I'll fix it later.
v-jizhang
reviewed
Jul 1, 2020
src/com/facebook/buck/features/apple/project/WorkspaceAndProjectGenerator.java
Outdated
Show resolved
Hide resolved
Lcsmarcal
added a commit
to ifood/buck
that referenced
this pull request
Oct 20, 2020
* Respect user input if abi_generation_mode is explicitly provided in java_library (facebook#2467) * Automate publishing docs on CirclrCI (facebook#2486) * Automate publishing docs on CirclrCI * Automate publishing docs on CircleCI * Automate publishing docs on CircleCI * Automate publishing docs using linux VM * Fixed Python3 compatible issues * Added code to install android_sdk * Automate docs, installed JDK for java docs * Automate publishing docs on CircleCI, added CNAME * Automate publishing docs deleted an extra line * Automate publishing docs, refactored code * Automate publishing docs, changed a required test name Co-authored-by: buck-bot <66389669+buck-bot@users.noreply.github.com> * Fixed git push access denied issue (facebook#2487) Co-authored-by: buck-bot <66389669+buck-bot@users.noreply.github.com> * handle bundling resources of static frameworks (Originally by rmaz) (facebook#2490) * handle bundling resources of static framworks add a test for bundling resources of static deps make formatter happy * Indentation fix * Reverted some filtering changes Co-authored-by: Richard Howell <richardh@uber.com> Co-authored-by: Gabe <gabriel.beltrame@luizalabs.com> * Add support for java 11 builds in jitpack (facebook#2499) * Fixed Unable to resolve dependency jdk8 (= 8.0.172) (facebook#2492) Co-authored-by: buck-bot <66389669+buck-bot@users.noreply.github.com> * CircleCI fix (facebook#2495) Co-authored-by: buck-bot <66389669+buck-bot@users.noreply.github.com> * Added python3 to interpreter names (facebook#2497) * Added python3 to interpreter names * Fixed an infinite recursive call * Test fixes * test fixes * Pull out unrelated changes Co-authored-by: buck-bot <66389669+buck-bot@users.noreply.github.com> * Fixes for rust tests (facebook#2503) Co-authored-by: v-jizhang <66389669+buck-bot@users.noreply.github.com> * Add support for ANDROID_SDK_ROOT environment variable (facebook#2507) Co-authored-by: v-jizhang <66389669+buck-bot@users.noreply.github.com> * Fix for facebook#2498 (facebook#2504) * Fix for facebook#2498 Buck fails to build non predex apps which include jars containing module-info.class files` facebook#2498 * Update DalvikStatsCache.java Typo fix * Update DalvikStatsCache.java Added comment for special case * Update DalvikStatsCache.java Added new New line and space before comments. v-jizhang 12 hours ago Contributor Could you move your comment 1 line down and add 1 space after "//"? Thanks @surapuramakhil * extracted filename from entry relative path, updated check with equals intead of ends with * updated comment * Set max store size bytes correctly so it is used by AbstractAsynchronousCache to decide what to upload (facebook#2510) * CI rust test fixes (facebook#2511) Co-authored-by: v-jizhang <66389669+buck-bot@users.noreply.github.com> * Fixed a phthon installation bug on CircleCI (facebook#2509) Co-authored-by: v-jizhang <66389669+buck-bot@users.noreply.github.com> * add Easynvest to buck showcases (facebook#2514) Co-authored-by: Lucas Marçal <lucas.fernandes@easynvest.com.br> * Make Python3 the first choice of python interpreter (facebook#2512) * Make Python3 the first choice of python interpreter * Fixed tests * Fixed tests * Fixed tests * Fixed testgs * Fixed tests * Fixed tests Co-authored-by: v-jizhang <66389669+buck-bot@users.noreply.github.com> * Fixed a python3 compatible issue (facebook#2517) Co-authored-by: v-jizhang <66389669+buck-bot@users.noreply.github.com> * Added application_region and application_language properties to xcode_workspace_config (facebook#2519) * Add region and language option to launch and test actions in xcode_workspace_config * Add application region and language arguments to projectV2 * Create unit test Co-authored-by: Lucas Marçal <lucas.fernandes@easynvest.com.br> * Added Android NDK tests (facebook#2515) * Added Android NDK 19 tests * Added ndk 20 tests * Added NDK 21 tests * Added timout for long running tests Co-authored-by: v-jizhang <66389669+buck-bot@users.noreply.github.com> * Saifulriza (facebook#2523) * Add logo * Update README.md * Update README.md * Add full kotlin configurations to kotlin rule (v2) (facebook#2526) * Add full kotlin configurations to kotlin rule * Adding test for KotlinConfiguredComilerFactory * Fixing pmd issue on setup method * Renaming remaining references to extra_kotlinc_arguments to free_compiler_args * Adding license info to new test file Co-authored-by: Zac Sweers <pandanomic@gmail.com> * Fixing paths that fail during project generation (facebook#2527) * (Rebase) Add snapshot_images_diff_path to apple_test (facebook#2528) * Add snapshot_images_diff_path to apple_test * Fix some issues found at code review Co-authored-by: Lucas Marçal <lucas.fernandes@easynvest.com.br> * Fixes for Windows tests (facebook#2529) * Fixes for Windows tests * Fixes for Windows tests * Fixes for Windows tests * Fixes for Windows tests * Fixes for Windows tests * Fixes for Windows tests * Fixes for Windows tests * Fixes for Windows tests * Fixes for Windows tests * Fixes for Windows tests * Fixes for Windows tests * Fixes for Windows tests * Added Flavors to docs (facebook#2522) * Added Flavors to docs * Flavor documentation, moddified per code review. Co-authored-by: v-jizhang <66389669+buck-bot@users.noreply.github.com> * Added a kotlin tutorial for android (facebook#2525) * Added a kotlin tutorial for android * Corrected errors in kotlin tutorial Co-authored-by: v-jizhang <66389669+buck-bot@users.noreply.github.com> * Prepare release v2020.09.01 (facebook#2532) Co-authored-by: v-jizhang <66389669+buck-bot@users.noreply.github.com> * Added dependency to pulbishing docs (facebook#2534) Co-authored-by: v-jizhang <66389669+buck-bot@users.noreply.github.com> * Fixed CircleCI tests (facebook#2531) Co-authored-by: v-jizhang <66389669+buck-bot@users.noreply.github.com> * Upgraded ruby syntax in replease process (facebook#2533) Co-authored-by: v-jizhang <66389669+buck-bot@users.noreply.github.com> * Skip flaky tests on Windows platform (facebook#2536) * Skip flaky tests on Windows platform * Skip flaky tests on Windows platform * Skip flaky tests on Windows platform * Skip flaky tests on Windows platform * Skip a flaky test on Windows * Increased a timeout in Winddows test. (facebook#2544) * Fixed dlang installation error (facebook#2543) * Fixed dlang installation error * Fixed dlang installation error Co-authored-by: v-jizhang <66389669+buck-bot@users.noreply.github.com> * Install python2 from archive. (facebook#2542) * Install python2 from archive. * Fixed an python3 installation error. Co-authored-by: v-jizhang <66389669+buck-bot@users.noreply.github.com> * Fixed a link (facebook#2539) Co-authored-by: v-jizhang <66389669+buck-bot@users.noreply.github.com> * Added a rust tutorial (facebook#2540) Co-authored-by: v-jizhang <66389669+buck-bot@users.noreply.github.com> * Add support for async worker processes (facebook#2547) * Allow WorkerProcess to be used for multiple jobs at the same time. No users yet, but next up is the process pool. * Introduce a new `async` option to worker tools. This makes buck call the tool concurrently, without waiting for responses. It's up to the tool to provide responses with the right ids, in any order. * Write some tests, fix some bugs * Add back license * Update WorkerProcessPool.java * Fix windows tests Don't crash all of buck when a worker process fails. * Code review feedback and clarifications, I hope Co-authored-by: David Z Hao <davidhao3300@yahoo.com> Co-authored-by: Jinlin Zhang <jinlinzhang@fb.com> Co-authored-by: buck-bot <66389669+buck-bot@users.noreply.github.com> Co-authored-by: Gabe <gabriel.unknoob@gmail.com> Co-authored-by: Richard Howell <richardh@uber.com> Co-authored-by: Gabe <gabriel.beltrame@luizalabs.com> Co-authored-by: Gautam Korlam <kageiit@users.noreply.github.com> Co-authored-by: Akhil <surapuramakhil@gmail.com> Co-authored-by: Ravi Agarwal <raviagarwal7@users.noreply.github.com> Co-authored-by: Lucas Marçal <lucas.fernandes@easynvest.com.br> Co-authored-by: Saiful Riza <45083824+Saifulriza123@users.noreply.github.com> Co-authored-by: Ty Smith <tys@uber.com> Co-authored-by: Zac Sweers <pandanomic@gmail.com> Co-authored-by: Mike Kaplinskiy <mike.kaplinskiy@gmail.com> Co-authored-by: Lucas Marçal <lucas.marcal@ifood.com.br>
Lcsmarcal
added a commit
to ifood/buck
that referenced
this pull request
Oct 20, 2020
* Respect user input if abi_generation_mode is explicitly provided in java_library (facebook#2467) * Automate publishing docs on CirclrCI (facebook#2486) * Automate publishing docs on CirclrCI * Automate publishing docs on CircleCI * Automate publishing docs on CircleCI * Automate publishing docs using linux VM * Fixed Python3 compatible issues * Added code to install android_sdk * Automate docs, installed JDK for java docs * Automate publishing docs on CircleCI, added CNAME * Automate publishing docs deleted an extra line * Automate publishing docs, refactored code * Automate publishing docs, changed a required test name Co-authored-by: buck-bot <66389669+buck-bot@users.noreply.github.com> * Fixed git push access denied issue (facebook#2487) Co-authored-by: buck-bot <66389669+buck-bot@users.noreply.github.com> * handle bundling resources of static frameworks (Originally by rmaz) (facebook#2490) * handle bundling resources of static framworks add a test for bundling resources of static deps make formatter happy * Indentation fix * Reverted some filtering changes Co-authored-by: Richard Howell <richardh@uber.com> Co-authored-by: Gabe <gabriel.beltrame@luizalabs.com> * Add support for java 11 builds in jitpack (facebook#2499) * Fixed Unable to resolve dependency jdk8 (= 8.0.172) (facebook#2492) Co-authored-by: buck-bot <66389669+buck-bot@users.noreply.github.com> * CircleCI fix (facebook#2495) Co-authored-by: buck-bot <66389669+buck-bot@users.noreply.github.com> * Added python3 to interpreter names (facebook#2497) * Added python3 to interpreter names * Fixed an infinite recursive call * Test fixes * test fixes * Pull out unrelated changes Co-authored-by: buck-bot <66389669+buck-bot@users.noreply.github.com> * Fixes for rust tests (facebook#2503) Co-authored-by: v-jizhang <66389669+buck-bot@users.noreply.github.com> * Add support for ANDROID_SDK_ROOT environment variable (facebook#2507) Co-authored-by: v-jizhang <66389669+buck-bot@users.noreply.github.com> * Fix for facebook#2498 (facebook#2504) * Fix for facebook#2498 Buck fails to build non predex apps which include jars containing module-info.class files` facebook#2498 * Update DalvikStatsCache.java Typo fix * Update DalvikStatsCache.java Added comment for special case * Update DalvikStatsCache.java Added new New line and space before comments. v-jizhang 12 hours ago Contributor Could you move your comment 1 line down and add 1 space after "//"? Thanks @surapuramakhil * extracted filename from entry relative path, updated check with equals intead of ends with * updated comment * Set max store size bytes correctly so it is used by AbstractAsynchronousCache to decide what to upload (facebook#2510) * CI rust test fixes (facebook#2511) Co-authored-by: v-jizhang <66389669+buck-bot@users.noreply.github.com> * Fixed a phthon installation bug on CircleCI (facebook#2509) Co-authored-by: v-jizhang <66389669+buck-bot@users.noreply.github.com> * add Easynvest to buck showcases (facebook#2514) Co-authored-by: Lucas Marçal <lucas.fernandes@easynvest.com.br> * Make Python3 the first choice of python interpreter (facebook#2512) * Make Python3 the first choice of python interpreter * Fixed tests * Fixed tests * Fixed tests * Fixed testgs * Fixed tests * Fixed tests Co-authored-by: v-jizhang <66389669+buck-bot@users.noreply.github.com> * Fixed a python3 compatible issue (facebook#2517) Co-authored-by: v-jizhang <66389669+buck-bot@users.noreply.github.com> * Added application_region and application_language properties to xcode_workspace_config (facebook#2519) * Add region and language option to launch and test actions in xcode_workspace_config * Add application region and language arguments to projectV2 * Create unit test Co-authored-by: Lucas Marçal <lucas.fernandes@easynvest.com.br> * Added Android NDK tests (facebook#2515) * Added Android NDK 19 tests * Added ndk 20 tests * Added NDK 21 tests * Added timout for long running tests Co-authored-by: v-jizhang <66389669+buck-bot@users.noreply.github.com> * Saifulriza (facebook#2523) * Add logo * Update README.md * Update README.md * Add full kotlin configurations to kotlin rule (v2) (facebook#2526) * Add full kotlin configurations to kotlin rule * Adding test for KotlinConfiguredComilerFactory * Fixing pmd issue on setup method * Renaming remaining references to extra_kotlinc_arguments to free_compiler_args * Adding license info to new test file Co-authored-by: Zac Sweers <pandanomic@gmail.com> * Fixing paths that fail during project generation (facebook#2527) * (Rebase) Add snapshot_images_diff_path to apple_test (facebook#2528) * Add snapshot_images_diff_path to apple_test * Fix some issues found at code review Co-authored-by: Lucas Marçal <lucas.fernandes@easynvest.com.br> * Fixes for Windows tests (facebook#2529) * Fixes for Windows tests * Fixes for Windows tests * Fixes for Windows tests * Fixes for Windows tests * Fixes for Windows tests * Fixes for Windows tests * Fixes for Windows tests * Fixes for Windows tests * Fixes for Windows tests * Fixes for Windows tests * Fixes for Windows tests * Fixes for Windows tests * Added Flavors to docs (facebook#2522) * Added Flavors to docs * Flavor documentation, moddified per code review. Co-authored-by: v-jizhang <66389669+buck-bot@users.noreply.github.com> * Added a kotlin tutorial for android (facebook#2525) * Added a kotlin tutorial for android * Corrected errors in kotlin tutorial Co-authored-by: v-jizhang <66389669+buck-bot@users.noreply.github.com> * Prepare release v2020.09.01 (facebook#2532) Co-authored-by: v-jizhang <66389669+buck-bot@users.noreply.github.com> * Added dependency to pulbishing docs (facebook#2534) Co-authored-by: v-jizhang <66389669+buck-bot@users.noreply.github.com> * Fixed CircleCI tests (facebook#2531) Co-authored-by: v-jizhang <66389669+buck-bot@users.noreply.github.com> * Upgraded ruby syntax in replease process (facebook#2533) Co-authored-by: v-jizhang <66389669+buck-bot@users.noreply.github.com> * Skip flaky tests on Windows platform (facebook#2536) * Skip flaky tests on Windows platform * Skip flaky tests on Windows platform * Skip flaky tests on Windows platform * Skip flaky tests on Windows platform * Skip a flaky test on Windows * Increased a timeout in Winddows test. (facebook#2544) * Fixed dlang installation error (facebook#2543) * Fixed dlang installation error * Fixed dlang installation error Co-authored-by: v-jizhang <66389669+buck-bot@users.noreply.github.com> * Install python2 from archive. (facebook#2542) * Install python2 from archive. * Fixed an python3 installation error. Co-authored-by: v-jizhang <66389669+buck-bot@users.noreply.github.com> * Fixed a link (facebook#2539) Co-authored-by: v-jizhang <66389669+buck-bot@users.noreply.github.com> * Added a rust tutorial (facebook#2540) Co-authored-by: v-jizhang <66389669+buck-bot@users.noreply.github.com> * Add app clip support * Add integration test * fix target name at Integration test * fix typo * Fix bundle ProductType * Add support for async worker processes (facebook#2547) * Allow WorkerProcess to be used for multiple jobs at the same time. No users yet, but next up is the process pool. * Introduce a new `async` option to worker tools. This makes buck call the tool concurrently, without waiting for responses. It's up to the tool to provide responses with the right ids, in any order. * Write some tests, fix some bugs * Add back license * Update WorkerProcessPool.java * Fix windows tests Don't crash all of buck when a worker process fails. * Code review feedback and clarifications, I hope * remove duplicated change Co-authored-by: David Z Hao <davidhao3300@yahoo.com> Co-authored-by: Jinlin Zhang <jinlinzhang@fb.com> Co-authored-by: buck-bot <66389669+buck-bot@users.noreply.github.com> Co-authored-by: Gabe <gabriel.unknoob@gmail.com> Co-authored-by: Richard Howell <richardh@uber.com> Co-authored-by: Gabe <gabriel.beltrame@luizalabs.com> Co-authored-by: Gautam Korlam <kageiit@users.noreply.github.com> Co-authored-by: Akhil <surapuramakhil@gmail.com> Co-authored-by: Ravi Agarwal <raviagarwal7@users.noreply.github.com> Co-authored-by: Lucas Marçal <lucas.fernandes@easynvest.com.br> Co-authored-by: Saiful Riza <45083824+Saifulriza123@users.noreply.github.com> Co-authored-by: Ty Smith <tys@uber.com> Co-authored-by: Zac Sweers <pandanomic@gmail.com> Co-authored-by: Lucas Marçal <lucas.marcal@ifood.com.br> Co-authored-by: Mike Kaplinskiy <mike.kaplinskiy@gmail.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
2 years ago @rmaz opened this PR but later couldn't update it anymore.
@v-jizhang, you asked for a rebase, can we get this reviewed now?