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

Manage Transitive Dependencies #60

Open
ollieatkinson opened this Issue Jul 20, 2018 · 13 comments

Comments

Projects
2 participants
@ollieatkinson

ollieatkinson commented Jul 20, 2018

I want to be able to include all transitive dependencies inside an executable bundle.

When building an app and breaking it up by features, I want to organise them by creating static libraries. If I link one of those static libraries in my App then I also need to link every single transitive dependency.

Example:
diagram_1

In the example above, when building App I have to ensure FeatureA, FeatureB and FeatureC are all included in the "Link Binary With Libraries" build phase. Otherwise App will fail to compile due to unresolved symbols.

App
  Linked Libraries:
      - FeatureA
      - FeatureB
      - FeatureC

It can get a little more complicated too, if we start to include resource bundles.

diagram_2

In this example, App must also include the the resource bundles which are dependencies for FeatureB and FeatureC. These will be included in an Copy Files Phase.

App
  Linked Libraries:
      - FeatureA
      - FeatureB
      - FeatureC
  Copy Files Phase:
      - FeatureBResources
      - FeatureCResources

We should have the ability to automate this, otherwise the overhead in maintenance of keeping these up-to date will be costly.

Consider a more contrived example when we introduce a test bundle for FeatureA

diagram_3

Here we now have to include all the transitive dependencies for FeatureA into the FeatureATest executable bundle.

FeatureATest
  Linked Libraries:
      - FeatureA
      - FeatureC
  Copy Files Phase:
      - FeatureCResources

Now scale this with more projects and more features and you then have quite a difficult task to manage them all.

Describe alternatives you've considered

This could be written externally from the project inside a different tool.
This could not be done at all, and transitive dependencies would have to manually be managed.

Additional context

Static Libraries do cause a bit of an issue when managing these transitive dependencies and trying to build them in the correct order. We need some way to force this, I experimented a few ways and have found the below method to be the least-intrusive method.

https://github.com/facebook/buck/blob/724d0741f130ede31671ecdbb65add1d2b6e02ac/src/com/facebook/buck/apple/project_generator/ProjectGenerator.java#L1117-L1127

If the current target, which is non-shared (e.g., static lib), depends on other focused targets which include Swift code, we must ensure those are treated as dependencies so that Xcode builds the targets in the correct order. Unfortunately, those deps can be part of other projects which would require cross-project references.

Thankfully, there's an easy workaround because we can just create a phony copy phase which depends on the outputs of the deps (i.e., the static libs). The copy phase will effectively say "Copy libX.a from Products Dir into Products Dir" which is a nop. To be on the safe side, we're explicitly marking the copy phase as only running for deployment postprocessing (i.e., "Copy only when installing") and disabling deployment postprocessing (it's enabled by default for release builds).

By investigating into the above, these are the changes which get made to a modules xcodeproj when creating a copy files phase and adding a dependency to another module build product.

-- File Reference -- 

FE49F8B22101BD0200DF2D9E /* libFeatureA.a */ = {isa = PBXFileReference; explicitFileType = archive.ar; path = libFeatureA.a; sourceTree = BUILT_PRODUCTS_DIR; };

-- Build File --

FE49F8B32101BD0700DF2D9E /* libFeatureA.a in [MyProject] Dependencies */ = {isa = PBXBuildFile; fileRef = FE49F8B22101BD0200DF2D9E /* libFeatureA.a */; };

-- Group --

FE49F8B22101BD0200DF2D9E /* libFeatureA.a */,

-- Copy Phase --

FE49F8B02101BCE300DF2D9E /* [MyProject] Dependencies */ = {
    isa = PBXCopyFilesBuildPhase;
    buildActionMask = 8;
    dstPath = "";
    dstSubfolderSpec = 16;
    files = (
        FE49F8B32101BD0700DF2D9E /* libFeatureA.a in [MyProject] Dependencies */,
    );
    name = "[MyProject] Dependencies";
    runOnlyForDeploymentPostprocessing = 1;
};

@ollieatkinson ollieatkinson changed the title from Transitive dependencies when using static libraries to Transitive Dependencies Jul 20, 2018

@ollieatkinson ollieatkinson changed the title from Transitive Dependencies to Manage Transitive Dependencies Jul 20, 2018

@pepibumur

This comment has been minimized.

Contributor

pepibumur commented Jul 21, 2018

Hey @ollieatkinson. This is a brilliant idea. I've always hoped Apple to provide such thing built in Xcode but that has never happened. In modular projects like yours, keeping all the settings right is a lot of manual work and error prone.

The current implementation of xpm analyzes the transitive dependencies but only for dynamic frameworks. It sets up the linking and copy frameworks phases. I haven't had time to implement something like that for static graphs, but it's definitively in the scope of xpm. I can't prioritize this now but if you feel like you'd like to work on it by yourself I'm happy to do some pairing or chat about how to the implementation would look.

@ollieatkinson

This comment has been minimized.

ollieatkinson commented Jul 21, 2018

I can take a look, do you have an idea of how you would want this implemented?

@pepibumur pepibumur referenced this issue Jul 21, 2018

Merged

Remove build phases models #67

4 of 4 tasks complete
@pepibumur

This comment has been minimized.

Contributor

pepibumur commented Aug 8, 2018

You can have a look at this class. For every target, use this class to generate the linking settings (build phases and build settings). That generator receives a Graph object that contains the representation of the dependency tree.

@ollieatkinson

This comment has been minimized.

ollieatkinson commented Aug 8, 2018

So we have a couple options for bundled resources, we can either include all transitive resources inside the main bundle, or package them up into their own bundles and be required to load them separately - do you have a preference?

FWIW buckbuild includes everything in the application/test bundles

@pepibumur

This comment has been minimized.

Contributor

pepibumur commented Aug 8, 2018

I've been thinking about the two approaches and to be honest, I lean more towards bundling them. CocoaPods also adds them to the application/tests target, but I personally think resources that belong to different modules shouldn't be merged.

My only concern is whether developers will know how to access the bundle to get the resources from it. If it's a framework or an app, you can get the bundle passing a class, or getting the main bundle, however, if the bundle is copied over to the built product, developers might not know where to find it.

What do you think about adding a helper class to get the URL of the bundle from the static library?

@ollieatkinson

This comment has been minimized.

ollieatkinson commented Aug 9, 2018

Yes we could generate some code to surface the URL and the bundle for loading the resources:

extension Bundle {
    static let myLibraryResourceBundleURL: URL = Bundle.main.url(forResource: "MyLibraryResource", withExtension: "bundle")!
    static let myLibraryResourceBundle: Bundle = Bundle(url: Bundle.myLibraryResourceBundleURL)!
}

The usage would be something like:

let nib = UINib(nibName: "MyLibraryCell", bundle: .myLibraryResourceBundle)

You're right for having the concern, it wouldn't be obvious to the end user about where to locate their resources - the documentation would have to be crystal clear & obvious.

If we did go with that option we could create a Generated/ reference folder in the $PROJECT_DIR for each of the modules and link it in when generating the project. We could build this feature in as an option and we can decide if we would like it on by default.

I am still in two minds, I will need to think on it.

@pepibumur

This comment has been minimized.

Contributor

pepibumur commented Aug 9, 2018

Exactly, we could have a .tuist/generated folder with all those files that we could reference from the libraries.

Another thought that I had when I was thinking about supporting transitive dependencies is that we could transparently setup the dependencies to be static libraries or dynamic frameworks.

Let's say you are building the app for release, in which case you'd like your app to boot fast. In that case, it'd make more sense to make static the deps that can be made static (because all its transitive dependencies are static). If you are working on the app, we'd like the compilation to be fast. In that case, going with dynamic frameworks would make more sense.

By providing those helpers, we'd abstract developers from whether the project is a library or a framework.

Out of curiosity, what are in your opinion strong arguments to lean towards using the main bundle? I'm also not 100% convinced.

@ollieatkinson

This comment has been minimized.

ollieatkinson commented Aug 9, 2018

Out of curiosity, what are in your opinion strong arguments to lean towards using the main bundle

  1. Easy to locate resources - developers just reference the main bundle.
  2. There's no default template in Xcode for iOS resource bundles, so they are largely not used by developers.
  3. No extra generated code required.

Personally, I am leaning towards the separate bundles. I just need to try envision how it's all going to piece together with 3rd party tools like cocoapods, who may have their own mechanism of managing these.

@pepibumur

This comment has been minimized.

Contributor

pepibumur commented Aug 9, 2018

Good points. To be honest, I haven't used bundles a lot. The projects I worked with were set up with frameworks so the resources were copied into their bundles.

I kind of like to see resources as source code. A class that is part of a feature x, which is represented as a library, should be in that library. If an image belongs to the same feature, it should be in a place where all the resources for that feature are.

how it's all going to piece together with 3rd party tools like cocoapods, who may have their own mechanism of managing these.

😅 yeah, this is another thing to be discussed. Supporting dependency managers and integrate them into the project that tuist generates.

I'd rather focus on envisioning this and maybe start thinking about supporting 3rd party tools on a different issue. Carthage, swiftlint, swiftformat, are tools that developer will most likely ask support for.

Feel free to drive this forward and take any necessary steps to make this possible. Happy to help with anything!

@pepibumur

This comment has been minimized.

Contributor

pepibumur commented Sep 11, 2018

@ollieatkinson

This comment has been minimized.

ollieatkinson commented Sep 20, 2018

I have come across a problem with the "fake" dependency management using the copy files phase.

This works inside of the Xcode IDE but when using xcodebuild from the command line it does not correctly resolve those dependencies resulting in unresolved symbols error as the static library has not been complied and swiftmodule has not placed inside of the built products directory.

So far I have only tested this using the old build system, I will have to investigate into the new build system or work out why the IDE operates differently.

@pepibumur pepibumur added this to To do in Issues Nov 22, 2018

@ollieatkinson

This comment has been minimized.

ollieatkinson commented Dec 6, 2018

I've added this code to Graph to traverse it DFS and grab all the transitive static library dependencies.

    func linkableDependencies(path: AbsolutePath, name: String) throws -> [DependencyReference] {
        guard let targetNode = self.targetNode(path: path, name: name) else { return [] }

        var references: [DependencyReference] = []

        /// Precompiled libraries and frameworks
        references.append(contentsOf: targetNode
            .dependencies
            .compactMap({ $0 as? PrecompiledNode })
            .map({ DependencyReference.absolute($0.path) }))
        
        switch targetNode.target.product {
        case .staticLibrary:
            // do not link anything if the current target is a static library, we want to link it to the top level dynamic library
            break
        case .app, .unitTests, .uiTests:
            
            // Find all static libraries and add them to the references.

            var stack = Stack<TargetNode>()
            
            for node in targetNode.dependencies where node is TargetNode {
                stack.push(node as! TargetNode)
            }
            
            var visited: Set<GraphNode> = .init()
            var staticLibraries: [TargetNode] = [ ]
            
            while !stack.isEmpty {
                
                guard let node = stack.pop() else {
                    continue
                }
                
                if visited.contains(node) {
                    continue
                }
                
                visited.insert(node)
                
                if node.target.product == .staticLibrary {
                    staticLibraries.append(node)
                }
                
                for child in node.dependencies where !visited.contains(child) && child is TargetNode {
                    stack.push(child as! TargetNode)
                }
                
            }
            
            references.append(contentsOf: staticLibraries.map{
                DependencyReference.product($0.target.productName)
            })
            
            break
        case _:
            break
        }
        
        // Link dynamic libraries and frameworks
        references.append(contentsOf: targetNode
            .dependencies
            .compactMap{ $0 as? TargetNode }
            .filter{ $0.target.product == .framework || $0.target.product == .dynamicLibrary }
            .map{ targetNode in
                return DependencyReference.product(targetNode.target.productName)
            })
        
        return references
    }

This works for the use case described above, can you think of (or have any) other use cases I should also be checking here.

@ollieatkinson

This comment has been minimized.

ollieatkinson commented Dec 6, 2018

@pepibumur I created a pull request to start that conversation, I hope it's along the lines of what you were expecting.

Do we have any integration tests for me to validate these changes against?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment