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

import HTTPTypesFoundation pulls in FoundationNetworking #76

Open
weissi opened this issue Dec 5, 2024 · 16 comments · May be fixed by #77 or #79
Open

import HTTPTypesFoundation pulls in FoundationNetworking #76

weissi opened this issue Dec 5, 2024 · 16 comments · May be fixed by #77 or #79

Comments

@weissi
Copy link
Member

weissi commented Dec 5, 2024

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?

@guoye-zhang
Copy link
Contributor

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

@Lukasa
Copy link

Lukasa commented Dec 6, 2024

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.

@weissi
Copy link
Member Author

weissi commented Dec 6, 2024

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.

@guoye-zhang
Copy link
Contributor

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?

@weissi
Copy link
Member Author

weissi commented Dec 7, 2024

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? } :)

@guoye-zhang
Copy link
Contributor

Oh I see

@parkera
Copy link
Member

parkera commented Dec 7, 2024

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

@guoye-zhang
Copy link
Contributor

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

@weissi
Copy link
Member Author

weissi commented Dec 9, 2024

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*.

@Lukasa
Copy link

Lukasa commented Dec 9, 2024

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.

@parkera
Copy link
Member

parkera commented Dec 9, 2024

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

@guoye-zhang
Copy link
Contributor

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?

@parkera
Copy link
Member

parkera commented Dec 9, 2024

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

@guoye-zhang
Copy link
Contributor

Would you recommend that we do something like:

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

@FranzBusch
Copy link
Member

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.

@guoye-zhang
Copy link
Contributor

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

@guoye-zhang guoye-zhang linked a pull request Dec 12, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants