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

Applying fixes does not work #481

Closed
vkucera opened this issue May 23, 2021 · 20 comments
Closed

Applying fixes does not work #481

vkucera opened this issue May 23, 2021 · 20 comments
Labels
bug Something isn't working O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity

Comments

@vkucera
Copy link
Contributor

vkucera commented May 23, 2021

Describe the bug

The only way of applying fixes that seems to work is the commit mode on pull request events from one branch to another in the same repository. All other cases fail for various reasons.

To Reproduce

  • Create a new branch.
  • Commit changes that require fixes.
  • Push to the remote branch.
  • Create a pull request to the main branch.
  • See the log.
  • Repeat for all combinations of APPLY_FIXES_EVENT and APPLY_FIXES_MODE.

Expected behaviour
Fixes are applied as commit or pull request (according to APPLY_FIXES_MODE) for push or pull request events (according to APPLY_FIXES_EVENT).

Additional context

  • Applying of fixes on push events is blocked by the condition github.event.pull_request.head.repo.full_name == github.repository which can never be satisfied because github.event.pull_request exists only for pull request event.
  • Applying of fixes in the pull request mode does not work because the repository is checked out in a detached head and the peter-evans/create-pull-request action complains with:
fatal: ref HEAD is not a symbolic ref
Error: When the repository is checked out on a commit instead of a branch, the 'base' input must be supplied.

I tried to run the workflow with the branch HEAD checked out using: ref: ${{ github.event.pull_request.head.sha }} in the checkout action but that makes the Mega-Linter runner fail with:

Warning: Unable to pull current branch <SHA>
cmdline: git diff --name-only master...<SHA>
stderr: 'fatal: Invalid symmetric difference expression master...<SHA>

where <SHA> is a commit SHA which is different from the branch HEAD at which the repository was initially checked out.
(Could it be the commit with the fixes?)

How about pull requests between repositories?
Besides fixing these issues it would also be very useful if the fixes could be applied in pull request events from a fork to an upstream repository, since that is the way pull requests are mostly used.

I don't know if it is related, but for completeness
For tests of pull requests from a fork to an upstream repository, this warning always appears:

Listing updated files in [/github/workspace] using git diff, then filter with:
Warning: Unable to pull current branch <SHA>
Git diff :

where <SHA> corresponds to the merge commit created by the checkout action.
This is then followed by this error for each linter:

[GitHub Status Reporter] Error posting Status for <CATEGORY>with <linter>: 403
GitHub API response: {"message":"Resource not accessible by integration","documentation_url":"https://docs.github.com/rest/reference/repos#create-a-commit-status"}
@vkucera vkucera added the bug Something isn't working label May 23, 2021
@nvuillam
Copy link
Member

nvuillam commented May 23, 2021

Thanks for this feedback, I think you're the first to check other cases than Commit within the same repo !

If you have proposition of fixes, please be my guest to make a PR
Else i'll try to check all that one of these week-ends...

@vkucera
Copy link
Contributor Author

vkucera commented May 24, 2021

Allowing fixes for push events can be done simply by replacing
github.event.pull_request.head.repo.full_name == github.repository
with
(github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository)
That one is quite straightforward. But if you prefer, I can make a PR.

I don't have any further suggestions on the other errors, unfortunately.

@nvuillam
Copy link
Member

@vkucera please make a PR, that will make you a contributor :)

@vkucera
Copy link
Contributor Author

vkucera commented May 24, 2021

I followed the instructions at https://nvuillam.github.io/mega-linter/contributing/
but running ./build.sh ends with:

Formatting markdown tables...
npx: installed 17 in 1.736s
Unexpected token ;

Do you have an idea where this error comes from?

@nvuillam
Copy link
Member

@nvuillam
Copy link
Member

and do you have latest node.js version ?

@vkucera
Copy link
Contributor Author

vkucera commented May 24, 2021

Did you install https://www.npmjs.com/package/markdown-table-formatter ?

No, I didn't. The instructions mention installation of markdown-table-formatter only if the previous steps didn't work. In my case they did work, so I had no reason to do so.

