Git Product home page Git Product logo

Comments (19)

pepicrft avatar pepicrft commented on July 23, 2024 1

For anyone coming across this issue in the future, I'd recommend checking out this PR, which will soon be available in the documentation.

The trade-off Tuist is presented with design-wise is convenient implicit configuration vs less convenient explicit configuration. We leaned on the latter for the reasons documented here that led to Xcode (and also SPM) to be challenging to use at scale, so we don't want to fall into the same trap.

from tuist.

pepicrft avatar pepicrft commented on July 23, 2024

@mihahoz would you mind sharing the logs that you get when you run tuist generate?

from tuist.

jgoodrick avatar jgoodrick commented on July 23, 2024

Just for some additional information, I've created a reproducible example of the behavior here: https://github.com/jgoodrick/repro-tuist-shared-identifiedArray

from tuist.

jgoodrick avatar jgoodrick commented on July 23, 2024

also, here are my generate logs. Everything looks good:

➜ shared-identified-array-bug on  main tuist generate
Loading and constructing the graph
It might take a while if the cache is empty
Using cache binaries for the following targets: 
Generating workspace SharedIdentifiedArrayBug.xcworkspace
Generating project SharedIdentifiedArrayBug
Generating project combine-schedulers
Generating project swift-collections
Generating project swift-composable-architecture
Generating project swift-custom-dump
Generating project swift-case-paths
Generating project swift-concurrency-extras
Generating project swift-clocks
Generating project swift-perception
Generating project swift-identified-collections
Generating project swift-dependencies
Generating project swift-syntax
Generating project swiftui-navigation
Generating project xctest-dynamic-overlay
Project generated.
Total time taken: 1.180s

from tuist.

mihahoz avatar mihahoz commented on July 23, 2024

@pepicrft sure, here is the output:

Screenshot 2024-05-29 at 08 49 57

from tuist.

Narsail avatar Narsail commented on July 23, 2024

@mihahoz I moved yesterday to the same setup (From a simple staticFramework integration). What works for me to solve the duplicate symbol issue is to set the external libraries to .framework:

#if TUIST
    import ProjectDescription

    let packageSettings = PackageSettings(
        productTypes: [
            "Alamofire": .framework,
            "DeviceKit": .framework,
            "SwiftCollections": .framework,
            "SwiftUIIntrospect": .framework,
            "ComposableArchitecture": .framework,
            "ComposableUserNotifications": .framework,
            "CustomKeyboardKit": .framework,
            "Dependencies": .framework,
            "OrderedCollections": .framework,
            "Perception": .framework,
            "CombineSchedulers": .framework,
            "XCTestDynamicOverlay": .framework,
            "ConcurrencyExtras": .framework
        ]
    )
#endif

Note: I also have '-ObjC' set for the LDOtherFlags and use two TCA Frameworks 'ComposableArchitecture' and 'ComposableUserNotifications' which share a set of dependencies as well.

That said, while this is solving the duplicate symbol issue, I now have crashes in the release builds which look like

failed to demangle witness for associated type 'Body' in conformance 'ExampleProject.WorkoutListView: View' from mangled name '�\361\343�' - subject type x does not conform to protocol Equatable

from tuist.

mihahoz avatar mihahoz commented on July 23, 2024

EDIT 06/30: @Narsail I overlooked this detail when reading; I didn't set all of the TCA subdependencies to .framework too, only the ComposableArchitecture. After setting all of the subdependencies as well, the runtime crash goes away for me. But I haven't tried the release build yet.


Yes, I tried this approach as well, but it eventually led to runtime crashes, which made me think it might be a project configuration issue.

While leaving the package settings on default values and changing the feature frameworks to static ones does work, I don't believe it is the right approach. I think using a dynamic framework that depends on a static framework, which is then shared between different feature frameworks, would be the correct approach.

from tuist.

Narsail avatar Narsail commented on July 23, 2024

I guess those are the runtime issues I'm currently experience. I agree with you, having one dynamic framework encapsulating the static ones to be repurposed into the feature frameworks should be the way. Thanks for pushing this forward!

from tuist.

pepicrft avatar pepicrft commented on July 23, 2024

I think the error that you receive is very explicit about what the issue is:

objc[1170]: Class _TtC22ComposableArchitecture22CancellablesCollection is implemented in both /private/var/containers/Bundle/Application/118AA03B-45DE-414B-A0DC-E9C3348D7CF6/Demo.app/Frameworks/FeatureB.framework/FeatureB (0x103955618) and /private/var/containers/Bundle/Application/118AA03B-45DE-414B-A0DC-E9C3348D7CF6/Demo.app/Frameworks/FeatureA.framework/FeatureA (0x1035d5618). One of the two will be used. Which one is undefined.

So FeatureB and FeatureA both contain symbols from composable architecture. While this can sometimes work, with the big caveat that you are unnecessarily increasing the size of the app, in this case, it's causing your app to crash. What you'll have to do is:

  • Make everything either static or dynamic, so everything is linked at the Demo level.
  • Make the transitive and shared dependencies dynamic (FeatureCommon and ComposableArchitecture), and the others static (FeatureA, and FeatureB)

Could you try and confirm if the issue goes away?
I'd have expected Tuist to output a warning in this case about "side effects" of static products. Do you have them disabled by mistake?

from tuist.

Narsail avatar Narsail commented on July 23, 2024

I think the error that you receive is very explicit about what the issue is:

objc[1170]: Class _TtC22ComposableArchitecture22CancellablesCollection is implemented in both /private/var/containers/Bundle/Application/118AA03B-45DE-414B-A0DC-E9C3348D7CF6/Demo.app/Frameworks/FeatureB.framework/FeatureB (0x103955618) and /private/var/containers/Bundle/Application/118AA03B-45DE-414B-A0DC-E9C3348D7CF6/Demo.app/Frameworks/FeatureA.framework/FeatureA (0x1035d5618). One of the two will be used. Which one is undefined.

So FeatureB and FeatureA both contain symbols from composable architecture. While this can sometimes work, with the big caveat that you are unnecessarily increasing the size of the app, in this case, it's causing your app to crash. What you'll have to do is:

  • Make everything either static or dynamic, so everything is linked at the Demo level.
  • Make the transitive and shared dependencies dynamic (FeatureCommon and ComposableArchitecture), and the others static (FeatureA, and FeatureB)

Could you try and confirm if the issue goes away? I'd have expected Tuist to output a warning in this case about "side effects" of static products. Do you have them disabled by mistake?

FYI the first variant 'Make everything either static or dynamic, so everything is linked at the Demo level' is something I have tried (made everything dynamic) which resulted in release version runtime issues:

failed to demangle witness for associated type 'Body' in conformance 'ExampleProject.WorkoutListView: View' from mangled name '�\361\343�' - subject type x does not conform to protocol Equatable

That doesn't seem to be a straight forward solution.

from tuist.

pepicrft avatar pepicrft commented on July 23, 2024

https://drive.google.com/file/d/1c462mTAMcFoXfS0e0vHdwK08Hzf7bIIs/view?usp=sharing

I just tried the recommended way forward with this project making everything static and it works.

Just for some additional information, I've created a reproducible example of the behavior here: https://github.com/jgoodrick/repro-tuist-shared-identifiedArray

@jgoodrick the project works by turning everything into dynamic frameworks:

#if TUIST
    import ProjectDescription

    let packageSettings = PackageSettings(
        productTypes: [
            "ComposableArchitecture": .framework,
            "Dependencies": .framework,
            "Clocks": .framework,
            "ConcurrencyExtras": .framework,
            "CombineSchedulers": .framework,
            "IdentifiedCollections": .framework,
            "OrderedCollections": .framework,
            "_CollectionsUtilities": .framework,
            "DependenciesMacros": .framework,
            "SwiftUINavigationCore": .framework,
            "Perception": .framework,
            "CasePaths": .framework,
            "CustomDump": .framework,
            "XCTestDynamicOverlay": .framework
        ]
    )
#endif

Why it is required to be dynamic is more of a question for the Pointfree team. I noticed that using the default SPM integration, SPM integrates them dynamically, too, so things fall apart when the integration is done statically, which is Tuist's default.
Although it's a bit annoying as a user having to think and configure the linking, I think it's better than letting a tool do that automatically, because it's a decision that can have important consequences in your app, for example, if composable architecture targets are dynamic, they might end up impacting the launch time of your app.

FYI the first variant 'Make everything either static or dynamic, so everything is linked at the Demo level' is something I have tried (made everything dynamic) which resulted in release version runtime issues:

