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

Accio attempts to build the same dependency multiple times #51

Closed
acecilia opened this issue May 20, 2019 · 2 comments · Fixed by #53
Closed

Accio attempts to build the same dependency multiple times #51

acecilia opened this issue May 20, 2019 · 2 comments · Fixed by #53

Comments

@acecilia
Copy link
Contributor

Hi,

When I set RxCocoa as a dependency (version 5.0.1) I see that Accio attempts to build RxSwift twice. From the logs:

✨  Found cached build products for RxSwift in local cache - skipping build.
✨  Found cached build products for RxRelay in local cache - skipping build.
✨  Found cached build products for RxSwift in local cache - skipping build.
✨  Found cached build products for RxCocoa in local cache - skipping build.

Seems like this is because RxCocoa depends on RxSwift and RxRelay, and RxRelay depends on RxSwift. Is this intended behaviour? Shouldn't Accio, after dependency resolution, only attempt to build the necessary dependencies once?

Thanks,
Andres

@fredpi
Copy link
Contributor

fredpi commented May 21, 2019

Thanks for reporting this 👍 I was able to reconstruct this, and actually, Accio doesn't detect if a framework is required by multiple frameworks. This can probably be easily fixed by using a Set here:

for framework in try appTarget.frameworkDependencies(manifest: manifest, dependencyGraph: dependencyGraph).flattenedDeepFirstOrder() {

@Jeehut
Copy link
Contributor

Jeehut commented Jun 11, 2019

It's correct that this is the case and I never thought this would be a problem since there's always at least a local caching in place which will skip the rendering. I just didn't go the extra mile since I didn't see much benefit in it, but you guys seem to do, so let me review #53 and merge it. Thanks, @fredpi!

Jeehut added a commit that referenced this issue Jun 19, 2019
Remove duplicated processing of frameworks
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 a pull request may close this issue.

3 participants