fix: remove unsupport configurations in Cargo.toml #2035
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yangby-cryptape
force-pushed
the
pr/fix-cargo-toml
branch
from
a17e26a to
a6ca5e7
Compare
quake
previously approved these changes
Apr 23, 2020
driftluo
previously approved these changes
Apr 25, 2020
|
bors r=quake, driftluo |
bors bot
added a commit
that referenced
this pull request
Apr 26, 2020
2035: fix: remove unsupport configurations in Cargo.toml r=quake,driftluo a=yangby-cryptape ### The Issue Here is a potential issue which was introduced by #1940. - Since our `rust-toolchain` is still be `1.41.0`, there will be no errors or warnings when we build the project. (This is why #1940 was merged) https://github.com/nervosnetwork/ckb/blob/22829ae85db1643480bd810150f9619250aac92e/rust-toolchain#L1 - But if you update the `rust-toolchain` to `1.42.0`, when you build the whole project (just execute `make prod`), you will get two warnings: > warning: ckb/Cargo.toml: Found `feature = ...` in `target.'cfg(...)'.dependencies`. This key is not supported for selecting dependencies and will not work as expected. Use the [features] section instead: https://doc.rust-lang.org/cargo/reference/features.html > warning: ckb/Cargo.toml: Found `feature = ...` in `target.'cfg(...)'.dependencies`. This key is not supported for selecting dependencies and will not work as expected. Use the [features] section instead: https://doc.rust-lang.org/cargo/reference/features.html ~~Although I haven't found any evidences now, I still strongly suspect that it was related to our recent memory leak.~~ **After a lot of tests, I think this issue was not related to our recent memory leak.** But it's still a bug. ### My Solution I read a lot of issues about cargo in few weeks, I didn't find any official / standard solution to fix it, and I thought they wouldn't fix it in few months. (`build.rs` can't fix it, too) So, I use a complex and dirty way to fix it, just review the code, please. #### How can I prove my modification was valid? - Please check [this example](https://github.com/yangby-cryptape/rust-features-trials/tree/99455ef/features/example-1), I have run it under Debian Buster and Windows 10. - I have build various kinds of CKB with different features and check a lot logs of them under Debian Buster. ### A Very Important Note **If any features of a rust bindings (to c library) crate was changed, please do `cargo clean` before build the project.** Some C libraries wouldn't re-compile by cargo after features changed, for example, `jemalloc-sys`. Co-authored-by: Boyu Yang <[email protected]>
|
bors retry |
bors bot
added a commit
that referenced
this pull request
Apr 26, 2020
2035: fix: remove unsupport configurations in Cargo.toml r=quake,driftluo a=yangby-cryptape ### The Issue Here is a potential issue which was introduced by #1940. - Since our `rust-toolchain` is still be `1.41.0`, there will be no errors or warnings when we build the project. (This is why #1940 was merged) https://github.com/nervosnetwork/ckb/blob/22829ae85db1643480bd810150f9619250aac92e/rust-toolchain#L1 - But if you update the `rust-toolchain` to `1.42.0`, when you build the whole project (just execute `make prod`), you will get two warnings: > warning: ckb/Cargo.toml: Found `feature = ...` in `target.'cfg(...)'.dependencies`. This key is not supported for selecting dependencies and will not work as expected. Use the [features] section instead: https://doc.rust-lang.org/cargo/reference/features.html > warning: ckb/Cargo.toml: Found `feature = ...` in `target.'cfg(...)'.dependencies`. This key is not supported for selecting dependencies and will not work as expected. Use the [features] section instead: https://doc.rust-lang.org/cargo/reference/features.html ~~Although I haven't found any evidences now, I still strongly suspect that it was related to our recent memory leak.~~ **After a lot of tests, I think this issue was not related to our recent memory leak.** But it's still a bug. ### My Solution I read a lot of issues about cargo in few weeks, I didn't find any official / standard solution to fix it, and I thought they wouldn't fix it in few months. (`build.rs` can't fix it, too) So, I use a complex and dirty way to fix it, just review the code, please. #### How can I prove my modification was valid? - Please check [this example](https://github.com/yangby-cryptape/rust-features-trials/tree/99455ef/features/example-1), I have run it under Debian Buster and Windows 10. - I have build various kinds of CKB with different features and check a lot logs of them under Debian Buster. ### A Very Important Note **If any features of a rust bindings (to c library) crate was changed, please do `cargo clean` before build the project.** Some C libraries wouldn't re-compile by cargo after features changed, for example, `jemalloc-sys`. Co-authored-by: Boyu Yang <[email protected]>
|
bors is blocked by CI, see #2044 |
|
bors retry |
bors bot
added a commit
that referenced
this pull request
Apr 27, 2020
2035: fix: remove unsupport configurations in Cargo.toml r=quake,driftluo a=yangby-cryptape ### The Issue Here is a potential issue which was introduced by #1940. - Since our `rust-toolchain` is still be `1.41.0`, there will be no errors or warnings when we build the project. (This is why #1940 was merged) https://github.com/nervosnetwork/ckb/blob/22829ae85db1643480bd810150f9619250aac92e/rust-toolchain#L1 - But if you update the `rust-toolchain` to `1.42.0`, when you build the whole project (just execute `make prod`), you will get two warnings: > warning: ckb/Cargo.toml: Found `feature = ...` in `target.'cfg(...)'.dependencies`. This key is not supported for selecting dependencies and will not work as expected. Use the [features] section instead: https://doc.rust-lang.org/cargo/reference/features.html > warning: ckb/Cargo.toml: Found `feature = ...` in `target.'cfg(...)'.dependencies`. This key is not supported for selecting dependencies and will not work as expected. Use the [features] section instead: https://doc.rust-lang.org/cargo/reference/features.html ~~Although I haven't found any evidences now, I still strongly suspect that it was related to our recent memory leak.~~ **After a lot of tests, I think this issue was not related to our recent memory leak.** But it's still a bug. ### My Solution I read a lot of issues about cargo in few weeks, I didn't find any official / standard solution to fix it, and I thought they wouldn't fix it in few months. (`build.rs` can't fix it, too) So, I use a complex and dirty way to fix it, just review the code, please. #### How can I prove my modification was valid? - Please check [this example](https://github.com/yangby-cryptape/rust-features-trials/tree/99455ef/features/example-1), I have run it under Debian Buster and Windows 10. - I have build various kinds of CKB with different features and check a lot logs of them under Debian Buster. ### A Very Important Note **If any features of a rust bindings (to c library) crate was changed, please do `cargo clean` before build the project.** Some C libraries wouldn't re-compile by cargo after features changed, for example, `jemalloc-sys`. Co-authored-by: Boyu Yang <[email protected]>
|
Build failed:
|
|
bors retry |
|
Please rebase. I guess the different github actions directory in the two branches block the merge. |
|
bors r- |
|
bors r=quake,driftluo |
|
bors retry |
2 similar comments
|
bors retry |
|
bors retry |
bors bot
added a commit
that referenced
this pull request
Apr 28, 2020
2035: fix: remove unsupport configurations in Cargo.toml r=quake,driftluo a=yangby-cryptape ### The Issue Here is a potential issue which was introduced by #1940. - Since our `rust-toolchain` is still be `1.41.0`, there will be no errors or warnings when we build the project. (This is why #1940 was merged) https://github.com/nervosnetwork/ckb/blob/22829ae85db1643480bd810150f9619250aac92e/rust-toolchain#L1 - But if you update the `rust-toolchain` to `1.42.0`, when you build the whole project (just execute `make prod`), you will get two warnings: > warning: ckb/Cargo.toml: Found `feature = ...` in `target.'cfg(...)'.dependencies`. This key is not supported for selecting dependencies and will not work as expected. Use the [features] section instead: https://doc.rust-lang.org/cargo/reference/features.html > warning: ckb/Cargo.toml: Found `feature = ...` in `target.'cfg(...)'.dependencies`. This key is not supported for selecting dependencies and will not work as expected. Use the [features] section instead: https://doc.rust-lang.org/cargo/reference/features.html ~~Although I haven't found any evidences now, I still strongly suspect that it was related to our recent memory leak.~~ **After a lot of tests, I think this issue was not related to our recent memory leak.** But it's still a bug. ### My Solution I read a lot of issues about cargo in few weeks, I didn't find any official / standard solution to fix it, and I thought they wouldn't fix it in few months. (`build.rs` can't fix it, too) So, I use a complex and dirty way to fix it, just review the code, please. #### How can I prove my modification was valid? - Please check [this example](https://github.com/yangby-cryptape/rust-features-trials/tree/99455ef/features/example-1), I have run it under Debian Buster and Windows 10. - I have build various kinds of CKB with different features and check a lot logs of them under Debian Buster. ### A Very Important Note **If any features of a rust bindings (to c library) crate was changed, please do `cargo clean` before build the project.** Some C libraries wouldn't re-compile by cargo after features changed, for example, `jemalloc-sys`. Co-authored-by: Boyu Yang <[email protected]> Co-authored-by: ian <[email protected]>
|
Timed out. |
yangby-cryptape
force-pushed
the
pr/fix-cargo-toml
branch
from
e7306bb to
eddbcb8
Compare
TheWaWaR
approved these changes
Apr 29, 2020
doitian
approved these changes
Apr 29, 2020
|
bors r=doitian,TheWaWaR |
|
Build succeeded:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The Issue
Here is a potential issue which was introduced by #1940.
Since our
rust-toolchainis still be1.41.0, there will be no errors or warnings when we build the project. (This is why feat: add a feature to enable jemalloc profiling #1940 was merged)ckb/rust-toolchain
Line 1 in 22829ae
But if you update the
rust-toolchainto1.42.0, when you build the whole project (just executemake prod), you will get two warnings:Although I haven't found any evidences now, I still strongly suspect that it was related to our recent memory leak.After a lot of tests, I think this issue was not related to our recent memory leak. But it's still a bug.
My Solution
I read a lot of issues about cargo in few weeks, I didn't find any official / standard solution to fix it, and I thought they wouldn't fix it in few months. (
build.rscan't fix it, too)So, I use a complex and dirty way to fix it, just review the code, please.
How can I prove my modification was valid?
Please check this example, I have run it under Debian Buster and Windows 10.
I have build various kinds of CKB with different features and check a lot logs of them under Debian Buster.
A Very Important Note
If any features of a rust bindings (to c library) crate was changed, please do
cargo cleanbefore build the project.Some C libraries wouldn't re-compile by cargo after features changed, for example,
jemalloc-sys.