search

jakebailey / pyright-action

GitHub Action for pyright
MIT License
76 stars 13 forks source link

fix: use relative paths to working directory in annotations #116

Closed mskrajnowski closed 5 months ago

mskrajnowski commented 5 months ago

pyright outputs absolute paths in diagnostics and that's what is currently provided when creating GitHub annotations.

When a working directory is provided, which is outside of the GitHub workspace path, annotations would show up in the workflow summary, but wouldn't appear in the diff.

This PR changes the absolute paths in annotations to relative to the specified working directory to fix this problem.

Absolute paths will still be reported when working-directory is omitted. I tried to change that, but some tests failed and I didn't want to make broad changes.

jakebailey commented 5 months ago

Can you please undo the formatting changes? Not sure why the code was entirely reformatted.

codecov[bot] commented 5 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.00%. Comparing base (3bdde3b) to head (f481bae).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #116 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 4 4 Lines 664 666 +2 Branches 137 166 +29 ========================================= + Hits 664 666 +2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

jakebailey commented 5 months ago

There are still a lot of random formatting changes which obscure the fix.

mskrajnowski commented 5 months ago

Yes, sorry, had prettier enabled by default in VSCode, should be good now

jakebailey commented 5 months ago

Thanks.

It's not clear to me how this is correct; the working directory option is pyright-action specific, so how does GitHub understand anything's about the annotations being relative to any random dir like this? I could be in a deeply nested dir and emit an annotation for main.py and this PR seemingly makes it annotate main.py versus /path/to/main.py, which doesn't seem specific enough... There could be another file with that name somewhere else.

mskrajnowski commented 5 months ago

In my case I'm running pyright-action from within a container that already has all the dependencies and code in it, e.g. in /opt/example and I'm using working-directory to point pyright-action there so it doesn't use the default workspace path.

I agree that this wouldn't work for running pyright in nested directories though. Would you consider adding a separate input to change the project root path, which would be used for relative paths instead?

mskrajnowski commented 5 months ago

Also, thanks for the very quick response

jakebailey commented 5 months ago

Can you provide a link to a workflow where this isn't working today?

Would you consider adding a separate input to change the project root path, which would be used for relative paths instead?

I'm not 100% sure what you mean by this, but it sounds like you're asking for a flag which just does that this PR does conditionally, but it's not clear to me how this relative path stuff even works period.

I can imagine needing to do things relative to the original working directory for some weird reasons, but no matter what that sure sounds like a workaround for a GitHub bug or something...

mskrajnowski commented 5 months ago

Will create a small example repo and get back to you. I'll switch the PR to draft for now.

mskrajnowski commented 5 months ago

Added demo PRs:

Let me know if you can access workflow logs. In both you can see the errors in the workflow summary, but only the second one has the annotations in the diff.

Basically, GitHub has no way of knowing that /opt/demo/demo.py is ./demo.py in the repository.

mskrajnowski commented 5 months ago

I assume GitHub actions has some magic that converts absolute paths within the workspace to relative paths, which is why it works now.

mskrajnowski commented 5 months ago

Alternative solution, which adds workspace-directory input:

jakebailey commented 5 months ago

Ah, so this is actually to do with the fact that this workflow is running within a docker container?

I can see how this wouldn't be working right, yeah. I guess GitHub's own annotation examples don't indicate that the paths can be absolute. There may be a more general fix that needs to happen here; working-directory is an option specific to pyright-action, but maybe I should actually not do this at all and instead always make paths relative to $GITHUB_WORKSPACE...

jakebailey commented 5 months ago

The problem I can see is that it's possible to say, actions/checkout to a directory. What kind of path is GitHub expecting there?

jakebailey commented 5 months ago

Is there any chance you could rerun the "broken" workflow again with debug logging enabled? There should be a button for it at the top right of https://github.com/codequest-eu/pyright-action/actions/runs/9286266122/job/25552623528?pr=3

mskrajnowski commented 5 months ago

Is there any chance you could rerun the "broken" workflow again with debug logging enabled?

done https://github.com/codequest-eu/pyright-action/actions/runs/9286266122/job/26018045227

maybe I should actually not do this at all and instead always make paths relative to $GITHUB_WORKSPACE...

The problem I can see is that it's possible to say, actions/checkout to a directory. What kind of path is GitHub expecting there?

Looking at actions/checkout docs they only support relative paths from the $GITHUB_WORKSPACE.

actions/checkout itself is currently quite broken when using containers and a non-root user (e.g. https://github.com/actions/checkout/issues/1487, https://github.com/actions/checkout/issues/1575), which is why I'm putting everything that's needed for the workflow in the image and ignoring the $GITHUB_WORKSPACE.

However, if you decide that this is not a supported use case then that's perfectly fine.

In the meantime, I'll try the suggestion from https://github.com/actions/checkout/issues/841#issuecomment-1220502440 to use the root user in the container, which might fix actions/checkout and then I should be able to run pyright-action from within the workspace.

mskrajnowski commented 5 months ago

Ran my workflow as root with actions/checkout in a container and everything seems to be working fine without passing a working-directory to pyright-action. Annotations in the diff are present.

I don't like running containers as root, but in this context it might be good enough. Closing the PR, since it might overcomplicate this action for a niche use case.

jakebailey commented 5 months ago

done codequest-eu/pyright-action/actions/runs/9286266122/job/26018045227

Thanks for running that; annoyingly, they don't seem to log anything about how they're handling the annotations.

Looking at actions/checkout docs they only support relative paths from the $GITHUB_WORKSPACE.

I was mainly meaning that in context of the annotations, but it's definitely telling that they seem to want you to keep your code in GITHUB_WORKSPACE, yeah.

I do think there's a real problem here, though; definitely need to figure this out, but will definitely take me attempting to reverse engineer it a bit...