Skip to content

Conversation

Lukasa
Copy link
Contributor

@Lukasa Lukasa commented Aug 13, 2021

Motivation:

AWSLambdaRuntimeCore and MockServer both have dependencies on the NIO
module, but they don't express this in their Package.swift: they
implicitly rely on these dependencies being met by the NIOHTTP1 import.
This was always brittle: it could lead to timing issues in the build
that could lead to it failing.

Modifications:

  • Correctly express the dependency on NIO in Package.swift
  • Correctly import _NIOConcurrency in files where it's used

Result:

Better and more accurate builds.

Motivation:

AWSLambdaRuntimeCore and MockServer both have dependencies on the NIO
module, but they don't express this in their Package.swift: they
implicitly rely on these dependencies being met by the NIOHTTP1 import.
This was always brittle: it could lead to timing issues in the build
that could lead to it failing.

Modifications:

- Correctly express the dependency on NIO in Package.swift
- Correctly import _NIOConcurrency in files where it's used

Result:

Better and more accurate builds.
@Lukasa Lukasa force-pushed the cb-fix-up-package-file branch from 2f31557 to 6ed1760 Compare August 13, 2021 08:09
Copy link
Contributor

@fabianfett fabianfett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for spotting and fixing this!

@fabianfett fabianfett merged commit 48dc507 into awslabs:main Aug 13, 2021
@fabianfett
Copy link
Contributor

Should we cherry pick this into a patch release? @tomerd @Lukasa

@tomerd
Copy link
Contributor

tomerd commented Aug 13, 2021

Should we cherry pick this into a patch release? @tomerd @Lukasa

+1

@tomerd tomerd added the 🔨 semver/patch No public API change. label Aug 13, 2021
Lukasa added a commit to Lukasa/swift-nio that referenced this pull request Aug 16, 2021
Motivation:

In apple#1935 I removed NIO as a dependency of a number of our library
targets. This unfortunately led to some downstream breakage in modules
that were (incorrectly) assuming they could import NIO without
expressing a dependency on it in their Package.swift, e.g.
awslabs/swift-aws-lambda-runtime#218.

While a forums thread
(https://forums.swift.org/t/semantic-versioning-should-removing-a-dependency-be-a-semver-major/51179)
is ongoing to discuss the implications of this, we should re-add the
dependency to undo the breakage against main. I've validated this
locally: merely having the dependency is enough, we don't have to use
it.

Modifications:

- Re-add NIO as a dependency to:
    - _NIOConcurrency
    - NIOFoundationCompat
    - NIOHTTP1
    - NIOTLS
    - NIOWebSocket

Result:

Broken downstreams can build again.
Lukasa added a commit to apple/swift-nio that referenced this pull request Aug 16, 2021
Motivation:

In #1935 I removed NIO as a dependency of a number of our library
targets. This unfortunately led to some downstream breakage in modules
that were (incorrectly) assuming they could import NIO without
expressing a dependency on it in their Package.swift, e.g.
awslabs/swift-aws-lambda-runtime#218.

While a forums thread
(https://forums.swift.org/t/semantic-versioning-should-removing-a-dependency-be-a-semver-major/51179)
is ongoing to discuss the implications of this, we should re-add the
dependency to undo the breakage against main. I've validated this
locally: merely having the dependency is enough, we don't have to use
it.

Modifications:

- Re-add NIO as a dependency to:
    - _NIOConcurrency
    - NIOFoundationCompat
    - NIOHTTP1
    - NIOTLS
    - NIOWebSocket

Result:

Broken downstreams can build again.
fabianfett pushed a commit that referenced this pull request Aug 24, 2021
- Correctly express the dependency on NIO in Package.swift
- Correctly import _NIOConcurrency in files where it's used
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🔨 semver/patch No public API change.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants