opened
neonichu
username_0
Fixes the issue that all packages available in the sandbox are passed to each invocation of `llbuild()`, even when they are not dependencies of the package being built right now.

The issue is relevant when build products are being linked, currently only the case for binaries. The generated linker commands will potentially contain not yet build libraries, leading to linker errors.
Limit dependencies passed to llbuild()
issue
created
ddunbar
username_1
Thanks, that is a real problem. We intend to move to a model where the entire dependency graph is built in a single invocation, but we should definitely fix this until that is complete...

Can you add a test case that reproduces the failure and the fix? This seems like something that is important to capture in our tests.
comment
created
neonichu
username_0
Will do, am not entirely sure how the testsuite works, yet. 

I think I would add a new test to `FunctionalBuildTests.swift` as well as a fixture. Is the numbering of fixtures just consecutive?
comment
created
mxcl
username_2
Thanks this is indeed an issue, I will review it a bit more carefully than other PRs today since we should be planning for the better fix.
comment
created
neonichu
username_0
Added a test case for this now, based on `23_buildexec_multdep`. 

While doing that I discovered a problem with my original fix, though — for local dependencies, the URLs out of `get` do not match the ones from a `Manifest` (one is the filesystem location of the package, the other one the sandbox location). I'm now comparing just the basenames (see 6bce065577ca000d90054c7f90bf20f613496df8), because I am assuming the package name needs to be unique for a single build. Is that correct?
comment
created
mxcl
username_2
Yes, we need a proper URL class (Foundation isn't there yet). What I've been doing for now is ensuring the URL is normalized with some other code in other places. We'll probably want to do the same.

Still it would be nicer I think to add `==` for Package than do the URL comparison.
comment
created
mxcl
username_2
We can compare for the two different types.
comment
created
mxcl
username_2
Also we need the recursive dependencies passed. I think. This recently changed slightly due to a fix in SwiftC. I'll have to check.
comment
created
neonichu
username_0
That makes sense, linking recursive dependencies will definitely be required for dynamic linking.
comment
created
mxcl
username_2
In fact, the auto-linker should take care of both situations for us.
comment
created
mxcl
username_2
I'll make the adaptions needed here. Thanks.
comment
created
mxcl
username_2
It disturbs me that our current tests didn't identify this issue. I will make a note and add some more tests after merging this.
comment
created
mxcl
username_2
https://github.com/apple/swift-package-manager/commit/215b7269f4b28440241b8e20dfc21b5b91b582b7
comment