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

chore: rm rust-libp2p v0.49 and v0.50, update v0.52 #253

Merged
merged 6 commits into from
Aug 11, 2023

Conversation

p-shahi
Copy link
Member

@p-shahi p-shahi commented Aug 10, 2023

I'm not sure if we should still test v0.49 and v0.50 given that they are quite old (from 2022)
0.51 and 0.52 have been out for some time now.
Wdyt @mxinden @thomaseizinger? If there are notable/many rust-libp2p users still relying on 0.50 or 0.49, we should keep them

Copy link
Member

@mxinden mxinden left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thank you for porting our Makefile to the new version.lock mechanism @p-shahi!

Substrate is on v0.52.1. Lighthouse is upgrading to v0.52. Most downloaded version is v0.52.1.

mxinden and others added 4 commits August 10, 2023 10:39
When downloading a tag off of GitHub the folder structure differs to a download
of a commit sha.
@p-shahi
Copy link
Member Author

p-shahi commented Aug 11, 2023

@mxinden I pushed some more fixes to the makefile. it seems like it's failing now as it doesn't detect webrtc-direct as a transport? I haven't looked further but tagging you in case you've seen something like this before.

Copy link
Member

@mxinden mxinden left a comment

Choose a reason for hiding this comment

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

Referencing a single version instead of a commit sha is a bit tricky with rust-libp2p's ability to release patch releases of sub-crates. I am sorry for not catching this earlier @p-shahi. Suggestions below.

Copy link
Member

Choose a reason for hiding this comment

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

I investigated a bit.

The v0.51.3 tag points (as it should) to the commit we cut the libp2p v0.51.3 release off of. Since then we have cut many patch releases of sub-crates. But those patch releases of sub-crates don't require an additional patch release of the meta crate (i.e. libp2p). Downstream users can upgrade their sub-crate version without having to upgrade the meta crate version.

The previously specified commitSha pointed to the latest commit compatible with the libp2p v0.51.3 release. I.e. it contained all sub-crate patch releases. Thus CI was green.

The 0.51.3 git tag only points to the much older v0.51.3 release commit. It does not contain the patches of the sub-crates. Thus CI fails.

You can see the difference here.

libp2p/rust-libp2p@3c5940a...8c26c61

We could cut v0.51.4. But I am hesitant to do so for CI only.

Is the move from commitSha to version relevant? If not, I suggest we don't do it for v0.51.3. In other words I suggest leaving this file untouched.

Copy link
Member Author

@p-shahi p-shahi Aug 11, 2023

Choose a reason for hiding this comment

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

Is the move from commitSha to version relevant? If not, I suggest we don't do it for v0.51.3. In other words I suggest leaving this file untouched.

I am ok with that. Thank you for investigating!

@@ -1,23 +1,35 @@
image_name := rust-v0.52
commitSha := 68e6bf9c3cd0d3317415a5ba31a62e91cba0a5d2
version := 0.52.1
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
version := 0.52.1
version := 0.52.2

I released libp2p v0.52.2 today. Want to include it in this pull request?

libp2p/rust-libp2p#4312

No worries in case you don't want to mix these efforts in the same PR. I am happy to do a follow-up pull request.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll include 0.52.2 in here 👍

Copy link
Member

@mxinden mxinden left a comment

Choose a reason for hiding this comment

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

Thanks @p-shahi! Feel free to merge once CI is green.

@p-shahi p-shahi changed the title chore: rm rust-libp2p v0.49 and v0.50, update v0.51, v0.52 chore: rm rust-libp2p v0.49 and v0.50, update v0.52 Aug 11, 2023
@p-shahi p-shahi merged commit da62ebc into master Aug 11, 2023
2 checks passed
@p-shahi p-shahi deleted the update-rust-versions branch August 11, 2023 19:17
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