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

Check failing with "Resource not accessible by integration" #2

Open
yoshuawuyts opened this issue Oct 4, 2019 · 26 comments
Open

Check failing with "Resource not accessible by integration" #2

yoshuawuyts opened this issue Oct 4, 2019 · 26 comments

Comments

@yoshuawuyts
Copy link

In https://github.com/async-rs/async-std/pull/271/checks?check_run_id=247986116 we're getting an error about clippy failing:

2019-10-04-154921_1920x1080

I'm unsure what's causing this. I'd suspect a bad token of sorts, but I don't see how that could be possible. I saw the note in the readme, but this seems distinct also? Do you maybe know what might be going on? Thanks!

@bjorn3
Copy link

bjorn3 commented Oct 4, 2019

I saw the note in the readme, but this seems distinct also?

I think this case is covered by that note, as the pr does have a fork as source branch (sunjay:fromstream-impls)

@yoshuawuyts
Copy link
Author

@bjorn3 ahh okay, I probably misinterpreted it then. It'd be nice if this action could detect whether the check is from another repo, and fall back to regular reporting instead (rather than failing)

@svartalf
Copy link
Member

svartalf commented Oct 4, 2019

Yeah, @bjorn3 is right about the forked repo. We were investigating this problem earlier today with @Luni-4 and I had created a post about this problem at the github forums, but that is pretty much it.

I like an idea about falling back to the old plain stdout reporting, though (thanks, @yoshuawuyts!), that seems like a only reasonable option till Github will handle this problem somehow.

@Luni-4
Copy link

Luni-4 commented Oct 4, 2019

I like an idea about falling back to the old plain stdout reporting, though (thanks, @yoshuawuyts!), that seems like a only reasonable option till Github will handle this problem somehow.

Yep, I agree with this option too. In this way the Github Token isn't used at all for now.

svartalf added a commit that referenced this issue Oct 4, 2019
@svartalf
Copy link
Member

svartalf commented Oct 4, 2019

Alright, I added the stdout fallback as was suggested and published it already.
@yoshuawuyts can you restart that check to be sure that everything is okay there?

@yoshuawuyts
Copy link
Author

@svartalf thanks so much!

Though https://github.com/async-rs/async-std/pull/271/checks?check_run_id=249044165 seems like it falls back nicely now, but now also always exits with status code 0 even if clippy checks fail.

Screenshot_2019-10-05 FromStream impls for collections (and more ) by sunjay · Pull Request #271 · async-rs async-std

@svartalf
Copy link
Member

svartalf commented Oct 5, 2019

@yoshuawuyts right now it is an intended behavior, as the Action returns a non-zero exit code in case if any ICE or errors were found, but warning, note and help message types are considered as a "successful" result.
I was thinking about some kind of strict: true flag, but not sure yet about how should it look in a whole. Would love to hear your ideas about it!

@yoshuawuyts
Copy link
Author

@svartalf oh yeah, strict: true seems like a reasonable solution! -- Generally for cargo check we also like to pass RUSTFLAGS="-D warnings" to reject all warnings; if that's not part of the builder yet.

Riffing on the name slightly; if we were to use "rusty" names for this then -D warnings would probably best be translated to reject_warnings: true.

@svartalf
Copy link
Member

svartalf commented Oct 5, 2019

@yoshuawuyts reject_warnings: true sounds nice!

Here is a contrary thought, since we are already discussing it here: should not the project itself tune the clippy errors via #![deny(clippy::*)] attributes instead? Otherwise local cargo clippy and CI one are going be inconsistent (or callers must be forced to remember proper command to execute each time?)

@svartalf
Copy link
Member

svartalf commented Oct 5, 2019

Also, it is already possible to add -D warnings flag via the args input like this:

- uses: actions-rs/clippy-check@v1
  with:
    args:  --all-targets --all-features -- -D warnings

It kinda looks like a hack, since the args input is just passed to cargo clippy invocation as is, but it is a working hack :)

@yoshuawuyts
Copy link
Author

should not the project itself tune the clippy errors via #![deny(clippy::*)] attributes instead?

So in async-std we used to have this, but switched the flag to only run in CI because we wanted a slightly better cadence during development. E.g. when you're developing features you want to get things to compile, and not bother too much with the warnings much while you're working on it.

Then once you're ready to file a PR you want to have the quality enforced, and ensure that no warnings are merged into the main codebase. I think the inconsistency here is a feature, and overall leads to a much better developer experience!

It's probably possible to set this up through features and configs, but this feels like the kind of thing that having a simple toggle for would allow it to be documented in a single place, easy to use, and in turn make it more accessible to people!

@svartalf
Copy link
Member

This looks like a tracking issue for annotations: actions/toolkit#186

@CryZe
Copy link

CryZe commented Jan 21, 2020

Annotations seem to now be a thing. At least other actions now seem to be able to place annotations freely without any issues.

@CryZe
Copy link

CryZe commented Jan 21, 2020

Here's how a different Rust action is handling the annotations now (apparently via problem matchers): hecrj/setup-rust-action#14

@CryZe
Copy link

CryZe commented Jan 21, 2020

I guess that's quite different from what is being done here, but might be a decent workaround for now.

@antifuchs
Copy link

Is there a way to enable pull request annotations for PRs made from branches on non-forks? I cut quite a few branches on my repos, and there it would be really good to see the annotations in the PR conversation instead of getting them on the commit itself.

Alternatively, it would be kinda cool to be able to get these PR annotations inline on a bors build. (That does cut a branch on the source repo after all...)

bors bot added a commit to boinkor-net/governor that referenced this issue Jun 25, 2020
41: Use more github actions as checks  r=antifuchs a=antifuchs

This updates the CI integration for some cool cargo checks via the [actions-rs](https://github.com/actions-rs) repositories:

* [cargo clippy](https://github.com/actions-rs/clippy-check) for nice annotations on the commit. (No annotations on the PR, as there are token permission issues: actions-rs/clippy-check#2)
* [cargo audit](https://github.com/actions-rs/audit-check) for security checks.

Thanks to the [this blog post](https://svartalf.info/posts/2020-04-10-github-action-for-binary-crates-installation/) for pointing me to those.

Co-authored-by: Andreas Fuchs <[email protected]>
groszewn added a commit to groszewn/tensorboard that referenced this issue Jan 19, 2023
[actions-rs/toolchain](https://github.com/actions-rs/toolchain) and
[actions-rs/clippy-check](https://github.com/actions-rs/clippy-check)
have not been updated in the last two years. There are multiple GH
action warnings coming from our usage of these actions (Node.js version,
`save-state`, `set-output`), so we need to replace these.

[dtolnay/rust-toolchain](https://github.com/dtolnay/rust-toolchain) is
maintained and is roughly a drop-in replacement (modulo one of two
arguments to the action itself).

`clippy-check` seems to have been generally broken for us, since it
[can't post lints as annotations back to the PR if it comes from a
fork](actions-rs/clippy-check#2)
(which most of our PRs do). Instead, we can simply define the `cargo
clippy` command directly in our action and make sure we fail on any
raised warnings.
groszewn added a commit to groszewn/tensorboard that referenced this issue Jan 19, 2023
[actions-rs/toolchain](https://github.com/actions-rs/toolchain) and
[actions-rs/clippy-check](https://github.com/actions-rs/clippy-check)
have not been updated in the last two years. There are multiple GH
action warnings coming from our usage of these actions (Node.js version,
`save-state`, `set-output`), so we need to replace these.

[dtolnay/rust-toolchain](https://github.com/dtolnay/rust-toolchain) is
maintained and is roughly a drop-in replacement (modulo one of two
arguments to the action itself).

`clippy-check` seems to have been generally broken for us, since it
[can't post lints as annotations back to the PR if it comes from a
fork](actions-rs/clippy-check#2)
(which most of our PRs do). Instead, we can simply define the `cargo
clippy` command directly in our action and make sure we fail on any
raised warnings.
groszewn added a commit to groszewn/tensorboard that referenced this issue Jan 19, 2023
[actions-rs/toolchain](https://github.com/actions-rs/toolchain) and
[actions-rs/clippy-check](https://github.com/actions-rs/clippy-check)
have not been updated in the last two years. There are multiple GH
action warnings coming from our usage of these actions (Node.js version,
`save-state`, `set-output`), so we need to replace these.

[dtolnay/rust-toolchain](https://github.com/dtolnay/rust-toolchain) is
maintained and is roughly a drop-in replacement (modulo one of two
arguments to the action itself).

`clippy-check` seems to have been generally broken for us, since it
[can't post lints as annotations back to the PR if it comes from a
fork](actions-rs/clippy-check#2)
(which most of our PRs do). Instead, we can simply define the `cargo
clippy` command directly in our action and make sure we fail on any
raised warnings.
groszewn added a commit to groszewn/tensorboard that referenced this issue Jan 19, 2023
[actions-rs/toolchain](https://github.com/actions-rs/toolchain) and
[actions-rs/clippy-check](https://github.com/actions-rs/clippy-check)
have not been updated in the last two years. There are multiple GH
action warnings coming from our usage of these actions (Node.js version,
`save-state`, `set-output`), so we need to replace these.

[dtolnay/rust-toolchain](https://github.com/dtolnay/rust-toolchain) is
maintained and is roughly a drop-in replacement (modulo one of two
arguments to the action itself).

`clippy-check` seems to have been generally broken for us, since it
[can't post lints as annotations back to the PR if it comes from a
fork](actions-rs/clippy-check#2)
(which most of our PRs do). Instead, we can simply define the `cargo
clippy` command directly in our action and make sure we fail on any
raised warnings.
groszewn added a commit to groszewn/tensorboard that referenced this issue Jan 19, 2023
[actions-rs/toolchain](https://github.com/actions-rs/toolchain) and
[actions-rs/clippy-check](https://github.com/actions-rs/clippy-check)
have not been updated in the last two years. There are multiple GH
action warnings coming from our usage of these actions (Node.js version,
`save-state`, `set-output`), so we need to replace these.

[dtolnay/rust-toolchain](https://github.com/dtolnay/rust-toolchain) is
maintained and is roughly a drop-in replacement (modulo one of two
arguments to the action itself).

`clippy-check` seems to have been generally broken for us, since it
[can't post lints as annotations back to the PR if it comes from a
fork](actions-rs/clippy-check#2)
(which most of our PRs do). Instead, we can simply define the `cargo
clippy` command directly in our action and make sure we fail on any
raised warnings.
groszewn added a commit to tensorflow/tensorboard that referenced this issue Jan 19, 2023
[actions-rs/toolchain](https://github.com/actions-rs/toolchain) and
[actions-rs/clippy-check](https://github.com/actions-rs/clippy-check)
have not been updated in the last two years. There are multiple GH
action warnings coming from our usage of these actions (Node.js version,
`save-state`, `set-output`), so we need to replace these.

[dtolnay/rust-toolchain](https://github.com/dtolnay/rust-toolchain) is
maintained and is roughly a drop-in replacement (modulo one of two
arguments to the action itself).

`clippy-check` seems to have been generally broken for us, since it
[can't post lints as annotations back to the PR if it comes from a
fork](actions-rs/clippy-check#2) (which most
of our PRs do). Instead, we can simply define the `cargo clippy` command
directly in our action and make sure we fail on any raised warnings.

This resolves #6155.
yatbear pushed a commit to yatbear/tensorboard that referenced this issue Mar 27, 2023
[actions-rs/toolchain](https://github.com/actions-rs/toolchain) and
[actions-rs/clippy-check](https://github.com/actions-rs/clippy-check)
have not been updated in the last two years. There are multiple GH
action warnings coming from our usage of these actions (Node.js version,
`save-state`, `set-output`), so we need to replace these.

[dtolnay/rust-toolchain](https://github.com/dtolnay/rust-toolchain) is
maintained and is roughly a drop-in replacement (modulo one of two
arguments to the action itself).

`clippy-check` seems to have been generally broken for us, since it
[can't post lints as annotations back to the PR if it comes from a
fork](actions-rs/clippy-check#2) (which most
of our PRs do). Instead, we can simply define the `cargo clippy` command
directly in our action and make sure we fail on any raised warnings.

This resolves tensorflow#6155.
dimitrismistriotis added a commit to dimitrismistriotis/rdotenv that referenced this issue Apr 29, 2023
dna2github pushed a commit to dna2fork/tensorboard that referenced this issue May 1, 2023
[actions-rs/toolchain](https://github.com/actions-rs/toolchain) and
[actions-rs/clippy-check](https://github.com/actions-rs/clippy-check)
have not been updated in the last two years. There are multiple GH
action warnings coming from our usage of these actions (Node.js version,
`save-state`, `set-output`), so we need to replace these.

[dtolnay/rust-toolchain](https://github.com/dtolnay/rust-toolchain) is
maintained and is roughly a drop-in replacement (modulo one of two
arguments to the action itself).

`clippy-check` seems to have been generally broken for us, since it
[can't post lints as annotations back to the PR if it comes from a
fork](actions-rs/clippy-check#2) (which most
of our PRs do). Instead, we can simply define the `cargo clippy` command
directly in our action and make sure we fail on any raised warnings.

This resolves tensorflow#6155.
@agabani
Copy link

agabani commented Jul 7, 2023

My experience, the failure:

   {"reason":"build-finished","success":true}
      Finished dev [unoptimized + debuginfo] target(s) in 1m 12s
Clippy results: 0 ICE, 0 errors, 0 warnings, 0 notes, 0 help
Error: Resource not accessible by integration

was because the call to client.checks.create was using a token without enough permissions. The client references this doc which suggests permission checks:write needs to be provided.

Adding a permissions block to the Clippy job like the following resolved the failure for me.

  cargo-clippy:
    name: Cargo Clippy
    runs-on: ubuntu-latest
    permissions:
      checks: write
    needs:
      - cargo-build
    steps:
      - name: Checkout
        uses: actions/checkout@v3
      - name: Cargo Cache
        id: cargo-cache
        uses: actions/cache@v3
        with:
          path: |
            ~/.cargo/.crates.toml
            ~/.cargo/bin
            ./target
          key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }}
          restore-keys: |
            ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }}
      - name: Toolchain
        uses: actions-rs/toolchain@v1
        with:
          components: clippy
          profile: minimal
          override: true
          toolchain: stable
      - name: Clippy
        uses: actions-rs/clippy-check@v1
        with:
          args: --workspace -- -D warnings
          name: Clippy Results
          token: ${{ secrets.GITHUB_TOKEN }}

@AlexTMjugador
Copy link

To anyone reading this today: I'd recommend using giraffate/clippy-action instead, which offers a comparable feature set to this action, including lint annotations, and is much better maintained.

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

No branches or pull requests