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

fix: 0.50 pin libp2p-quic prerelease deps #3549

Closed
wants to merge 1 commit into from

Conversation

ozwaldorf
Copy link
Contributor

@ozwaldorf ozwaldorf commented Mar 3, 2023

Description

backport of #3548, and should fully resolve #3537

Notes

  • libp2p-quic bumped to -alpha.0
    • pinned libp2p-tls -alpha
  • bumped libp2p to 0.50.2
    • bumped to new libp2p-quic version

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • A changelog entry has been made in the appropriate crates

@ozwaldorf ozwaldorf changed the base branch from master to v0.50 March 3, 2023 20:32
version = "0.7.0-alpha"
version = "0.7.0-alpha.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

It is released without the 0 so I think we can't just change that now?

Copy link
Contributor Author

@ozwaldorf ozwaldorf Mar 3, 2023

Choose a reason for hiding this comment

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

Hmm, would cargo publish actually reject this version? the next version (and latest) is -alpha.2, does it have a check to ensure the prerelease number is incremental? afaik, since it's still unique it'd work. Also just tried a cargo publish --dry-run and it seems to pass at least on that side of things

Copy link
Contributor

Choose a reason for hiding this comment

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

What is the upside of publishing a new version?

Copy link
Contributor Author

@ozwaldorf ozwaldorf Mar 4, 2023

Choose a reason for hiding this comment

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

We need a version of libp2p-quic to depend on from libp2p 0.50 that has libp2p-tls pinned, so that it wont pull [email protected] anymore but instead correctly use tls@-alpha (which is still causing libp2p 0.50.1 to break)

Also see the comment from @mxinden #3548 (comment)

Since a later version of quic has already been released, to avoid mixing up the versions i used alpha.0 for the version to retain some ordering but still have a unique one to pin libp2p against

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah yes, you are completely right. I forgot about the dependency from libp2p-quic to libp2p-tls.

@ozwaldorf
Copy link
Contributor Author

Any updates here?

@thomaseizinger
Copy link
Contributor

I still need to verify that we can actually release this. @mxinden do you know?

@thomaseizinger
Copy link
Contributor

Do you actually need this to fix a problem where you can't just keep the version pinned in Cargo.lock? Would updating to v0.51 be an option for you?

@ozwaldorf
Copy link
Contributor Author

ozwaldorf commented Mar 9, 2023

Do you actually need this to fix a problem where you can't just keep the version pinned in Cargo.lock? Would updating to v0.51 be an option for you?

We aren't currently pushing locks atm. To temporarily fix things on our end, we're now using a patch with the git rev for 0.50.1, which works since it loads all deps from that commit and ignores the crate releases. We are planning to update to 0.51 as well, although it'll take some time as migration was not as easy as initially expected.

@thomaseizinger
Copy link
Contributor

Okay thanks for that.

Feel free to tag us in any update PRs if you have questions!

cc @mxinden The update to 0.51 seems to be more difficult than expected so I think it is worth trying to resolve this.

@thomaseizinger
Copy link
Contributor

I am proposing a different strategy on how to handle this: #3580.

@thomaseizinger
Copy link
Contributor

Closing this as a wont-fix (for the v0.50 version in particular). Please upgrade to v0.51 if you can. Feel free to tag us in PRs if you need help.

#3580 should prevent this in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants