swiftlang / swift-package-manager

The Package Manager for the Swift Programming Language
Apache License 2.0
9.76k stars 1.35k forks source link

[SR-5849] Consider allowing targets to depend on products in the same package #4951

Open swift-ci opened 7 years ago

swift-ci commented 7 years ago
Previous ID SR-5849
Radar rdar://problem/35241745
Original Reporter sjanuary (JIRA User)
Type Improvement

Attachment: Download

Environment Linux Ubuntu 16_04 swift-4.0-DEVELOPMENT-SNAPSHOT-2017-09-04-a-ubuntu16.04 (also seen on swift-4.0-DEVELOPMENT-SNAPSHOT-2017-08-15-a-ubuntu16.04)
Additional Detail from JIRA | | | |------------------|-----------------| |Votes | 1 | |Component/s | Package Manager | |Labels | Improvement | |Assignee | None | |Priority | Medium | md5: 3f6363e4ba44fccb9bf4260288ab0b80

Issue Description:

I'm working on two projects, one of which, SwiftMetrics depends on the other, omr-agentcore, which exports a number of dynamic libraries. Within omr-agentcore one library, 'hcapiplugin' has another library, 'agentcore' as a dependency. On building the SwiftMetrics package on Linux it seems like libhcapiplugin.so has duplicated all the object code from its dependency libagentcore.so. The file is much bigger than it should be and running nm shows a lot of symbols that should belong to agentcore. The outcome here is double free errors when running SwiftMetrics tests, which are also indicative of shared routines being duplicated instead of shared.

aciidgh commented 7 years ago

I could not reproduce the issue with latest Swift 4 toolchain (swift-4.0-DEVELOPMENT-SNAPSHOT-2017-09-14-a-ubuntu16.04.tar.gz). I built the package omr-agentcore in the attached zip and the symbols and file size seems to be fine.

```
root@cad43c335aa7:/swiftpm# du -h ./.build/x86_64-unknown-linux/debug/libhcapiplugin.so
40K ./.build/x86_64-unknown-linux/debug/libhcapiplugin.so
root@cad43c335aa7:/swiftpm# du -h ./.build/x86_64-unknown-linux/debug/libagentcore.so
1.5M ./.build/x86_64-unknown-linux/debug/libagentcore.so
```

Can you tell me the exact steps you did to reproduce this issue?

swift-ci commented 7 years ago

Comment by Sian January (JIRA)

I think I have accidentally uploaded an older SwiftMetrics.tar.gz which I created for a different bug report. I've uploaded SR-5849.tar.gz which should be right, sorry about that. Still seeing it on the 9/17 snapshot.

aciidgh commented 7 years ago

I am running into this error now:

root@587787ba80c6:/swiftpm# swift build
Compile hcapiplugin APIConnector.cpp
Compile agentcore ibmras/monitoring/Plugin.cpp
Compile agentcore ibmras/monitoring/connector/ConnectorManager.cpp
Compile agentcore ibmras/monitoring/connector/configuration/ConfigurationConnector.cpp
Compile agentcore ibmras/monitoring/agent/threads/WorkerThread.cpp
Compile agentcore ibmras/monitoring/agent/threads/ThreadPool.cpp
Compile agentcore ibmras/monitoring/agent/SystemReceiver.cpp
Compile agentcore ibmras/monitoring/agent/BucketList.cpp
Compile agentcore ibmras/monitoring/agent/Bucket.cpp
Compile agentcore ibmras/monitoring/agent/Agent.cpp
Compile agentcore ibmras/common/util/sysUtils.cpp
Compile agentcore ibmras/common/util/strUtils.cpp
Compile agentcore ibmras/common/util/LibraryUtils.cpp
Compile agentcore ibmras/common/util/FileUtils.cpp
Compile agentcore ibmras/common/PropertiesFile.cpp
Compile agentcore ibmras/common/Properties.cpp
Compile agentcore ibmras/common/port/ThreadData.cpp
Compile agentcore ibmras/common/port/Lock.cpp
Compile agentcore ibmras/common/port/linux/Thread.cpp
Compile agentcore ibmras/common/port/linux/Process.cpp
Compile agentcore ibmras/common/MemoryManager.cpp
Compile agentcore ibmras/common/LogManager.cpp
Compile agentcore ibmras/common/Logger.cpp
Compile paho src/utf-8.c
Compile paho src/Tree.c
Compile paho src/Thread.c
Compile paho src/StackTrace.c
Compile paho src/SocketBuffer.c
Compile paho src/Socket.c
Compile paho src/MQTTProtocolOut.c
Compile paho src/MQTTProtocolClient.c
Compile paho src/MQTTPersistenceDefault.c
Compile paho src/MQTTPersistence.c
Compile paho src/MQTTPacketOut.c
Compile paho src/MQTTPacket.c
Compile paho src/MQTTAsync.c
Compile paho src/Messages.c
Compile paho src/Log.c
Compile paho src/LinkedList.c
Compile paho src/Heap.c
Compile paho src/Clients.c
Compile cpuplugin cpuplugin.cpp
Compile memplugin MemoryPlugin.cpp
Compile envplugin envplugin.cpp
Compile mqttplugin MQTTConnector.cpp
Linking ./.build/x86_64-unknown-linux/debug/libhcapiplugin.so
Linking ./.build/x86_64-unknown-linux/debug/libagentcore.so
In file included from /swiftpm/src/mqttplugin/MQTTConnector.cpp:32:
/swiftpm/src/mqttplugin/../agentcore/ibmras/common/port/Process.h:20:10: fatal error: 'AgentExtensions.h' file not found
#include "AgentExtensions.h"
         ^
