Skip to content

check Cargo.lock into the version control system to prevent errors caused by yanked dependence #388

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

zhaodiaoer
Copy link
Contributor

fix errors in testing code like :

error[E0277]: the trait bound `Box<FakeServer>: containerd_shim_protos::shim_async::Task` is not satisfied
  --> crates/shim-protos/examples/ttrpc-server-async.rs:63:32
   |
63 |     let tservice = create_task(Arc::new(Box::new(FakeServer::new())));
   |                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `containerd_shim_protos::shim_async::Task` is not implemented for `Box<FakeServer>`
   |
   = help: the trait `containerd_shim_protos::shim_async::Task` is implemented for `FakeServer`
   = note: required for the cast from `Arc<Box<FakeServer>>` to `Arc<(dyn containerd_shim_protos::shim_async::Task + Send + Sync + 'static)>`

@github-actions github-actions bot added C-shim-protos Shim protos C-shim Containerd shim labels Apr 8, 2025
@jprendes
Copy link
Contributor

jprendes commented Apr 8, 2025

I don't see this error on my dev environment.

However, the underlying issue is caused by this change
containerd/ttrpc-rust#235

That breaking change was "briefly" (for 3 months) released as 0.6.3 without bumping the major version, but that version is now yanked (as of mid January 2025), and re-released as 0.7.0. See the versions page in crates.io.
Which version of ttrpc-compiler are you using?
What's the output of cargo tree -i -p ttrpc?
If you see 0.6.3, please try updating your Cargo.lock file and try again.

When we bump the ttrpc dependencies, then we would need to fix this breaking change as well here.

@github-actions github-actions bot removed C-shim-protos Shim protos C-shim Containerd shim labels Apr 9, 2025
@zhaodiaoer
Copy link
Contributor Author

I don't see this error on my dev environment.

However, the underlying issue is caused by this change containerd/ttrpc-rust#235

That breaking change was "briefly" (for 3 months) released as 0.6.3 without bumping the major version, but that version is now yanked (as of mid January 2025), and re-released as 0.7.0. See the versions page in crates.io. Which version of ttrpc-compiler are you using? What's the output of cargo tree -i -p ttrpc? If you see 0.6.3, please try updating your Cargo.lock file and try again.

When we bump the ttrpc dependencies, then we would need to fix this breaking change as well here.

Thanks for your context, you are right, the version of ttrpc-compiler in my Cargo.lock is precisely 0.6.3, when i have
downgraded it to 0.6.2, all errors have gone:

$ cargo tree -i -p ttrpc-compiler

ttrpc-compiler v0.6.3
└── ttrpc-codegen v0.4.2
    [build-dependencies]
    └── containerd-shim-protos v0.8.0 (/home/bytedance/code/opensource/zhaodiaoer/rust-extensions/crates/shim-protos)
        └── containerd-shim v0.8.0 (/home/bytedance/code/opensource/zhaodiaoer/rust-extensions/crates/shim)
            └── containerd-runc-shim v0.1.1 (/home/bytedance/code/opensource/zhaodiaoer/rust-extensions/crates/runc-shim)

But now i'm staring to be curious about another thing: should we let Cargo.lock out from .gitignore? Otherwise, we often
need to handle similar matters manually.

@zhaodiaoer zhaodiaoer changed the title fix: type errors in testing code check Cargo.lock into the version control system to prevent errors caused by yanked dependence Apr 9, 2025
@jprendes
Copy link
Contributor

jprendes commented Apr 9, 2025

should we let Cargo.lock out from .gitignore?

Since containerd-runc-shim is a binary crate, I think you are right and we should track Cargo.lock.
See the official advice.

@zhaodiaoer
Copy link
Contributor Author

@mxpv @jsturtevant PTAL, thanks!

@jokemanfire
Copy link
Contributor

https://github.com/containerd/rust-extensions/pull/375/files
We could use arc::from instead of arc::new to be compatible with such modifications.

@zhaodiaoer
Copy link
Contributor Author

https://github.com/containerd/rust-extensions/pull/375/files We could use arc::from instead of arc::new to be compatible with such modifications.

Useful information, thanks!

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.

3 participants