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

Removes prefixing of ignored files with workspace path #2725

Closed
wants to merge 7 commits into from

Conversation

iisisrael
Copy link
Contributor

@iisisrael iisisrael commented Jun 6, 2023

Ignored files fails to filter files when a workspace path is provided. This checks the file with the workspace path prepended against the ignored files list, which is compiled with the workspace path included.

Readiness Checklist

Author/Contributor

  • Add entry to the CHANGELOG listing the change and linking to the corresponding issue (if appropriate)
  • [~] If documentation is needed for this change, has that been included in this pull request

Reviewing Maintainer

  • Label as breaking if this is a large fundamental change
  • Label as either automation, bug, documentation, enhancement, infrastructure, or performance

@iisisrael iisisrael changed the title Adds workspace pat to ignored files check Adds workspace path to ignored files check Jun 6, 2023
@iisisrael iisisrael temporarily deployed to dev June 6, 2023 17:56 — with GitHub Actions Inactive
@iisisrael iisisrael temporarily deployed to dev June 6, 2023 17:56 — with GitHub Actions Inactive
@iisisrael iisisrael marked this pull request as ready for review June 6, 2023 20:47
@iisisrael iisisrael requested a review from nvuillam as a code owner June 6, 2023 20:47
@iisisrael iisisrael temporarily deployed to dev June 7, 2023 04:21 — with GitHub Actions Inactive
@iisisrael iisisrael temporarily deployed to dev June 7, 2023 04:21 — with GitHub Actions Inactive
@iisisrael iisisrael temporarily deployed to dev June 7, 2023 14:35 — with GitHub Actions Inactive
@iisisrael iisisrael temporarily deployed to dev June 7, 2023 14:35 — with GitHub Actions Inactive
@nvuillam
Copy link
Member

nvuillam commented Jun 7, 2023

@iisisrael please could you try to make the opposite, meaning sending ignored files without workspace ? :)

Probably by changing there ->
https://github.com/oxsecurity/megalinter/blob/3d230f3e3b7a44eef594e706f929599d1f89085a/megalinter/MegaLinter.py#LL799C1-L800C79

@iisisrael
Copy link
Contributor Author

@nvuillam I tried creating a unit test around the list_git_ignored_files function in MegaLinter.py, but couldn't get very far.

@iisisrael iisisrael temporarily deployed to dev June 18, 2023 08:21 — with GitHub Actions Inactive
@github-actions
Copy link
Contributor

This pull request 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 pull request should stay open, please remove the O: stale 🤖 label or comment on the pull request.

@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 16, 2023
@iisisrael
Copy link
Contributor Author

I just pulled the latest, and this is still an issue. The change fixes it, and I'm getting back to looking at tests, so keeping this PR alive.

@iisisrael
Copy link
Contributor Author

Rebased from main.

@iisisrael iisisrael temporarily deployed to dev July 28, 2023 21:11 — with GitHub Actions Inactive
@iisisrael iisisrael temporarily deployed to dev July 28, 2023 21:11 — with GitHub Actions Inactive
@iisisrael
Copy link
Contributor Author

This change was tested manually following these steps.

mega-linter.yml in a PHP project:

# Configuration file for MegaLinter
# See all available variables at https://megalinter.io/configuration/ and in linters documentation

APPLY_FIXES: none # all, none, or list of linter keys
# ENABLE: # If you use ENABLE variable, all other languages/formats/tooling-formats will be disabled by default
#   - PHP
ENABLE_LINTERS: # If you use ENABLE_LINTERS variable, all other linters will be disabled by default
  - PHP_PHPSTAN
  - PHP_PHPCS
  # - PHP_PSALM
  # - PHP_PHPLINT
PHP_PHPCS_ARGUMENTS:
  - -s
DISABLE_ERRORS_LINTERS: # List of linters that will not block CI to pass if they detect errors
  - PHP_PHPSTAN
  - PHP_PHPCS
PRE_COMMANDS:
  - command: apk add composer
    cwd: "root"
  - command: composer install --prefer-dist --no-ansi --no-interaction --no-progress --no-scripts --ignore-platform-req=ext-sockets --ignore-platform-req=ext-intl --ignore-platform-req=ext-fileinfo --ignore-platform-req=ext-sodium --ignore-platform-req=ext-posix --ignore-platform-req=ext-gd --ignore-platform-req=ext-xml
    cwd: "workspace"
  - command: "[ ! -f phpcs.xml ] && cp phpcs.xml.dist phpcs.xml"
    cwd: "workspace"
PARALLEL: false

Prior to the change:

MegaLinter now collects the files to analyse
Listing all files in directory [/tmp/lint], then filter with:
- File extensions: .php
- Excluding .gitignored files [41277]: /tmp/lint/.env, /tmp/lint/.php-cs-fixer.cache, /tmp/lint/.phpstorm.meta.php, /tmp/lint/.phpunit.result.cache, /tmp/lint/_ide_helper.php, /tmp/lint/bootstrap/cache/packages.php, /tmp/lint/bootstrap/cache/services.php, /tmp/lint/megalinter-reports/IDE-config.txt, /tmp/lint/megalinter-reports/IDE-config/.vscode/extensions.json, /tmp/lint/megalinter-reports/linters_logs/PHPCBF_files.dat,…(full list in DEBUG)
Kept [40923] files on [43513] found files