@Narsail could you include a project where I can reproduce the issue?

from tuist.

Narsail avatar Narsail commented on July 23, 2024

@pepicrft Let me check whether I can build an Example Project that has that problem.

In the meantime, have you ran the working project where you made everything dynamic in release mode as well? That's where I stumbled upon problems while the debug version was fine.

from tuist.

pepicrft avatar pepicrft commented on July 23, 2024

In the meantime, have you ran the working project where you made everything dynamic in release mode as well? That's where I stumbled upon problems while the debug version was fine.

Yes. I compiled the app for release and ran it on an Apple Silicon simulator and it works without any runtime crashes.

from tuist.

Narsail avatar Narsail commented on July 23, 2024

@pepicrft So far no success, I can't reproduce my issues with a minimal version where I linked everything dynamically. I assume there is a different problem somewhere in my Shared code that has nothing to do with the dependencies.

from tuist.

pepicrft avatar pepicrft commented on July 23, 2024

If you could reproduce that in a small project that you can share I'd be happy to take a look.

from tuist.

mbrandonw avatar mbrandonw commented on July 23, 2024

Why it is required to be dynamic is more of a question for the Pointfree team. I noticed that using the default SPM integration, SPM integrates them dynamically, too, so things fall apart when the integration is done statically, which is Tuist's default.

I'm not sure I understand the question. We are not doing anything special in our SPM packages. They are as vanilla as they come. It seems like this would be an issue in any SPM package, not just ours, and so I can't imagine there is anything we could do to fix this.

from tuist.

jgoodrick avatar jgoodrick commented on July 23, 2024

I kind of agree with @mbrandonw, in that I wonder why tuist doesn't default to the same behavior as SPM when using the Package.swift?

from tuist.

mihahoz avatar mihahoz commented on July 23, 2024

@pepicrft,

Thank you so much for your help and responsiveness. Your insights have been really helpful 🙏. I can confirm that marking all of the subdependencies of TCA as dynamic frameworks resolves the runtime crash issue.

Initially, I had everything set to dynamic, including the Composable Architecture framework (without subdependencies), but this resulted in runtime crashes (symbols weren't duplicated tho). For users integrating frameworks via SPM, this approach might lead to a challenging experience because each package would require checking all subdependencies and adjusting them as needed. This process can be somewhat tedious and error-prone.

Speaking of potential issues, the runtime crash pushed me to try the version where TCA is set to default (Static). Please correct me if I am wrong, but FeatureA and FeatureB should not have duplicated symbols because both depend on the dynamic CommonFeature, which in turn depends on a static framework. However, neither FeatureA nor FeatureB embed CommonFeature. All of this is done at the demo level, so CommonFeature is included only once; hence, there should be no duplicated symbols.

Thanks again for your guidance and support.

from tuist.

pepicrft avatar pepicrft commented on July 23, 2024

The issue is summarized very well in this discussion and we are taking steps to document it: pointfreeco/swift-composable-architecture#1657 (comment)

The linker might perform optimizations when targets are linked statically that can cause things to blow up at runtime. It's not specific to Composable Architecture.

SPM decides to link the packages dynamically, at least in the project that I created, perhaps because they detect that doing it statically might cause errors. This is bad design because you can't reliably know when the flag is required by analyzing the code statically. But opting into making something dynamic is a decision with consequences in your project, like increasing the boot time, hence why we require users to do a bit of a more involved work and think about their graph and the static/dynamic linking.

Dynamic configuration resolved at build time, like whether something is static or dynamic, while convenient at small scale, can make some features that rely on the build system, like Swift previews to work internally. Hence why we continue to recommend.

We acknowledge that this puts the user in a position to learn some build concepts, to design their dependency graph, and therefore we are going to work on better documentation that translates somewhat complex concepts into something more manageable.

Speaking of potential issues, the runtime crash pushed me to try the version where TCA is set to default (Static). Please correct me if I am wrong, but FeatureA and FeatureB should not have duplicated symbols because both depend on the dynamic CommonFeature, which in turn depends on a static framework. However, neither FeatureA nor FeatureB embed CommonFeature. All of this is done at the demo level, so CommonFeature is included only once; hence, there should be no duplicated symbols.

This is correct :). We link things at the right level so that you don't get those duplicated symbols

from tuist.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.