swift-http-types icon indicating copy to clipboard operation
swift-http-types copied to clipboard

`import HTTPTypesFoundation` pulls in `FoundationNetworking`

Open weissi opened this issue 11 months ago • 16 comments

Unfortunately, import HTTPTypesFoundation pulls in FoundationNetworking unconditionally which has issues on Linux.

We carry link-time checks to make sure that we're not accidentally using URLSession in places where it's important not to. Unfortunately, modules that import HTTPTypesFoundation now also pull in FoundationNetworking.

Would it be possible to remove the import FoundationNetworking and move that to a separate module like HTTPTypesFoundationNetworking?

weissi avatar Dec 05 '24 00:12 weissi

Does this require a major version bump due to source breakages?

guoye-zhang avatar Dec 05 '24 23:12 guoye-zhang

Yeah, unfortunately it does due to the leaky nature of Swift imports. In this case it a "cheap" major, and we can encourage users to depend on a cross-major range.

Lukasa avatar Dec 06 '24 00:12 Lukasa

Yeah, unfortunately it does due to the leaky nature of Swift imports. In this case it a "cheap" major, and we can encourage users to depend on a cross-major range.

Yes, thank you! A "cheap major" is perfect here. We've done this before and asking users to do "1.0.0" ..< "3.0.0" is usually not a problem. Especially because it can be done at any point in time and doesn't break stuff either way.

weissi avatar Dec 06 '24 04:12 weissi

I checked and URLRequest and URLResponse are both in FoundationNetworking. HTTPTypesFoundation would have no API at all if it doesn't import FoundationNetworking.

Why do these packages import HTTPTypesFoundation?

guoye-zhang avatar Dec 06 '24 21:12 guoye-zhang

I checked and URLRequest and URLResponse are both in FoundationNetworking. HTTPTypesFoundation would have no API at all if it doesn't import FoundationNetworking.

Why do these packages import HTTPTypesFoundation?

Just for the extension HTTPRequest { public var url: URL? } :)

weissi avatar Dec 07 '24 00:12 weissi

Oh I see

guoye-zhang avatar Dec 07 '24 01:12 guoye-zhang

URL is part of FoundationEssentials, so that could be the only requirement there.

parkera avatar Dec 07 '24 02:12 parkera

Alternatively, can we move extension HTTPRequest { public var url: URL? } to the main HTTPTypes package? Can we assume that FoundationEssentials exists everywhere?

guoye-zhang avatar Dec 09 '24 06:12 guoye-zhang

Alternatively, can we move extension HTTPRequest { public var url: URL? } to the main HTTPTypes package? Can we assume that FoundationEssentials exists everywhere?

I don't think NIOExtras has FoundationEssentials, @Lukasa?

update: I checked, NIOExtras nor anything it depends on currently imports Foundation*.

weissi avatar Dec 09 '24 06:12 weissi

Correct, currently we don't use FoundationEssentials in NIO-world. I'm not sure how long that state of affairs will last though: the existence of FoundationEssentials relieves many of the constraints of depending on Foundation.

Lukasa avatar Dec 09 '24 09:12 Lukasa

FoundationEssentials is present on all non-Darwin platforms as part of the Swift toolchain. On Darwin it's still just Foundation.

parkera avatar Dec 09 '24 17:12 parkera

What's the difference between import Foundation and import FoundationEssentials on non-Darwin? Does import FoundationEssentials work on Swift 5.7 or is it more recent?

guoye-zhang avatar Dec 09 '24 17:12 guoye-zhang

import FoundationEssentials is available in Swift 6.0 or later. See here for more information on how these are all layered.

parkera avatar Dec 09 '24 17:12 parkera

Would you recommend that we do something like:

#if canImport(FoundationEssentials)
import FoundationEssentials
#else
import Foundation
#endif

guoye-zhang avatar Dec 09 '24 18:12 guoye-zhang

Yes that's the recommend pattern to check if it exists and import only that. IMO that's the way we should do it here and move the extension for URL to the main module of this package behind the conditional import.

FranzBusch avatar Dec 11 '24 13:12 FranzBusch

Are people all OK with moving URL to the main package? I can make a new PR to do that

guoye-zhang avatar Dec 12 '24 21:12 guoye-zhang