+----MATCHING LINTERS--+----------+----------------+------------+
| Descriptor | Linter  | Criteria | Matching files | Format/Fix |
+------------+---------+----------+----------------+------------+
| PHP        | phpcs   | .php     | 40923          | no         |
| PHP        | phpstan | .php     | 40923          | no         |
+------------+---------+----------+----------------+------------+

Unable to get number of errors with regex_number and FOUND ([0-9]+) ERRORS
✅ Linted [PHP] files with [phpcs]: Found 1 non blocking error(s) - (0.49s)
- Using [phpcs v3.7.2] https://megalinter.io/7.2.1/descriptors/php_phpcs
- MegaLinter key: [PHP_PHPCS]
- Rules config: [/phpcs.xml]
- Number of files analyzed: [40923]
--Error detail:
Fatal error while calling phpcs: [Errno 7] Argument list too long: 'phpcs'

Unable to get number of errors with regex_number and Found ([0-9]+) error
✅ Linted [PHP] files with [phpstan]: Found 1 non blocking error(s) - (0.44s)
- Using [phpstan v1.10.26] https://megalinter.io/7.2.1/descriptors/php_phpstan
- MegaLinter key: [PHP_PHPSTAN]
- Rules config: [/phpstan.neon.dist]
- Number of files analyzed: [40923]
--Error detail:
Fatal error while calling phpstan: [Errno 7] Argument list too long: 'phpstan'


+----SUMMARY-+---------+---------------+-------+-------+--------+--------------+
| Descriptor | Linter  | Mode          | Files | Fixed | Errors | Elapsed time |
+------------+---------+---------------+-------+-------+--------+--------------+
| ⚠️ PHP     | phpcs   | list_of_files | 40923 |       |      1 |        0.49s |
| ⚠️ PHP     | phpstan | list_of_files | 40923 |       |      1 |        0.44s |
+------------+---------+---------------+-------+-------+--------+--------------+

After the change:

MegaLinter now collects the files to analyse
Listing all files in directory [/tmp/lint], then filter with:
- File extensions: .php
- Excluding .gitignored files [41277]: .env, .php-cs-fixer.cache, .phpstorm.meta.php, .phpunit.result.cache, _ide_helper.php, bootstrap/cache/packages.php, bootstrap/cache/services.php, megalinter-reports/IDE-config.txt, megalinter-reports/IDE-config/.vscode/extensions.json, megalinter-reports/linters_logs/PHPCBF_files.dat,…(full list in DEBUG)
Kept [2099] files on [43513] found files

+----MATCHING LINTERS--+----------+----------------+------------+
| Descriptor | Linter  | Criteria | Matching files | Format/Fix |
+------------+---------+----------+----------------+------------+
| PHP        | phpcs   | .php     | 2099           | no         |
| PHP        | phpstan | .php     | 2099           | no         |
+------------+---------+----------+----------------+------------+

✅ Linted [PHP] files with [phpcs]: Found 4 non blocking error(s) - (69.12s)
- Using [phpcs v3.7.2] https://megalinter.io/7.2.1/descriptors/php_phpcs
- MegaLinter key: [PHP_PHPCS]
- Rules config: [/phpcs.xml]
- Number of files analyzed: [2099]
--Error detail:

...

Time: 1 mins, 8.91 secs; Memory: 38MB



✅ Linted [PHP] files with [phpstan]: Found 16 non blocking error(s) - (271.66s)
- Using [phpstan v1.10.26] https://megalinter.io/7.2.1/descriptors/php_phpstan
- MegaLinter key: [PHP_PHPSTAN]
- Rules config: [/phpstan.neon.dist]
- Number of files analyzed: [2099]
--Error detail:

...

 [ERROR] Found 16 errors




+----SUMMARY-+---------+---------------+-------+-------+--------+--------------+
| Descriptor | Linter  | Mode          | Files | Fixed | Errors | Elapsed time |
+------------+---------+---------------+-------+-------+--------+--------------+
| ⚠️ PHP     | phpcs   | list_of_files |  2099 |       |      4 |       69.12s |
| ⚠️ PHP     | phpstan | list_of_files |  2099 |       |     16 |      271.66s |
+------------+---------+---------------+-------+-------+--------+--------------+

@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 Jul 29, 2023
@iisisrael iisisrael changed the title Adds workspace path to ignored files check Removes prefixing of ignored files with workspace path Jul 31, 2023
@nvuillam nvuillam temporarily deployed to dev August 21, 2023 20:26 — with GitHub Actions Inactive
@nvuillam nvuillam temporarily deployed to dev August 21, 2023 20:26 — with GitHub Actions Inactive
@github-actions
Copy link
Contributor

This pull request 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 pull request should stay open, please remove the O: stale 🤖 label or comment on the pull request.

@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 21, 2023
@iisisrael
Copy link
Contributor Author

@nvuillam I saw this was deployed to dev a while ago, and then nothing... commenting to avoid the stale auto-close. Should I rebase to resolve the changelog and support more testing?

@nvuillam
Copy link
Member

@iisisrael I think I solved it in #2967 , I'm sorry I had forgot about your PR :(

So if you rebase you'll probably see that the only remaining change is changelog :/

Sorry, I'll credit you in the release changelog as u solved it way before me ^^

@nvuillam nvuillam closed this Sep 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O: stale 🤖 This issue or pull request is stale, it will be closed if there is no activity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants