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.

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

Also run hooks when only Cargo.toml and/or Cargo.lock has changed #1

Open
tyilo opened this issue May 17, 2024 · 3 comments
Open

Also run hooks when only Cargo.toml and/or Cargo.lock has changed #1

tyilo opened this issue May 17, 2024 · 3 comments

Comments

@tyilo
Copy link

tyilo commented May 17, 2024

Also see doublify#33


Both the cargo-check and clippy hooks should also be run if only Cargo.toml and/or Cargo.lock has been changed.

The following shows the problem:

$ cd /tmp
$ cargo new test-pre-commit
$ cd test-pre-commit
$ cat > .pre-commit-config.yaml
repos:
- repo: https://github.com/FeryET/pre-commit-rust
  rev: v1.1.0
  hooks:
    - id: fmt
    - id: cargo-check
^D
$ pre-commit install
$ git add .
$ git commit -m 'init'
fmt......................................................................Passed
cargo check..............................................................Passed
...
$ cargo add libc
$ cat > src/main.rs
fn main() {
    let r = unsafe { libc::rand() };
    dbg!(r);
}
^D
$ git add .
$ git commit -m '2nd'
fmt......................................................................Passed
cargo check..............................................................Passed
...
$ cargo rm libc
$ git add .
$ git commit -m '3rd'
fmt..................................................(no files to check)Skipped
cargo check..........................................(no files to check)Skipped

In the 3rd commit we have removed libc so that the code no longer compiles. However the cargo-check step is skipped, so this is not caught :(

Running pre-commit run --all-files manually results in:

fmt......................................................................Passed
cargo check..............................................................Failed
- hook id: cargo-check
- exit code: 101

    Checking test-pre-commit v0.1.0 (/tmp/test-pre-commit)
error[E0433]: failed to resolve: use of undeclared crate or module `libc`
 --> src/main.rs:2:22
  |
2 |     let r = unsafe { libc::rand() };
  |                      ^^^^ use of undeclared crate or module `libc`

For more information about this error, try `rustc --explain E0433`.
error: could not compile `test-pre-commit` (bin "test-pre-commit") due to 1 previous error
@FeryET
Copy link
Owner

FeryET commented Aug 13, 2024

I will look into this. @tyilo

@FeryET
Copy link
Owner

FeryET commented Sep 6, 2024

@tyilo I have tweaked the hook configs and added cargo.toml and cargo.lock to the cargo-check hook. As of 'v1.1.1` therer should be no issue regarding this. Please check it out and post an update here if you had any problems.

Re clippy I don't see the reason why it should be run when Cargo.toml changes. If you could elaborate I will work on that too.

@FeryET FeryET closed this as completed Sep 6, 2024
@tyilo
Copy link
Author

tyilo commented Sep 12, 2024

@tyilo I have tweaked the hook configs and added cargo.toml and cargo.lock to the cargo-check hook. As of 'v1.1.1` therer should be no issue regarding this. Please check it out and post an update here if you had any problems.

Doesn't seem to work:

$ cd /tmp
$ cargo new test-pre-commit
$ cd test-pre-commit
$ cat > .pre-commit-config.yaml
repos:
- repo: https://github.com/FeryET/pre-commit-rust
  rev: v1.1.1
  hooks:
    - id: fmt
    - id: cargo-check
^D
$ pre-commit install
$ git add .
$ git commit -m 'init'
fmt......................................................................Failed
- hook id: fmt
- exit code: 1

error: expected item, found `[`
 --> /tmp/test-pre-commit/Cargo.toml:1:1
  |
1 | [package]
  | ^ expected item
  |
  = note: for a full list of items that can appear in modules, see <https://doc.rust-lang.org/reference/items.html>

cargo check..............................................................Passed

Re clippy I don't see the reason why it should be run when Cargo.toml changes. If you could elaborate I will work on that too.

I only use the fmt and clippy hooks as clippy also shows compiler warnings/errors, so adding cargo-check to my list of hooks would be redundant.

Thus the clippy hook should also run when Cargo.toml/Cargo.lock changes.

@FeryET FeryET reopened this Sep 15, 2024
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

No branches or pull requests

2 participants