Support Linting Passed Files

[Kurt-von-Laven]

Is your feature request related to a problem? Please describe.
Mega-Linter has the potential to be the biggest baddest pre-commit hook out there, but the main drawback to running it this way is that people generally expect excellent performance from pre-commit hooks.

Describe the solution you'd like
Most pre-commit hooks only run on staged files, which, by default, pre-commit passes as positional command-line arguments.

Describe alternatives you've considered
Presently, we run Mega-Linter on all files for each commit, which is quite slow. We also run the pre-commit GitHub Action since pre-commit.ci does not plan to support Docker for quite some time. In CI, it is desirable to run Mega-Linter on all files, so one could only run Mega-Linter in CI or perhaps run Mega-Linter asynchronously at commit time.

Additional context
Supporting running on a given subset of files would also enable Mega-Linter to run efficiently via lint-staged, and would make it easy for mega-linter-runner to also support running on a subset of files so a developer could type out much faster-running ad-hoc commands.

[Kurt-von-Laven]

Copying @nvuillam's reply when this was initially proposed on #569:

At the moment Mega-Linter collects files, we could do something like:

if MEGALINTER_FILES_TO_LINT env var is set
files_to_lint = MEGALINTER_FILES_TO_LINT
else
usual way

Could be adapted within an --files argument for mega-linter-runner too

[github-actions]

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.

[manishjagtap]

This is exactly what I was looking for. Should be able to lint only a given subset of files - especially used in pre-commit checks. This will be a huge value addition to CI.

[Kurt-von-Laven]

@manishjagtap, this is pretty high on my list. For the best approach we have found that works today, please have a look at #569. The incremental mode lints all of the files that have been changed relative to your default branch, which in practice often offers performance that is almost as good as linting precisely the files modified in the current commit.

[github-actions]

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.

[nvuillam]

not stale ^^

[github-actions]

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]

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]

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]

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]

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]

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.

[nvuillam]

@Kurt-von-Laven @manishjagtap PR on the way :)

[nvuillam]

All good, with test classes :)
If you wanted to do stuff with pre-commit, now it's possible with MegaLinter beta ^^

[llaville]

@nvuillam You should probably note in documentation that files listed is related to workspace path root (corresponding to code 84a2673#diff-acbf409a3e643e33b8a5618e27fc263237871cbe9e66e58e2b1ba27a01eabe0aR519)

[Kurt-von-Laven]

I guess we could have a pre-commit hook that calls a wrapper script that sets the new environment variable? Pre-commit and lint-staged expect to be able to pass the files directly to the command though.

[nvuillam]

@llaville

@nvuillam You should probably note in documentation that files listed is related to workspace path root (corresponding to code 84a2673#diff-acbf409a3e643e33b8a5618e27fc263237871cbe9e66e58e2b1ba27a01eabe0aR519)

I can do that yes :) Do you see cases where users could want to send absolute paths (with/tmp/lint) ?

I guess we could have a pre-commit hook that calls a wrapper script that sets the new environment variable? Pre-commit and lint-staged expect to be able to pass the files directly to the command though.

I still don't use MegaLinter locally but as it seems that lots of users want to do that... I let you handle the topic, as I don't know what I'm talking about :)
If it can help, I also added a variable SKIP_CLI_LINT_MODES that you can use as SKIP_CLI_LINT_MODES: project if you don't want to run linters like checkov or trivy everytime you update a file :)

[llaville]

@nvuillam

I can do that yes :) Do you see cases where users could want to send absolute paths (with/tmp/lint) ?

There is no reason to accept absolute path (to be compatible with locally run and CI run)

Sorry but not yet enough free time to handle any new topic right now -)

[nvuillam]

@llaville no problem, it was more adressed to @Kurt-von-Laven, the pre-commit expert of MegaLinter ^^

[Kurt-von-Laven]

He he, I have given this a bit more thought, and there are some drawbacks I see to passing filenames via the config option. Every pre-commit and lint-staged hook I am aware of (as well as virtually all CLI tools) that operate on files accept them directly as command line arguments. I expect many users will take this for granted (which IIRC has already occurred) and then be surprised that MegaLinter is different. Furthermore, while we can offer some pre-commit hooks as a convenience, I am skeptical that many users will be willing to write a wrapper script in order to implement a custom pre-commit hook or run MegaLinter via lint-staged. Conveniently, all of our CI pipelines were broken by an unrelated upstream issue today, so if you agree, I actually have time to change from a config option to directly accepting filenames on the command line. Alternatively, I suppose we could do both, but I am not sure I see a benefit to the added complexity yet.

[manishjagtap]

+1 for Kurt. IMO, file name list as a CLI arg would be very CI friendly. With this flag supported, we can skip the config option as it doesn't add too much value. Regards, Manish

…

On Fri, 12 Aug, 2022, 7:49 am Kurt von Laven, ***@***.***> wrote: He he, I have given this a bit more thought, and there are some drawbacks I see to passing filenames via the config option. Every pre-commit and lint-staged hook I am aware of (as well as virtually all CLI tools) that operate on files accept them directly as command line arguments. I expect many users will take this for granted (which IIRC has already occurred) and then be surprised that MegaLinter is different. Furthermore, while we can offer some pre-commit hooks as a convenience, I am skeptical that many users will be willing to write a wrapper script in order to implement a custom pre-commit hook or run MegaLinter via lint-staged. Conveniently, all of our CI pipelines were broken by an unrelated upstream issue today, so if you agree, I actually have time to change from a config option to directly accepting filenames on the command line. Alternatively, I suppose we could do both, but I am not sure I see a benefit to the added complexity yet. — Reply to this email directly, view it on GitHub <#808 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AJW2FDD4NZZDCYD4GTKUW7TVYWYCXANCNFSM5FK2KADQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[Kurt-von-Laven]

I guess the argument for having both could be if all of our other command line arguments already have a corresponding config option, but I trust @nvuillam to know that better than I.

[nvuillam]

So basically, you'd like to be able to send list of files as positional arguments for mega-linter-runner ? :)

[nvuillam]

Dev done, PR on the way :)

It will be callable like: mega-linter-runner --flavor python --release beta --filesonly megalinter/config.py megalinter/flavor_factory.py megalinter/MegaLinter.py

[nvuillam]

And also mega-linter-runner --flavor python --release beta megalinter/config.py megalinter/flavor_factory.py megalinter/MegaLinter.py if you also run want the project linters