1 error generated.
error: terminated(1): /usr/bin/swift-build-tool -f /swiftpm/.build/debug.yaml main
aciidgh commented 7 years ago

Ah, I see the problem. The thing is, SwiftPM does not allow you have product dependencies in the same package. If you need `hcapiplugin` to dynamically depend on `agentcore`, it should be in a separate package.

aciidgh commented 7 years ago

@ddunbar Do you think we should allow targets to depend on products in the same package?

swift-ci commented 7 years ago

Comment by Sian January (JIRA)

Did you get it to reproduce? I just extracted SR-5849.tar.gz and then ran cd SwiftMetrics then swift build and it works for me.

When you say SwiftPM does not allow it, do you mean it's unsupported? I didn't see this documented, could you point me to it please? The problem I had was that it will actually build the libraries, we just got some weird runtime errors on Linux. I don't see the runtime errors on the Mac so I assumed it was building correctly. IMO It would be better from a user's POV if it errored out at build time if the use case definitely can't be supported.

As an aside, to get it to build on Linux I actually remove the dependency and because of the way it loads the libraries at runtime it works ok, but it won't build on the Mac without the dependency because hcapiplugin needs header files from agentcore that aren't in the include directory (sandboxing?). So the actual package.swift looks like this, which is kind of weird.

ddunbar commented 7 years ago

I think it would be reasonable to allow a library to depend on a product from the same package; it makes sense when users are intentionally choosing to build dynamic libraries.

I also think we should have a warning if we notice that a dependency graph has the same module being linked into the same top-level product via two separate products (i.e. statically included and present via a dynamic dependency).

swift-ci commented 7 years ago

Comment by Sian January (JIRA)

👍

aciidgh commented 7 years ago

sjanuary (JIRA User) Unfortunately, We don't have detailed documentation on products yet but I plan to add it very soon (within the next 2 weeks). The unsupported part is about targets being able to depend on products from the same package. This is why you run into the issue where the same symbol is present in two products. For now, you can workaround this by keeping these two targets in different packages. Let me know if you have more questions. You can also message me on slack for quick turnaround time.

ephemer commented 7 years ago

With my superficial understanding of SwiftPM internals, I can see two (potentially!) reasonable ways around this:

  1. Where we define targets, allow "weak dependencies" that don't pull all the symbols statically into the built product when that target is included in a build product. I'm not sure how this would work with runtime linking though - there'd be no indication of where the symbols should come from so the dylib would have to be loaded manually (and I'm not sure this would work for mangled Swift symbols etc.)

  2. When building products, build up a dependency tree in memory. Any targets listed under other build products are not built and are instead linked dynamically to those products (whose names are known at build time). This would solve the naming issue from 1), but may involve more complexity re: building the dependency graph for all targets in memory.