Specify minimum platform versions in Package.swift
robinkunde opened this issue · 15 comments
There's a bug (https://bugs.swift.org/browse/SR-14048) in the Xcode build system that can be triggered in rare circumstances if a SwiftPM dependency does not specify its minimum required platform versions. Is that something we could add to the next version? I would open a PR bug I'm not sure which versions of iOS, etc should be supported.
I think the mere presence of the declarations in a Package.swift causes the issue. It's easily tested though.
Oh, and this doesn't really work anyway: the lowest iOS version supported by a SwiftPM is derived from the SDK in which it runs, meaning that it's a moving target.
@Lukasa but couldn't we just say platforms: [.iOS7, ...]? That's a lower bound, right?
Hmm, that seems like that might work? It's gotta be ios9 though, that's what Swift 5.4 puts as the lower bound, but that seems like that works?
seems to me like fixings it at the individual package level is the wrong spot, iow this would be better addressed either at SwifPM or Xcode level? @neonichu @abertelrud wdyt?
I don't understand the issue tbh. Yes, there is a default deployment target for each platform and that will be used if nothing else is configured. Since Xcode may drop deployment targets in future versions, the default may move in the future, so if a package wants to pin their deployment target, they naturally have to do it manually.
The issue is:
- SR-14048 is triggered in rare cases by having SwiftPM packages not express an explicit lower bound on their supported target platform.
- swift-log does not currently express an explicit lower bound
- Adding explicit lower bounds is normally semver-major as it forces depending packages to add one as well.
- Unless, apparently, you choose a bound lower than the minimum that SwiftPM supports??? (<— this one remains unclear to me)
Importantly, what I believe @tomerd is suggesting is that instead of applying (4) as a hacky workaround, we should instead fix (1).
With all of the above said, I don’t think from my read of SR-14048 that having an unspecified lower bound is specifically the problem. Our actual lower bound genuinely is that low.
But SR-14048 is a bug in the package that is being used, right? It has a header that errors for iOS < 10 but doesn't specify a minimum so it can build for iOS 9 (depending on the used Xcode). What am I missing here and what particular behavior do we want to change in SwiftPM? (Maybe more a question for @tomerd)
Unless, apparently, you choose a bound lower than the minimum that SwiftPM supports??? (<— this one remains unclear to me)
I think that makes sense to me, because the package hasn't changed and can still build for an older version if the tool being used supports it. It is just that the tool being used does not support building for that old a version anymore and therefore raises the deployment target to the oldest supported version. It doesn't seem like a major version bump is required here, because it is not possible to build for the older version in any case, we're just preventing a hard error from happening.
Yes, I agree: based on the resolution of the upstream issue I don’t see that we need to change anything here.
But SR-14048 is a bug in the package that is being used, right? It has a header that errors for iOS < 10 but doesn't specify a minimum so it can build for iOS 9 (depending on the used Xcode). What am I missing here and what particular behavior do we want to change in SwiftPM? (Maybe more a question for @tomerd)
Unless, apparently, you choose a bound lower than the minimum that SwiftPM supports??? (<— this one remains unclear to me)
I think that makes sense to me, because the package hasn't changed and can still build for an older version if the tool being used supports it. It is just that the tool being used does not support building for that old a version anymore and therefore raises the deployment target to the oldest supported version. It doesn't seem like a major version bump is required here, because it is not possible to build for the older version in any case, we're just preventing a hard error from happening.
@neonichu I was out sick for a few days, so I couldn't chime in. There's a couple of things at play here, so I want to clarify the context.
- My takeaway from the discussion in SR-14048 was that there were two issues in LLDB:
a) it can crash when it picks the wrong deployment target for a package.
b) the deployment target it picks for a package that does not specify its own is non-deterministic and influenced by other packages. - The non-deterministic interaction made it much harder to figure what the underlying issue was.
- At least one of the packages involved should have specified a minimum version in Package.swift, but the tooling currently does not flag the need to do so, so it's easy to overlook.
It wasn't obvious to me if swift-log didn't specify a minimum platform version on purpose or not, so I decided to open an issue. I hope that helps explain my thought process.
From the discussion here, it appears that specifying any minimum version for swift-log would be more trouble than it's worth, which is completly understandable.
It wasn't obvious to me if swift-log didn't specify a minimum platform version on purpose or not, so I decided to open an issue. I hope that helps explain my thought process.
I think this is the critical note: it’s impossible to tell, from the outside of a project, whether the absence of a minimum deployment target is an oversight (and they actually expect a higher version) or not. In this case, swift-log genuinely does support back to the old releases, so it’s happy to leave things unconstrained.
It is a real shame that SwiftPM doesn’t gracefully tolerate adding constraints without breaking downstream builds, but it’s been that way for a while
the deployment target it picks for a package that does not specify its own is non-deterministic and influenced by other packages.
Thanks for your clarification, @robinkunde, I missed that the Jira had more comments that were hidden by default. It seems to me that this isn't an issue just for packages, but for any kind of modules when using LLDB, but it might be exacerbated by packages because it is common that they use older deployment target because of the defaults. I'm not sure we have a solution here on the SwiftPM side, because the older deployment target is typically legitimate.
I think this is the critical note: it’s impossible to tell, from the outside of a project, whether the absence of a minimum deployment target is an oversight (and they actually expect a higher version) or not.
I don't think it's possible for it to be an oversight, because we will always build the package with that deployment target. If the package actually does not support say iOS 9, it won't ever build for that platform because it will always be build for iOS 9 (safe for the issue where someone is using a newer Xcode that dropped a version, so they never tested it with that old a deployment target). Now there is the case where someone is making a package for one particular platform and doesn't care about the others, but I think that is more of a problem of the missing ability to restrict the platforms a package supports.
It is a real shame that SwiftPM doesn’t gracefully tolerate adding constraints without breaking downstream builds, but it’s been that way for a while
This was intentional (see the proposal) because we thought that changing the deployment target would usually be a major semver bump anyway, making this unnecessary. We should probably revisit that assumption if it doesn't hold true in practice.
Thanks for the discussion. I've got workarounds for my personal use cases, and it doesn't sound like there's anything to be done for now, so I'm closing this issue for now.