super-linter: Super-linter algorithm for finding changed files is broken

Describe the bug

git diff-tree used by linter on push checks only files from the last commit and regardless of files status, so deleted or renamed files are also checked.

There are now 2 different git commands used for finding the list of broken files (find them here: https://github.com/github/super-linter/blob/v3.14.4/lib/functions/buildFileList.sh#L59-L105). git diff-tree is used on push, git diff is used otherwise. A separate behaviour for push was added in #905 and follow-up #1049 to fix push to a default branch. But it seems working only in rare special cases: only one commit is pushed and no files are deleted. Otherwise wrong list is reported.

Expected behavior

All changed non-deleted files from all commits added on top of the default branch should be checked, deleted files should be ignored.

Steps to Reproduce

Here are the steps to reproduce within git (super-linter is not involved.

Adding first file, everything works fine:

+ git log --oneline master...465e71b
465e71b (HEAD -> test) Add test1.txt

+ git diff-tree --no-commit-id --name-only -r 465e71b
test/test1.txt

+ git diff --name-only master...465e71b --diff-filter=d
test/test1.txt

Now adding the second file in a separate commit, diff git-tree reports only the last file - only files changed in the last commit:

+ git log --oneline master...1c3d652
1c3d652 (HEAD -> test) Add test2.txt
465e71b Add test1.txt

+ git diff-tree --no-commit-id --name-only -r 1c3d652
test/test2.txt

+ git diff --name-only master...1c3d652 --diff-filter=d
test/test1.txt
test/test2.txt

Renaming first file, git diff-tree reports again only files from the last commit and as another issue - reports deleted file as well:

+ git log --oneline master...f5617a2
f5617a2 (HEAD -> test) Rename test1.txt to test3.txt
1c3d652 Add test2.txt
465e71b Add test1.txt

+ git diff-tree --no-commit-id --name-only -r f5617a2
test/test1.txt
test/test3.txt

+ git diff --name-only master...f5617a2 --diff-filter=d
test/test2.txt
test/test3.txt

Now deleting the second file, git diff-tree reports only this deleted file:

+ git log --oneline master...ee90eaf
ee90eaf (HEAD -> test) Remove test2.txt
f5617a2 Rename test1.txt to test3.txt
1c3d652 Add test2.txt
465e71b Add test1.txt

+ git diff-tree --no-commit-id --name-only -r ee90eaf
test/test2.txt

+ git diff --name-only master...ee90eaf --diff-filter=d
test/test3.txt

Additional context

Right now git diff command looks better for all cases except finding changes for a push to a DEFAULT_BRANCH (the case described in #900). I’d say git diff should be used instead of the other command, or git diff-tree needs to be called with a right list of parameters.

I don’t have suggestions on how better to fix the original issue from #900. If there needs to be a special code condition for that case maybe it should be checking not only git push event but also if current branch is a DEFAULT_BRANCH.

This project really needs testing for the “get diff files list” behaviour. It was broken couple of times already. Having wrong files checked is really disappointing. What makes it even more disappointing is that you often don’t even realize this. With current broken behaviour wrong files were linted in the last 3 months when push event was used. I believe that’s quite a lot of people and projects since this setup is recommended by super-linter readme 😢

Additional tough thing is that it’s impossible to debug “get diff files list” in local, since VALIDATE_ALL_CODEBASE=true behaviour is used in that case.

About this issue

  • Original URL
  • State: closed
  • Created 3 years ago
  • Reactions: 1
  • Comments: 18 (12 by maintainers)

Commits related to this issue

Most upvoted comments

Ping @admiralAwkbar @GaboFDC @ferrarimarco @zkoppert. Could you check please, I think that’s a very important issue.