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

ping/rust: remove the master dependency #45

Merged
merged 3 commits into from
Sep 23, 2022
Merged

ping/rust: remove the master dependency #45

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
290b21e
ping/rust: pin sdk-rust version
laurentsenta Sep 22, 2022
3bae10d
ping/rust: make master branch explicit
laurentsenta Sep 22, 2022
fae0ea9
ping/rust: drop version for master builds
laurentsenta Sep 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 4 additions & 5 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,13 @@ When a new version of libp2p is released, we want to make it permanent in the `p
When a new version of libp2p is released, we want to make it permanent in the `ping/rust` test folder.

1. In the `ping/_compositions/rust.toml` file,
- Copy the `[master]` section and turn it into a item in the `[[groups]]` array
- Update the `[master]` section with the new master version
- Copy the latest `[[groups]]` section and update it's `Id` and `CargoFeature` name.
2. In the `ping/rust` folder,
- `Cargo.toml`: update the feature flags `libp2pvxxx` to fix the released version and add the new `master`
- `src/main.rs`: Update the `mod libp2p` definition with the new master
- `Cargo.toml`: create the feature flags `libp2pvxxx` with the released version,
- `src/main.rs`: Update the `mod libp2p` definition with the new version,
- Run `cargo update` if needed. Try to build with `cargo build --features libp2pvxxx`
3. Run the test on your machine
- Do once, from the test-plans root: import the test-plans with `testground plan import ./ --name libp2p`
- Do once, from the test-plans root: import the test-plans with `testground plan import --from ./ --name libp2p`
- Run the test with `testground run composition -f ping/_compositions/rust-cross-versions.toml --wait`

## License
Expand Down
2 changes: 1 addition & 1 deletion ping/_compositions/go-rust-interop-latest.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@
CARGO_FEATURES = '{{ .CargoFeatures }}'
CARGO_REMOVE = '{{ .CargoFeatures }}'
CARGO_PATCH = """
{{ .CargoFeatures }} = {package = "libp2p", git = "https://{{ or $.Env.GitTarget "github.com/libp2p/rust-libp2p" }}", rev = "{{ $.Env.GitReference }}", default_features = false, features = [ "websocket", "mplex", "yamux", "tcp-async-io", "ping", "noise", "dns-async-std" ], version = "{{ .Version }}", optional = true}
{{ .CargoFeatures }} = {package = "libp2p", git = "https://{{ or $.Env.GitTarget "github.com/libp2p/rust-libp2p" }}", rev = "{{ $.Env.GitReference }}", default_features = false, features = [ "websocket", "mplex", "yamux", "tcp-async-io", "ping", "noise", "dns-async-std" ], optional = true}
"""
{{ end }}
{{ end }}
Expand Down
2 changes: 1 addition & 1 deletion ping/_compositions/go-rust-interop.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@
CARGO_FEATURES = '{{ .CargoFeatures }}'
CARGO_REMOVE = '{{ .CargoFeatures }}'
CARGO_PATCH = """
{{ .CargoFeatures }} = {package = "libp2p", git = "https://{{ or $.Env.GitTarget "github.com/libp2p/rust-libp2p" }}", rev = "{{ $.Env.GitReference }}", default_features = false, features = [ "websocket", "mplex", "yamux", "tcp-async-io", "ping", "noise", "dns-async-std" ], version = "{{ .Version }}", optional = true}
{{ .CargoFeatures }} = {package = "libp2p", git = "https://{{ or $.Env.GitTarget "github.com/libp2p/rust-libp2p" }}", rev = "{{ $.Env.GitReference }}", default_features = false, features = [ "websocket", "mplex", "yamux", "tcp-async-io", "ping", "noise", "dns-async-std" ], optional = true}
"""
{{ end }}
{{ end }}
Expand Down
2 changes: 1 addition & 1 deletion ping/_compositions/rust-cross-versions.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
CARGO_FEATURES = '{{ .CargoFeatures }}'
CARGO_REMOVE = '{{ .CargoFeatures }}'
CARGO_PATCH = """
{{ .CargoFeatures }} = {package = "libp2p", git = "https://{{ or $.Env.GitTarget "github.com/libp2p/rust-libp2p" }}", rev = "{{ $.Env.GitReference }}", default_features = false, features = [ "websocket", "mplex", "yamux", "tcp-async-io", "ping", "noise", "dns-async-std" ], version = "{{ .Version }}", optional = true}
{{ .CargoFeatures }} = {package = "libp2p", git = "https://{{ or $.Env.GitTarget "github.com/libp2p/rust-libp2p" }}", rev = "{{ $.Env.GitReference }}", default_features = false, features = [ "websocket", "mplex", "yamux", "tcp-async-io", "ping", "noise", "dns-async-std" ], optional = true}
"""
{{ end }}
{{ end }}
Expand Down
6 changes: 2 additions & 4 deletions ping/_compositions/rust.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
[master]
Version = '0.48.0'
CargoFeatures = 'libp2pv0480'
CargoFeatures = 'libp2pmaster'

[custom]
Version = '0.48.0'
CargoFeatures = 'libp2pv0480'
CargoFeatures = 'libp2pmaster'

[[groups]]
Id = "v0.47.0"
Expand Down
5 changes: 3 additions & 2 deletions ping/rust/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ rand = "0.8"
serde = {version = "1", features = ["derive"]}
serde_json = "1"
soketto = "0.7.1"
testground = {git = "https://github.com/testground/sdk-rust", branch = "master", version = "0.4.0"}
testground = {git = "https://github.com/testground/sdk-rust", rev = "94a9a72796f94cc7ca786a5f019d07f328c76d4b", version = "0.4.0"}
Copy link
Contributor

Choose a reason for hiding this comment

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

Just saw this change while working on #42. Was this a temporary change because until #42 is merged we are not compatible with the master branch, or should I also use a concrete revision there?

Copy link
Member

Choose a reason for hiding this comment

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

I just released testground v0.4.0. How about using that instead?

Copy link
Member

Choose a reason for hiding this comment

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

testground/sdk-rust#38

thiserror = "1"
tokio = { version = "1", default-features = false, features = ["sync", "rt-multi-thread", "macros", "net"] }
tokio-stream = { version = "0.1", default-features = false, features = [] }
Expand All @@ -24,4 +24,5 @@ libp2pv0440 = {package = "libp2p", default_features = false, features = [ "webso
libp2pv0450 = {package = "libp2p", default_features = false, features = [ "websocket", "mplex", "yamux", "tcp-async-io", "ping", "noise", "dns-async-std" ], version = "0.45.0", optional = true}
libp2pv0460 = {package = "libp2p", default_features = false, features = [ "websocket", "mplex", "yamux", "tcp-async-io", "ping", "noise", "dns-async-std" ], version = "0.46.0", optional = true}
libp2pv0470 = {package = "libp2p", default_features = false, features = [ "websocket", "mplex", "yamux", "tcp-async-io", "ping", "noise", "dns-async-std" ], version = "0.47.0", optional = true}
libp2pv0480 = {package = "libp2p", git = "https://github.com/libp2p/rust-libp2p", branch = "master", default_features = false, features = [ "websocket", "mplex", "yamux", "tcp-async-io", "ping", "noise", "dns-async-std" ], version = "0.48.0", optional = true}

libp2pmaster = {package = "libp2p", git = "https://github.com/libp2p/rust-libp2p", branch = "master", default_features = false, features = [ "websocket", "mplex", "yamux", "tcp-async-io", "ping", "noise", "dns-async-std" ], optional = true}
4 changes: 2 additions & 2 deletions ping/rust/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ use testground::network_conf::{
};

pub mod libp2p {
#[cfg(all(feature = "libp2pv0480",))]
pub use libp2pv0480::*;
#[cfg(all(feature = "libp2pmaster",))]
pub use libp2pmaster::*;

#[cfg(all(feature = "libp2pv0470",))]
pub use libp2pv0470::*;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

(creating here to get a thread)

@mxinden, I'm trying to catch other cases where the build might break after a push to master.

  1. If rust-libp2p remove a feature flag in master, I guess the libmaster line in Cargo.tomlwill become invalid, and it will break the build. Even for old versions, Correct?
  2. if rust-libp2p publishes an API breaking change in master, could it break the build of previous versions? I assume the decoration #[cfg(all(feature = "libp2pmaster",))] completely removes the code and we're safe here.

Copy link
Member

Choose a reason for hiding this comment

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

If rust-libp2p remove a feature flag in master, I guess the libmaster line in Cargo.tomlwill become invalid, and it will break the build. Even for old versions, Correct?

Correct. Though this will change soon where we can replace all the features with a single full. See libp2p/rust-libp2p#2918.

Copy link
Member

Choose a reason for hiding this comment

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

if rust-libp2p publishes an API breaking change in master, could it break the build of previous versions? I assume the decoration #[cfg(all(feature = "libp2pmaster",))] completely removes the code and we're safe here.

I am not sure I follow. Once a crate is published to crates.io it can never be changed again.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Apologies, I mixed versions and packages which is unclear,

I'm wondering if there are other cases where a push to rust-libp2p master branch can break the build for previous versions of the ping test (cargo build --feature libp2pv0470, for example).

  • We fixed one case with this PR (when we push a version bump),
  • We know another case (when we change feature flags),
  • Do you see other cases?

Trying to play around with macro it looks like it's enough, but I might miss some rust's edge cases.

// cargo build --features libp2pmaster (FAIL)
// cargo build --features libp2pv2 (OK)
fn main() {
    println!("got: {}", s() + 1)
}

#[cfg(all(feature = "libp2pmaster",))]
fn s() -> String {
    return String::from("new master API that breaks the current helloworld test");
}

#[cfg(all(feature = "libp2pv2",))]
fn s() -> i32 {
    return 2;
}

Expand Down