Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

sdk-c requires separate cargo clippy run on CI #5503

Closed
TristanDebrunner opened this issue Aug 12, 2019 · 6 comments · Fixed by #10107
Closed

sdk-c requires separate cargo clippy run on CI #5503

TristanDebrunner opened this issue Aug 12, 2019 · 6 comments · Fixed by #10107
Milestone

Comments

@TristanDebrunner
Copy link
Contributor

Problem

clippy gets stuck if it is run in CI with --all, but succeeds if sdk-c is excluded. As a workaround, the sdk-c check is run separately.

Proposed Solution

root cause the problem and fix

@TristanDebrunner TristanDebrunner added this to the The Future! milestone Aug 12, 2019
@svenski123
Copy link
Contributor

From ci/test-checks.sh:

# Clippy gets stuck for unknown reasons if sdk-c is included in the build, so check it separately.
# See https://github.com/solana-labs/solana/issues/5503
_ cargo +"$rust_stable" clippy --version
_ cargo +"$rust_stable" clippy --all --exclude solana-sdk-c -- --deny=warnings
_ cargo +"$rust_stable" clippy --manifest-path sdk-c/Cargo.toml -- --deny=warnings

Comments:

  1. cargo build --help states --all is a deprecated alias for --workspace
  2. running cargo clippy --workspace appear to work fine (i.e. completes and does not get stuck)

FYI - this is on Centos 7.6 box with:

$ cargo --version; cargo clippy --version
cargo 1.43.0 (2cbe9048e 2020-05-03)
clippy 0.0.212 (204bb9b 2020-03-17)

Is this still an issue? If so, how can it be reproduced?

@ryoqun
Copy link
Contributor

ryoqun commented May 18, 2020

@svenski123 Heh, thanks for spotting this rather dated issue! I even didn't know this issue existed.

If so, how can it be reproduced?

Feel free to open an PR of editing ci/test-checks.sh and I'll make it run on our CI. And see if the problem still exists. :)

EDIT: Also, I think you found this in the pursuit of #10030 (comment). We rather want to enable --tests (or equivalent) sooner or later otherwise we'll introduce more clippy errors because CI won't complain, including me ;)

@CriesofCarrots
Copy link
Contributor

We actually might want to consider removing sdk-c at this point. The downstream use-cases we expected didn't materialize, and we know how to get it back if we want it...

@mvines
Copy link
Contributor

mvines commented May 18, 2020

💯, nuke it

@ryoqun
Copy link
Contributor

ryoqun commented May 18, 2020

Heh, another spring cleaning-up time after #9992, then? @jackcmay wdyt? Also, is that removal good-for-first one? :)

@CriesofCarrots
Copy link
Contributor

Super simple; I just went ahead with it

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants