oxsecurity / megalinter

🦙 MegaLinter analyzes 50 languages, 22 formats, 21 tooling formats, excessive copy-pastes, spelling mistakes and security issues in your repository sources with a GitHub Action, other CI tools or locally.
https://megalinter.io
GNU Affero General Public License v3.0
1.89k stars 232 forks source link

mega-linter-runner appears to use a different config for rubocop #302

Closed joe-sharp closed 2 years ago

joe-sharp commented 3 years ago

Describe the bug mega-linter-runner claims a ruby file that goes against my configuration is OK. When I push up the bad change the Github Action seems to catch the issue with no problems.

To Reproduce Steps to reproduce the behavior:

  1. Create a file called test.rb containing a single line: p 'hello world!'
  2. Run mega-linter-runner or in my case I usually do mega-linter-runner -e 'SHOW_ELAPSED_TIME=true' -e 'LINTER_RULES_PATH=https://raw.githubusercontent.com/joe-sharp/linter-configs/main' (I tried both)
  3. Optionally also verify .ruby-lint.yml by running rubocop -c .ruby-lint.yml test.rb
  4. mega-linter-runner passes the file, rubocop or Mega Linter in a Github Action will fail on the file for:
    test.rb:1:1: C: Style/FrozenStringLiteralComment: Missing frozen string literal comment.

Expected behavior Locally the same rubocop rules should apply to mega-linter-runner as they do the Github Action (assuming the same config is provided.)

Screenshots

### Processing [RUBY] files
- Using [rubocop v0.82.0] https://rubocop.org/
- Mega-Linter key: [RUBY_RUBOCOP]
- Rules config: [https://raw.githubusercontent.com/joe-sharp/linter-configs/main/.ruby-lint.yml]
[rubocop] test.rb - SUCCESS - 0 error(s)
✅ Linted [RUBY] files with [rubocop] successfully - (1.59s)
[Text Reporter] Generated TEXT report: /tmp/lint/report/linters_logs/SUCCESS-RUBY_RUBOCOP.log

and with just a local config:

### Processing [RUBY] files
- Using [rubocop v0.82.0] https://rubocop.org/
- Mega-Linter key: [RUBY_RUBOCOP]
- Rules config: [/.ruby-lint.yml]
[rubocop] test.rb - SUCCESS - 0 error(s)
✅ Linted [RUBY] files with [rubocop] successfully - (1.73s)
[Text Reporter] Generated TEXT report: /tmp/lint/report/linters_logs/SUCCESS-RUBY_RUBOCOP.log

Additional context Here is the draft PR I created to ensure my rules worked at all: https://github.com/joe-sharp/linter-configs/pull/11/checks?check_run_id=1767368673

### Processing [RUBY] files
- Using [rubocop v0.82.0] https://rubocop.org/
- Mega-Linter key: [RUBY_RUBOCOP]
- Rules config: [/github/workspace/.ruby-lint.yml]
[rubocop] test.rb - ERROR - 1 error(s)
--Error detail:
Inspecting 1 file
C

Offenses:

test.rb:1:1: C: Style/FrozenStringLiteralComment: Missing frozen string literal comment.
p 'hello world!'
^

1 file inspected, 1 offense detected

❌ Linted [RUBY] files with [rubocop]: Found 1 error(s) - (1.41s)
joe-sharp commented 3 years ago

I have had mega-linter-runner catch quite a few things with cspell and one of the markdown linters for sure. I am not sure if any other linters are affected by this issue.

nvuillam commented 3 years ago

mmmm my computer is not very nice about running Docker locally... (windows with almost full disk) I'll see what I can do :$

nvuillam commented 3 years ago

Please could you run mega-linter-runner -e 'LOG_LEVEL=DEBUG'? It may help so see more logs :)

joe-sharp commented 3 years ago
### Processing [RUBY] files
- Using [rubocop v0.82.0] https://rubocop.org/
- Mega-Linter key: [RUBY_RUBOCOP]
- Rules config: [/.ruby-lint.yml]
[rubocop] test.rb - SUCCESS - 0 error(s)
✅ Linted [RUBY] files with [rubocop] successfully - (2.36s)
Skipped post of Github Status for RUBY with rubocop
[Text Reporter] Generated TEXT report: /tmp/lint/report/linters_logs/SUCCESS-RUBY_RUBOCOP.log
Linter result: 0 CSpell: Files checked: 8, Issues found: 0 in 0 files

Popen(['git', 'diff', '--abbrev=40', '--full-index', '-M', '--raw', '-z', '--no-color'], cwd=/tmp/lint, universal_newlines=False, shell=None, istream=None)
Linter version command: ['cspell', '--version']
Linter version result: 0 4.1.3
joe-sharp commented 3 years ago

Full output: https://gist.github.com/joe-sharp/e6b10d1107a3543cd08cdf508e5fb040

nvuillam commented 3 years ago

Ok, so to summarize, the problem is that locally, rubocop uses default Mega-Linter .ruby-lint.yml, whereas in other cases (GH Action) it succeeds to use the remote https://raw.githubusercontent.com/joe-sharp/linter-configs/main/.ruby-lint.yml ?

joe-sharp commented 3 years ago

Ok, so to summarize, the problem is that locally, rubocop uses default Mega-Linter .ruby-lint.yml, whereas in other cases (GH Action) it succeeds to use the remote https://raw.githubusercontent.com/joe-sharp/linter-configs/main/.ruby-lint.yml ?

Actually no. Sorry if this wasn't clear. It appears even a local config is ignored here. In the full output I provided I left off the remote config because this particular repository is the one I use for remote configs. I have been trying to reduce the possible variables involved in this issue.

joe-sharp commented 3 years ago

Found this in the debug output:

[Filters] {'name': 'RUBY_RUBOCOP', 'filter_regex_include': None, 'filter_regex_exclude': None, 'files_sub_directory': None, 'lint_all_files': False, 'lint_all_other_linters_files': False, 'file_extensions': ['.rb'], 'file_names_regex': [], 'file_names_not_ends_with': [], 'file_contains_regex': []}
RUBY_RUBOCOP linter files after applying linter filters:
- /tmp/lint/test.rb

So it looks like it found the right file to run rubocop against.

and it looks like it is running the correct command:

Linter command: ['rubocop', '--force-exclusion', '-c', '/tmp/lint/.ruby-lint.yml', '/tmp/lint/test.rb']

Me manually running the same command:

~/Documents/repos/linter-configs 🔮❯❯❯ rubocop --force-exclusion -c .ruby-lint.yml test.rb
Inspecting 1 file
C

Offenses:

test.rb:1:1: C: Style/FrozenStringLiteralComment: Missing frozen string literal comment.
p 'hello world!'
^
nvuillam commented 3 years ago

First, do we agree that cobfig file name should be .rubocop.yml and not .ruby-config.yml ? (It's a super-linter legacy, i think it's wrong)

And what is wrong with the command line you identified ? It ignores the config file ?

(Sorry i'm a little bit lost ^^)

joe-sharp commented 3 years ago

First, do we agree that cobfig file name should be .rubocop.yml and not .ruby-config.yml ? (It's a super-linter legacy, i think it's wrong)

Yes, we definitely agree that the rubocop config file should be .rubocop.yml, although it obviously can support anything, the default is preferred. The docs confirm this: https://docs.rubocop.org/rubocop/configuration.html

Additionally, although it appears to no longer be maintained, there actually is a Ruby Lint and its config file is .ruby-lint.yml: https://gitlab.com/yorickpeterse/ruby-lint/-/blob/master/doc/configuration.md

And what is wrong with the command line you identified ? It ignores the config file ?

(Sorry i'm a little bit lost ^^)

I don't think I found anything wrong. I was just trying to rule out all the obvious things. I can hopefully dig into this more tomorrow and maybe find something useful. Any troubleshooting tips would be highly appreciated. Worst case we could potentially find a time to work on it together via screen-share. Thanks again for your time!

nvuillam commented 3 years ago

I just started a new job as CTO which is veryyyy time consuming, so if you'd like to become an active contritor to mega-linter, you'll have my full support, I can show you the architecture this weekend if you want :)

joe-sharp commented 3 years ago

I just started a new job as CTO which is veryyyy time consuming, so if you'd like to become an active contritor to mega-linter, you'll have my full support, I can show you the architecture this weekend if you want :)

That sounds time consuming indeed, félicitations on your new role! I would very much be interested in contributing to Mega-Linter, we can certainly set something up for this weekend, thank you very much! 😄

nvuillam commented 3 years ago

I worked all week-end... they don't have CI yet, so much to do... ^^ But we'll find time to onboard you on mega linter ;)

joe-sharp commented 3 years ago

No worries at all, I had a chance to dig a little further, but unfortunately don't have many more answers. I even logged into the docker image while it was running and manually ran rubocop in there. It actually flagged the issue just like it should. I think my next step is to see if it could be an inheritance problem with the config. Are there any flags that could be sent to rubocop that aren't included in the output? I don't think I noticed anything but I don't know that I have exhausted all my verbosity options yet. (I may be able to get rubocop to tell me more for example.)

joe-sharp commented 3 years ago

Found this in my debugging today. I am not sure how this is happening, but it seems the most likely cause.

[rubocop] result: 0 configuration from /tmp/lint/.ruby-lint.yml
Default configuration from /usr/lib/ruby/gems/2.7.0/gems/rubocop-0.82.0/config/default.yml
Inspecting 0 files

0 files inspected, no offenses detected
Finished in 0.11177593899628846 seconds

I'll look around for that issue. Side question, do you know a way to prevent the container from exiting after Mega-Linter is done? I haven't had luck getting a shell prompt afterwards.

joe-sharp commented 3 years ago

I don't know if I am more or less confused by this issue at this point. I still cannot figure out any reason why it decides to inspect 0 files. I did try changing to use the proper config name and this time also included a POST_COMMAND running the same command as rubocop. (Unless the debug log is lying about what command it ran. 🤷🏼‍♂️ ) Here is the relevant output, but again no real answers here:

POST_COMMANDS=[{'command': 'rubocop --force-exclusion -d -c /tmp/lint/.rubocop.yml /tmp/lint/test.rb', 'cwd': 'workspace'}]
RUBY_RUBOCOP_ARGUMENTS=-d
RUBY_RUBOCOP_CONFIG_FILE=.rubocop.yml
RUBY_RUBOCOP_RULES_PATH=/tmp/lint
...
[Filters] {'name': 'RUBY_RUBOCOP', 'filter_regex_include': None, 'filter_regex_exclude': None, 'files_sub_directory': None, 'lint_all_files': False, 'lint_all_other_linters_files': False, 'file_extensions': ['.rb'], 'file_names_regex': [], 'file_names_not_ends_with': [], 'file_contains_regex': []}
RUBY_RUBOCOP linter files after applying linter filters:
- /tmp/lint/test.rb
...
[rubocop] command: ['rubocop', '--force-exclusion', '-d', '-c', '/tmp/lint/.rubocop.yml', '/tmp/lint/test.rb']
[rubocop] CWD: /
[rubocop] result: 0 configuration from /tmp/lint/.rubocop.yml
Default configuration from /usr/lib/ruby/gems/2.7.0/gems/rubocop-0.82.0/config/default.yml
Inspecting 0 files

0 files inspected, no offenses detected
Finished in 0.11704031995031983 seconds
### Processing [RUBY] files
- Using [rubocop v0.82.0] https://rubocop.org/
- Mega-Linter key: [RUBY_RUBOCOP]
- Rules config: [/.rubocop.yml]
[rubocop] test.rb - SUCCESS - 0 error(s)
✅ Linted [RUBY] files with [rubocop] successfully - (1.52s)
Skipped post of Github Status for RUBY with rubocop
...
[Post] run: [rubocop --force-exclusion -d -c /tmp/lint/.rubocop.yml /tmp/lint/test.rb] in cwd [/tmp/lint]
[Post] [configuration from /tmp/lint/.rubocop.yml
Default configuration from /usr/lib/ruby/gems/2.7.0/gems/rubocop-0.82.0/config/default.yml
Inspecting 1 file
Scanning /tmp/lint/test.rb
C

Offenses:

test.rb:1:1: C: Style/FrozenStringLiteralComment: Missing frozen string literal comment.
p 'hello world!'
^

1 file inspected, 1 offense detected
Finished in 0.1909610760048963 seconds
]
...
joe-sharp commented 3 years ago

WAIT, I think I may have found the culprit!:

[Post] run: [rubocop --force-exclusion -d -c /tmp/lint/.rubocop.yml /tmp/lint/test.rb] in cwd [/]
[Post] [configuration from /tmp/lint/.rubocop.yml
Default configuration from /usr/lib/ruby/gems/2.7.0/gems/rubocop-0.82.0/config/default.yml
Inspecting 0 files

0 files inspected, no offenses detected
Finished in 0.07821669999975711 seconds
] configuration from /tmp/lint/.rubocop.yml

I noticed that the current working directory was root during rubocop so I set my POST_COMMAND to the same and it skips the file. I will keep digging to see if I can find out why Rubocop fails on this...

joe-sharp commented 3 years ago

This may have been fixed on Rubocop 0.83.0: https://github.com/rubocop-hq/rubocop/commit/75542a8d8dc201fdd2ec7002bf85636e27ab9116

But it also might still have this behavior due to the default config:

 Exclude:
    - 'node_modules/**/*'
    - 'tmp/**/*'
...

EDIT: I was able to confirm the exclude in the default config causes the issue along with the working directory used by mega-linter-runner. Using the following in my .mega-linter.yml works around the issue:

PRE_COMMANDS:
  - command: sed -i "s/- 'tmp\/\*\*\/\*'//g" $(rubocop -d --force-default-config -v | grep Default | awk '{print $4}')
    cwd: "root"

If you upgrade rubocop I would be happy to try this again but it may be best to use a different working directory rather than '/tmp/lint'. Side note, I tried to make the PRE-COMMANDS run a mkdir -p /github/workspace and set the DEFAULT_WORKSPACE to it, but it didn't seem to override /tmp/lint

nvuillam commented 3 years ago

Great investigation :) I think that to avoid such issue that may happen with other linters, we should replace /tmp/lint everywhere

I'd like to avoid to hardcode it in python as much as possible, could u:

joe-sharp commented 3 years ago

Thanks! 😅 I definitely agree on the strategy, I will start working on a fix hopefully some time today, but for sure this week!

github-actions[bot] commented 3 years ago

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.

joe-sharp commented 3 years ago

Sorry this has gone stale, I have a PR started barely but we got hit with some crazy weather that put me offline mostly for a week and got very behind at work. Hoping to be caught up next week.

nvuillam commented 3 years ago

Do the best you can, no problem :)

joe-sharp commented 3 years ago

Got A PR started that is linked above. Looks like I broke some things that I need to look into after work but I made a fair bit of progress already.

github-actions[bot] commented 3 years ago

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.

Yann-J commented 2 years ago

Hello here @nvuillam @joe-sharp

I've witnessed the exact same issue using rubocop today: all my files are skipped when running locally (docker run -it -v $(pwd):/tmp/lint megalinter/megalinter-ruby:v5), presumably because it's picking up the default Exclude rule for /tmp/**. No idea why that doesn't happen when run as a Github action.

I've noticed that manually running the same rubocop command, but removing the --force-exclusion from inside the container works well enough for me, but I could not figure out how to instruct Mega-linter to remove that flag...

Has there been any progress on a possible resolution / workaround?

nvuillam commented 2 years ago

@Yann-J I removed --force-exclusion, please could you check again with docker image megalinter/megalinter:test-nvuillam-rubocop and confirm it works ok with updated descriptor config ? :)

Yann-J commented 2 years ago

Yes @nvuillam , I confirm this works in my CI now!

nvuillam commented 2 years ago

@Yann-J thanks for your feedback :)

Merged in main, so available in megalinter/megalinter:beta in 30mn, then in the next official MegaLinter release :)