-
Notifications
You must be signed in to change notification settings - Fork 43
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
Conversation
There was a problem hiding this 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
.
When downloading a tag off of GitHub the folder structure differs to a download of a commit sha.
@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. |
There was a problem hiding this 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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
version := 0.52.1 | |
version := 0.52.2 |
I released libp2p
v0.52.2
today. Want to include it in this pull request?
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.
There was a problem hiding this comment.
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 👍
There was a problem hiding this 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.
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