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

Problem matcher annotations are created on the wrong file #224

Closed
xt0rted opened this issue Nov 17, 2019 · 9 comments
Closed

Problem matcher annotations are created on the wrong file #224

xt0rted opened this issue Nov 17, 2019 · 9 comments
Assignees
Labels

Comments

@xt0rted
Copy link
Contributor

xt0rted commented Nov 17, 2019

I'm trying to replace one of my actions with a problem matcher since that fits in to the workflow better, but all of the annotations are being created on the workflow file. The step that's creating them (npm test) is being run with working-directory: src/website which causes the log output to be rooted in the sub folder.

This is the default output from stylelint:

scss/_qr-codes.scss
  9:2   ×  Expected indentation of 2 spaces                        indentation
 11:16  ×  Unexpected unit                                         length-zero-no-unit
 11:28  ×  Unexpected unit                                         length-zero-no-unit
 16:14  ×  Unexpected unit                                         length-zero-no-unit
 19:5   ×  Expected newline after ","                              selector-list-comma-newline-after
 19:5   ×  Expected single space after "," in a single-line list   selector-list-comma-space-after
 20:15  ×  Unexpected unit                                         length-zero-no-unit
 22:14  ×  Unexpected unit                                         length-zero-no-unit
 34:1   ×  Expected indentation of 2 spaces                        indentation
 35:3   ×  Expected indentation of 4 spaces                        indentation
 36:1   ×  Expected indentation of 2 spaces                        indentation
 37:1   ×  Unexpected missing end-of-source newline                no-missing-end-of-source-newline

I saw in #198 that there's a fromPath value that can be used to set the base path of the annotation file. Is there a way to default that to the working-directory value if the file in question couldn't be found?

@ericsciple
Copy link
Contributor

@chrispat fyi. pretty interesting feedback. i like this idea

@chrispat
Copy link
Member

chrispat commented Dec 7, 2019

We have a work item to fix the problem where annotations with no valid path will not be attributed to the workflow file. I think once that is done you won’t have the problem that you currently see where files that can’t be found are improperly attributed.

@ericsciple
Copy link
Contributor

@chrispat today unrooted paths are rooted against the github.workspace. The feedback is to root against the working-directory input instead

@chrispat
Copy link
Member

chrispat commented Dec 7, 2019

Ah, I missed that.

That make sense.

@chrispat
Copy link
Member

chrispat commented Dec 9, 2019

Any concern that we may not know the working directory in the case of many actions? For all JavaScript actions the working directory from the perspective of the runner will be the same as github.workspace, however, in the code the action author could have changed that via process.chdir() and the problem matcher code would not know.

@edhgoose
Copy link

edhgoose commented Jan 6, 2020

@ericsciple, when you say:

today unrooted paths are rooted against the github.workspace

Is that what I'm seeing when I get these notes in the annotations of GitHub Actions:

image

I've cheated in my problem matcher by adding /github/workspace/ in the regex, but that does not feel like the right way to solve this problem!

{
    "problemMatcher": [
        {
            "owner": "phpcs-csv",
            "pattern": [
                {
                    "regexp": "^\"\\/github\\/workspace\\/(.*)\",(\\d+),(\\d+),(\\w+),\"([^\"]+\",[^,+]*)",
                    "file": 1,
                    "line": 2,
                    "column": 3,
                    "severity": 4,
                    "message": 5
                }
            ]
        }
    ]
}

This is currently a private repo for PHP CodeSniffer, which outputs lines like:

File,Line,Column,Type,Message,Source,Severity,Fixable
"/github/workspace/src/our/path/to/file/ReportingHelperRepository.php",129,13,warning,"Line exceeds 120 characters; contains 133 characters",Generic.Files.LineLength.TooLong,5,0
"/github/workspace/src/our/path/to/file/ReportingHelperRepository.php",203,124,warning,"Line exceeds 120 characters; contains 124 characters",Generic.Files.LineLength.TooLong,5,0
"/github/workspace/src/our/path/to/file/ReportingHelperRepository.php",204,1,error,"Multi-line function call not indented correctly; expected 12 spaces but found 16",PEAR.Functions.FunctionCallSignature.Indent,5,1

Is there a better way, or is this issue the right place to track the problem?

@ericsciple
Copy link
Contributor

@edhgoose can you confirm the step is a container action (i.e. uses syntax) and not an ad hoc script (run syntax)? I just skimmed the source code, i think i know what the bug is. Technically actions/runner would be a better place for the issue (recently changed to public repo). I can open an issue there for the issue you described or you can, either way is fine. I'll work on a fix tomorrow.

@edhgoose
Copy link

edhgoose commented Jan 8, 2020

@ericsciple

This is in .github/workflows/phpcs:

on: [pull_request]

jobs:
    phpcs:
        runs-on: ubuntu-latest
        name: Do PHP CS Code Quality Testing
        steps:
            # To use this repository's private action, you must check out the repository
            - name: Checkout
              uses: actions/checkout@v2

            - name: Setup Matcher
              run: echo "::add-matcher::./docker/phpcs/problem-matcher.json"

            - name: PHPCS
              uses: ./docker/phpcs # Uses an action in the docker/phpcs directory

So, yes I think is the answer to your question. Maybe best if you open the issue, as you probably can describe the problem better?

@thboop
Copy link
Collaborator

thboop commented Mar 11, 2020

Should be fixed by actions/runner#269

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

No branches or pull requests

5 participants