Skip to content
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

Enable virtual file system overlay replacements #71

Merged
merged 6 commits into from
Feb 7, 2022

Conversation

polac24
Copy link
Collaborator

@polac24 polac24 commented Feb 4, 2022

Part II of the Virtual File System Support (#70 continuation)

Problem:

When dynamic libraries are built in parallel, Xcode provides a mapping for virtual file system (-vfsoverlay argument) because some dependency files may not be ready yet, e.g. prepared .framework and compilers should look into "temporary" locations. Because these temporary locations are reported as dependencies, their absolute paths might leak to a meta json.

Solution

This PR inserts an extra step in the producer and consumer flows:

  • Producer: the overlay (assumed to be placed in $(TARGET_TEMP_DIR)/all-product-headers.yaml) replacement is performed for all recognized dependencies, before applying any ENV mappings (e.g. to substitute $(SRCROOT)) - a change to XCPostbuild
  • Consumer: the overlay (assumed to be placed in $(TARGET_TEMP_DIR)/all-product-headers.yaml) replacement is performed for all recognized dependencies, after applying any ENV mappings (e.g. to substitute $(SRCROOT)) - a change to XCPrebuild

Fixes #59

let overlayRemapper = try OverlayDependenciesRemapper(
overlayReader: overlayReader
)
let pathRemapper = DependenciesRemapperComposite([overlayRemapper, envsRemapper])
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

DependenciesRemapperComposite has a logic to invert remappers when making the generic->local transformation

@@ -0,0 +1 @@
{"case-sensitive":"false","roots":[],"version":0}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The default overlay when no dynamic libraries/frameworks are built

@@ -179,8 +179,7 @@ def self.run_cocoapods_scenario(template_path)
dump_podfile(consumer_configuration, template_path)
puts('Building consumer ...')
Dir.chdir(E2E_COCOAPODS_SAMPLE_DIR) do
# TODO: Change DerivedData's path to emulate multi-machines setup. Blocked by #59
build_project({'derivedDataPath' => "#{DERIVED_DATA_PATH}"})
build_project({'derivedDataPath' => "#{DERIVED_DATA_PATH}_consumer"})
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This PR resolves that - previously some absolute paths in meta were causing cache miss

@polac24 polac24 marked this pull request as ready for review February 7, 2022 18:44
class OverlayDependenciesRemapper: DependenciesRemapper {
private var mappings: [OverlayMapping]

init(overlayReader: OverlayReader) throws {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I would try to avoid throwing exceptions and executing any logic during initialization

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep, I did that because we already had a protocol DependenciesRemapper and didn't want to introduce noise. Let me do that in a separate PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

See #72

@polac24 polac24 merged commit 1e86cac into spotify:master Feb 7, 2022
@polac24 polac24 deleted the 20220204-overlay-mapper branch February 7, 2022 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

json file contains direct paths
2 participants