and do you have latest node.js version ?

No I don't. My version is v10.19.0 (available in Ubuntu 20.04). node.js is not mentioned anywhere in the instructions. If only specific versions are required, it should be mentioned.

Btw, the line pip install -r requirements.dev. seems to be missing the txt extension.

@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity.
It will be closed in 14 days if no further activity occurs.
Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

@github-actions github-actions bot added the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Jun 24, 2021
@vkucera
Copy link
Contributor Author

vkucera commented Jun 24, 2021

not stale

@github-actions github-actions bot removed the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Jun 24, 2021
@nvuillam
Copy link
Member

Agreed... sorry my rent-paying job is very time consuming these days !

@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity.
It will be closed in 14 days if no further activity occurs.
Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

@github-actions github-actions bot added the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Jul 29, 2021
@dgokcin
Copy link

dgokcin commented Aug 10, 2021

Hello

What I am trying to do is analyzing the newly changed files against the .editorconfig file at the root of my repository and if there are any broken properties, fixing them automatically(either via a commit or a pr, doesn't matter).

When I create a feature branch, create an issue and create a PR, the editorconfig scan gives an error as expected and the steps about creating a pr and fixing things are skipped. I am adding screenshots and my workflow to describe the issue ina better way. Am I missing anything?

Looking forward for your help @nvuillam

Thanks

Screen Shot 2021-08-10 at 16 23 45

Screen Shot 2021-08-10 at 16 24 02

---
# Mega-Linter GitHub Action configuration file
# More info at https://nvuillam.github.io/mega-linter
name: Mega-Linter

on:
  # Trigger mega-linter at every push. Action will also be visible from Pull Requests to master
  #push: # Comment this line to trigger action only on pull-requests (not recommended if you don't pay for GH Actions)
  pull_request:
    branches: [master, develop]

env: # Comment env block if you do not want to apply fixes
  # Apply linter fixes configuration
  APPLY_FIXES: all # When active, APPLY_FIXES must also be defined as environment variable (in github/workflows/mega-linter.yml or other CI tool)
  APPLY_FIXES_EVENT: pull_request # Decide which event triggers application of fixes in a commit or a PR (pull_request, push, all)
  APPLY_FIXES_MODE: commit # If APPLY_FIXES is used, defines if the fixes are directly committed (commit) or posted in a PR (pull_request)

jobs:
  # Cancel duplicate jobs: https://github.com/fkirc/skip-duplicate-actions#option-3-cancellation-only
  cancel_duplicates:
    name: Cancel duplicate jobs
    runs-on: ubuntu-latest
    steps:
      - uses: fkirc/skip-duplicate-actions@master
        with:
          github_token: ${{ secrets.PAT || secrets.GITHUB_TOKEN }}

  build:
    name: Mega-Linter
    runs-on: ubuntu-latest
    steps:
      # Git Checkout
      - name: Checkout Code
        uses: actions/checkout@v2
        with:
          token: ${{ secrets.PAT || secrets.GITHUB_TOKEN }}
          fetch-depth: 0

      # Mega-Linter
      - name: Mega-Linter
        id: ml
        # You can override Mega-Linter flavor used to have faster performances
        # More info at https://nvuillam.github.io/mega-linter/flavors/
        uses: nvuillam/mega-linter@v4
        env:
          # All available variables are described in documentation
          # https://nvuillam.github.io/mega-linter/configuration/
          VALIDATE_ALL_CODEBASE: false
          DEFAULT_BRANCH: develop
          #VALIDATE_ALL_CODEBASE: ${{ github.event_name == 'push' && github.ref == 'refs/heads/master' }} # Validates all source when push on master, else just the git diff with master. Override with true if you always want to lint all sources
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
          # ADD YOUR CUSTOM ENV VARIABLES HERE OR DEFINE THEM IN A FILE .mega-linter.yml AT THE ROOT OF YOUR REPOSITORY
          # DISABLE: COPYPASTE,SPELL # Uncomment to disable copy-paste and spell checks

      # Upload Mega-Linter artifacts
      - name: Archive production artifacts
        if: ${{ success() }} || ${{ failure() }}
        uses: actions/upload-artifact@v2
        with:
          name: Mega-Linter reports
          path: |
            report
            mega-linter.log

      # Create pull request if applicable (for now works only on PR from same repository, not from forks)
      - name: Create Pull Request with applied fixes
        id: cpr
        if: steps.ml.outputs.has_updated_sources == 1 && (env.APPLY_FIXES_EVENT == 'all' || env.APPLY_FIXES_EVENT == github.event_name) && env.APPLY_FIXES_MODE == 'pull_request' && (github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository)
        uses: peter-evans/create-pull-request@v3
        with:
          token: ${{ secrets.PAT || secrets.GITHUB_TOKEN }}
          commit-message: "[Mega-Linter] Apply linters automatic fixes"
          title: "[Mega-Linter] Apply linters automatic fixes"
          labels: bot
      - name: Create PR output
        if: steps.ml.outputs.has_updated_sources == 1 && (env.APPLY_FIXES_EVENT == 'all' || env.APPLY_FIXES_EVENT == github.event_name) && env.APPLY_FIXES_MODE == 'pull_request' && (github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository)
        run: |
          echo "Pull Request Number - ${{ steps.cpr.outputs.pull-request-number }}"
          echo "Pull Request URL - ${{ steps.cpr.outputs.pull-request-url }}"

      # Push new commit if applicable (for now works only on PR from same repository, not from forks)
      - name: Prepare commit
        if: steps.ml.outputs.has_updated_sources == 1 && (env.APPLY_FIXES_EVENT == 'all' || env.APPLY_FIXES_EVENT == github.event_name) && env.APPLY_FIXES_MODE == 'commit' && github.ref != 'refs/heads/master' && (github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository)
        run: sudo chown -Rc $UID .git/
      - name: Commit and push applied linter fixes
        if: steps.ml.outputs.has_updated_sources == 1 && (env.APPLY_FIXES_EVENT == 'all' || env.APPLY_FIXES_EVENT == github.event_name) && env.APPLY_FIXES_MODE == 'commit' && github.ref != 'refs/heads/master' && (github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository)
        uses: stefanzweifel/git-auto-commit-action@v4
        with:
          branch: ${{ github.event.pull_request.head.ref || github.head_ref || github.ref }}
          commit_message: "[Mega-Linter] Apply linters fixes"

@github-actions github-actions bot removed the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Aug 10, 2021
@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity.
It will be closed in 14 days if no further activity occurs.
Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

@github-actions github-actions bot added the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Sep 10, 2021
@nvuillam
Copy link
Member

@dgokcin to apply fixes with a new PR or commit, there must be no blocking lint error on the Mega-Linter step

If you want to apply fixes while there are still not auto-fixable lint errors, you can copy the content of "updated_files" in report artifact

@github-actions github-actions bot removed the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Sep 10, 2021
@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity.
It will be closed in 14 days if no further activity occurs.
Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

@github-actions github-actions bot added the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Oct 11, 2021
joe-sharp added a commit to joe-sharp/linter-configs that referenced this issue Oct 11, 2021
@vkucera
Copy link
Contributor Author

vkucera commented Oct 11, 2021

Not stale

@github-actions github-actions bot removed the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Oct 12, 2021
@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity.
It will be closed in 14 days if no further activity occurs.
Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

@github-actions github-actions bot added the O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity label Nov 12, 2021
@nvuillam nvuillam reopened this Nov 27, 2021
@vkucera
Copy link
Contributor Author

vkucera commented Apr 14, 2022

Hi @nvuillam , I am wondering whether there has been any progress on this. Especially the option to apply formatting as a PR would be very much appreciated.

@nvuillam
Copy link
Member

Not yet, sorry

@Kurt-von-Laven
Copy link
Collaborator

See #2125.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity
Projects
None yet
Development

No branches or pull requests

4 participants