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.

Sign up for GitHub

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

Jump to bottom

Release v0.4.5 did a break change #308

Closed
bachue opened this issue May 10, 2022 · 7 comments
Closed

Release v0.4.5 did a break change #308

bachue opened this issue May 10, 2022 · 7 comments

Comments

@bachue
Copy link

bachue commented May 10, 2022

The PR "Replace winapi dependency with windows-sys" (#303) breaks public apis, cause dns-lookup are failed to compile on windows keeperofdakeys/dns-lookup#22. dns-lookup calls getnameinfo from winapi, but now socket2 exports SockAddr from windows-sys, which are incompatible with SockAddr from winapi.
@Thomasdezeeuw
Copy link
Collaborator

I've yanked v0.4.5 for now.

@bachue
Copy link
Author

bachue commented May 10, 2022

@Thomasdezeeuw 👍

@bachue bachue closed this as completed May 10, 2022
This was referenced May 10, 2022
Closed
Merged
olix0r added a commit to linkerd/linkerd2-proxy that referenced this issue May 18, 2022
@olix0r
Revert "build(deps): bump socket2 from 0.4.4 to 0.4.5 (#1654)"
65eb2f9 
socket2 v0.4.5 has been yanked (rust-lang/socket2#308).

This reverts commit 9004d9c.
@olix0r olix0r mentioned this issue May 18, 2022
Merged
olix0r added a commit to linkerd/linkerd2-proxy that referenced this issue May 18, 2022
@olix0r
Revert "build(deps): bump socket2 from 0.4.4 to 0.4.5 (#1654)"
eb1ff2b 
socket2 v0.4.5 has been yanked (rust-lang/socket2#308).

This reverts commit 9004d9c.

Signed-off-by: Oliver Gould <ver@buoyant.io>
olix0r added a commit to linkerd/linkerd2-proxy that referenced this issue May 18, 2022
@olix0r
Revert "build(deps): bump socket2 from 0.4.4 to 0.4.5 (#1654)" (#1681)
05c740d 
socket2 v0.4.5 has been yanked (rust-lang/socket2#308).

This reverts commit 9004d9c.

Signed-off-by: Oliver Gould <ver@buoyant.io>
hawkw pushed a commit to linkerd/linkerd2-proxy that referenced this issue May 18, 2022
@olix0r @hawkw
Revert "build(deps): bump socket2 from 0.4.4 to 0.4.5 (#1654)" (#1681)
afd80a6 
socket2 v0.4.5 has been yanked (rust-lang/socket2#308).

This reverts commit 9004d9c.

Signed-off-by: Oliver Gould <ver@buoyant.io>
hawkw pushed a commit to linkerd/linkerd2-proxy that referenced this issue May 18, 2022
@olix0r @dependabot
Simplify admin server feature-flagging in #1642 (#1682)
9c764c1 
* opencensus: Include empty generated protobuf (#1676)

prost generates empty files for google protobuf types. This change adds
these generated files so that they are not regenerated on `make test`.

Signed-off-by: Oliver Gould <ver@buoyant.io>

* build(deps): bump rustls from 0.20.5 to 0.20.6 (#1679)

Bumps [rustls](https://github.com/rustls/rustls) from 0.20.5 to 0.20.6.
- [Release notes](https://github.com/rustls/rustls/releases)
- [Changelog](https://github.com/rustls/rustls/blob/main/RELEASE_NOTES.md)
- [Commits](rustls/rustls@v/0.20.5...v/0.20.6)

---
updated-dependencies:
- dependency-name: rustls
  dependency-type: indirect
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Revert "build(deps): bump socket2 from 0.4.4 to 0.4.5 (#1654)" (#1681)

socket2 v0.4.5 has been yanked (rust-lang/socket2#308).

This reverts commit 9004d9c.

Signed-off-by: Oliver Gould <ver@buoyant.io>

* build(deps): bump EmbarkStudios/cargo-deny-action from 1.2.17 to 1.3.0 (#1678)

Bumps [EmbarkStudios/cargo-deny-action](https://github.com/EmbarkStudios/cargo-deny-action) from 1.2.17 to 1.3.0.
- [Release notes](https://github.com/EmbarkStudios/cargo-deny-action/releases)
- [Commits](EmbarkStudios/cargo-deny-action@3481b77...b655a95)

---
updated-dependencies:
- dependency-name: EmbarkStudios/cargo-deny-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* build(deps): bump clang-sys from 1.3.1 to 1.3.2 (#1680)

Bumps [clang-sys](https://github.com/KyleMayes/clang-sys) from 1.3.1 to 1.3.2.
- [Release notes](https://github.com/KyleMayes/clang-sys/releases)
- [Changelog](https://github.com/KyleMayes/clang-sys/blob/master/CHANGELOG.md)
- [Commits](https://github.com/KyleMayes/clang-sys/commits)

---
updated-dependencies:
- dependency-name: clang-sys
  dependency-type: indirect
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Simplify admin server feature-flagging in #1642

* Rather than push feature flagging down into the `serve` logic, we can
  avoid even matching the logs.json endpoint when the feature isn't
  disabled. This avoids the need for conditional feature gatingc in the
  handler.
* The log module is split into two submodules: level and stream. The
  stream module is feature-gated. This allows to extract function-local
  defintions into the module, which simplifies scoping and makes things
  easier to read (imo).

Signed-off-by: Oliver Gould <ver@buoyant.io>

* bump thingbuf to 1.0.3

Signed-off-by: Oliver Gould <ver@buoyant.io>

* to_stream => into_stream

Signed-off-by: Oliver Gould <ver@buoyant.io>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@sn99
Copy link

sn99 commented Jun 3, 2022

@Thomasdezeeuw I am working on shifting dns-lookup from winapi to window_sys, I am using your master branch instead of crates.io and it seems to be working, I will make a pull in dns-lookup soon.

This was referenced Jun 3, 2022
Merged
Merged
@Thomasdezeeuw
Copy link
Collaborator

@Thomasdezeeuw I am working on shifting dns-lookup from winapi to window_sys, I am using your master branch instead of crates.io and it seems to be working, I will make a pull in dns-lookup soon.

Sounds good, but unfortunately we still can't make a breaking change in a v0.4.x release, so the switch to window_sys will have to go into v0.5.

@sn99
Copy link

sn99 commented Jun 5, 2022

@Thomasdezeeuw I am working on shifting dns-lookup from winapi to window_sys, I am using your master branch instead of crates.io and it seems to be working, I will make a pull in dns-lookup soon.

Sounds good, but unfortunately we still can't make a breaking change in a v0.4.x release, so the switch to window_sys will have to go into v0.5.

Oh yeah, no problem, just wanted to inform you about the change in dns-lookup

@sn99 sn99 mentioned this issue Jun 9, 2022
Merged
Thomasdezeeuw added a commit to Thomasdezeeuw/socket2 that referenced this issue Aug 26, 2022
@Thomasdezeeuw
Revert back to winapi dependency
edcab0d 
As noted in <rust-lang#308>, switching
from winapi to windows-sys was a breaking change as we expose some of
the winapi types that are different in windows-sys.

This reverts the following commits:
 * "Relax windows-sys dependency version" 81056b9.
 * "Support Socket::(set_)recv_tos on Windows" 242e087.
 * "Replace winapi with windows-sys" aeb6c2d.

We'll make this switch in v0.5 instead.
@Thomasdezeeuw Thomasdezeeuw mentioned this issue Aug 26, 2022
Merged
Thomasdezeeuw added a commit that referenced this issue Aug 26, 2022
@Thomasdezeeuw
Revert back to winapi dependency
daf64ce 
As noted in <#308>, switching
from winapi to windows-sys was a breaking change as we expose some of
the winapi types that are different in windows-sys.

This reverts the following commits:
 * "Relax windows-sys dependency version" 81056b9.
 * "Support Socket::(set_)recv_tos on Windows" 242e087.
 * "Replace winapi with windows-sys" aeb6c2d.

We'll make this switch in v0.5 instead.
@keeperofdakeys
Copy link
Contributor

For future reference, since socket2 is exposing the SOCKADDR type from the underlying windows_sys library via getaddrinfo/getnameinfo, any change to SOCKADDR in windows_sys will also change the SOCKADDR that socket2 uses. Apparently windows_sys has changed SOCKADDR A Few Times recently, which means the exact subversion of windows_sys needs to match in libraries using socket2.

Since both windows_sys and socket2 are version 0.X.Y, I'd suggest that anybody using these libraries hard code socket2 and windows_sys versions explicitly. We'll likely need to wait till both libraries are version 1.0 before we can get stable APIs.

@Thomasdezeeuw
Copy link
Collaborator

For future reference, since socket2 is exposing the SOCKADDR type from the underlying windows_sys library via getaddrinfo/getnameinfo, any change to SOCKADDR in windows_sys will also change the SOCKADDR that socket2 uses. Apparently windows_sys has changed SOCKADDR A Few Times recently, which means the exact subversion of windows_sys needs to match in libraries using socket2.

Since both windows_sys and socket2 are version 0.X.Y, I'd suggest that anybody using these libraries hard code socket2 and windows_sys versions explicitly. We'll likely need to wait till both libraries are version 1.0 before we can get stable APIs.

Those changes are just aliases for the same underlying types. The actual layout of the struct is fixed as any changes to it would break ABI compatibility with Windows' system calls. So I don't think this is an issue.

Relying on 0.x libraries, especially exposing them in the public API as socket2 does, isn't great, but having to maintain these (public) types/type aliases ourselves wouldn't be an improvement either.

keeperofdakeys referenced this issue in keeperofdakeys/dns-lookup May 3, 2023
Pin socket2 at version 0.5.2
8fe8618 
Socket2 exposes definitions from windows_sys, which
has changed the definition of SOCKADDR a few times. Pin
socket2 and windows_sys until they reach 1.0 and get a
stable API.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@keeperofdakeys @bachue @Thomasdezeeuw @sn99