Super-linter: Allow running of GitHub linter on non-git tracked files

[bazzadp]

Is your feature request related to a problem? Please describe.

Recently it has only become possible to run the Super-Linter locally against committed files, I think due to this line.

As part of pull requests I build generated HTML files, that are not checked back into git, and used to use the GitHub Super-Linter to lint them, but this looks to no longer be possible with the latest versions of the Super-Linter - even when using IGNORE_GITIGNORED_FILES=false

Describe the solution you'd like
When choosing IGNORE_GITIGNORED_FILES=false flag the Super-Linter should lint all files and not just the files tracked in git. Otherwise there is no real point in this flag.

Describe alternatives you've considered
Rolling back to previous versions of the Super-Linter

[bazzadp]

Looks like this was discussed in https://github.com/github/super-linter/pull/1185#issuecomment-776191880 but then forgotten about when #1192 was merged.

I tried reverting part of #1192 when this is not set:

    debug "----------------------------------------------"
    if [ "${IGNORE_GITIGNORED_FILES}" == "true" ]; then
      debug "Populating the file list with:[git -C \"${WORKSPACE_PATH}\" ls-tree -r --name-only HEAD | xargs -I % sh -c \"echo ${WORKSPACE_PATH}/%\"]"
      mapfile -t RAW_FILE_ARRAY < <(git -C "${WORKSPACE_PATH}" ls-tree -r --name-only HEAD | xargs -I % sh -c "echo ${WORKSPACE_PATH}/%" 2>&1)
    else
      debug "Populating the file list with all the files in the ${WORKSPACE_PATH} workspace"
      mapfile -t RAW_FILE_ARRAY < <(find "${WORKSPACE_PATH}" \
        -not \( -path '*/\.git' -prune \) \
        -not \( -path '*/\.pytest_cache' -prune \) \
        -not \( -path '*/\.rbenv' -prune \) \
        -not \( -path '*/\.terragrunt-cache' -prune \) \
        -not \( -path '*/\.venv' -prune \) \
        -not \( -path '*/\__pycache__' -prune \) \
        -not \( -path '*/\node_modules' -prune \) \
        -not -name ".DS_Store" \
        -not -name "*.gif" \
        -not -name "*.ico" \
        -not -name "*.jpg" \
        -not -name "*.jpeg" \
        -not -name "*.pdf" \
        -not -name "*.png" \
        -not -name "*.webp" \
        -not -name "*.woff" \
        -not -name "*.woff2" \
        -not -name "*.zip" \
        -type f \
        2>&1 | sort)
    fi
    debug "RAW_FILE_ARRAY contents: ${RAW_FILE_ARRAY[*]}"

But having problems building it locally to test it:

 ---> Running in e0b27d8f950f
Connecting to phar.io (188.94.27.6:443)
Connecting to github.com (140.82.121.4:443)
wget: bad address 'github-releases.githubusercontent.com'
The command '/bin/sh -c wget --tries=5 -O phive.phar https://phar.io/releases/phive.phar     && wget --tries=5 -O phive.phar.asc https://phar.io/releases/phive.phar.asc     && PHAR_KEY_ID="0x9D8A98B29B2D5D79"     && ( gpg --keyserver ha.pool.sks-keyservers.net --recv-keys "$PHAR_KEY_ID"     || gpg --keyserver pgp.mit.edu --recv-keys "$PHAR_KEY_ID"     || gpg --keyserver keyserver.pgp.com --recv-keys "$PHAR_KEY_ID" )     && gpg --verify phive.phar.asc phive.phar     && chmod +x phive.phar     && mv phive.phar /usr/local/bin/phive     && rm phive.phar.asc     && phive install --trust-gpg-keys 31C7E470E2138192,CF1A108D0E7AE720,8A03EA3B385DBAA1' returned a non-zero code: 1

Any ideas?

Or is there a better way to do this?

[ferrarimarco]

Thanks for reporting.

@nemchik what was the rationale behind #1192? I forgot about why we needed to use git ls-tree to build the file list, and the PR description doesn't describe that.

[nemchik]

Thanks for reporting.

@nemchik what was the rationale behind #1192? I forgot about why we needed to use git ls-tree to build the file list, and the PR description doesn't describe that.

The find command was actually leaving out dotfiles (seemingly unintentionally). git ls-tree should be a more consistent way of checking files that are part of the git repo. We'll need to add something back in in to check files that are not tracked by git.

[bazzadp]

Ok so what do you think of the above suggested fix? And any ideas why on the build error I get when trying to test it locally?

[nemchik]

Ok so what do you think of the above suggested fix? And any ideas why on the build error I get when trying to test it locally?

The logic you have for when to use the find command vs git ls-tree is fine, but we should adjust the find command itself to ensure it includes dotfiles.

[bazzadp]

I've tested the above find command (which was the original one in the repo before #1192 ) on both MacOS and CentOS and it finds dotfiles.

So confused what the original issue with dotfiles was?

On that note it really would help if PRs had more detail in this repo. For a well-used project with 6.4k stars and counting we really need to get better at that.

[nemchik]

So confused what the original issue with dotfiles was?

That's interesting. From what I could tell using git ls-tree was picking up dotfiles that the find command was missing. This wasn't specifically the original motive behind the change though (see below).

On that note it really would help if PRs had more detail in this repo. For a well-used project with 6.4k stars and counting we really need to get better at that.

You are 100% correct about this. #1192 was originally meant to merge into #1185 and be part of the development of that PR, so details for it was overlooked. There was some discussion about it in the comments of #1185

[bazzadp]

Reading #1185 was it the opposite? We were including dotfiles when we shouldn't be? So moving to git ls-tree only linted real files?

[nemchik]

Per the title of #1185 the intention was to optionally allow ignoring files that were not tracked by the git repository. git ls-tree was discussed as a solution to ensure only files tracked by git would be linted. The issue with #1192 is that it did not make the functionality optional. A side effect of git ls-tree was that it included files that the find command did not in our CI, which leads me to believe this would affect others as well, and I noticed all of these newly recognized files were dotfiles. So ideally we should find a way to use a find command for files that are NOT tracked or intentionally ignored by git, and we should add those files to the array of files found by git ls-tree. A possible solution would be to use something like git ls-files --others

[bazzadp]

So we've two issues with #1192 moving to git ls-tree:

1. Extra files being linted now that were not linted before (maybe they should have been, maybe they should not)
2. Missing files outside of git that were being picked up before.

A possible solution would be to use something like git ls-files --others

My concern with that is including all the excluded files above (e.g. node_modules). In fact that brings up potential issue 3 with #1192:

1. Including un-lintable files (e.g. .jpg files) that were previously ignored thanks to all the exclusions above.

Should we revert #1192 until this is fully understood to revert to functionality to the way it was?

So ideally we should find a way to use a find command for files that are NOT tracked or intentionally ignored by git

I don't understand this. Do you mean ARE tracked or are intentionally ignored by git? Isn't that just ALL files?

And I argue we want exclusions to that list - like .git or node_modules or... well all the ones excluded in the original code reproduced above!

Do you have the concrete examples of the dotfiles that were picked up with git ls-tree but not with the original find command?

[nemchik]

So we've two issues with #1192 moving to git ls-tree:

1. Extra files being linted now that were not linted before (maybe they should have been, maybe they should not)
2. Missing files outside of git that were being picked up before.

A possible solution would be to use something like git ls-files --others

My concern with that is including all the excluded files above (e.g. node_modules). In fact that brings up potential issue 3 with #1192:

1. Including un-lintable files (e.g. .jpg files) that were previously ignored thanks to all the exclusions above.

Should we revert #1192 until this is fully understood to revert to functionality to the way it was?

So ideally we should find a way to use a find command for files that are NOT tracked or intentionally ignored by git

I don't understand this. Do you mean ARE tracked or are intentionally ignored by git? Isn't that just ALL files?

And I argue we want exclusions to that list - like .git or node_modules or... well all the ones excluded in the original code reproduced above!

Do you have the concrete examples of the dotfiles that were picked up with git ls-tree but not with the original find command?

Comparing the log output from https://github.com/github/super-linter/runs/1963102172 and https://github.com/github/super-linter/runs/1962042196 in the Run against all code base step and accounting for the change in folder structure due to the files no longer moving from .automation/test/ to .automation/automation/ and ignoring that all the test files with bad in the filename (files that should intentionally fail) are now being included in the RAW_FILE_ARRAY and then just considering the differences, here's what I can say for sure;

• The following files were linted that were NOT part of the repo:
  
  
  
  • /tmp/lint/.automation/test/.snakemake/log/2021-02-23T145450.027375.snakemake.log
  
  
  • /tmp/lint/.automation/test/.snakemake/log/2021-02-23T145450.335705.snakemake.log
  
  
  • /tmp/lint/.lintr
  
  
  • /tmp/lint/super-linter.log
  
  
• The following files were NOT linted that were part of the repo:
  
  
  
  • /tmp/lint/.automation/test/dockerfile_hadolint/good/Dockerfile
  
  
  • /tmp/lint/.automation/test/dockerfile/good/Dockerfile
  
  
  • /tmp/lint/.automation/test/javascript_prettier/javascript_good_1.js
  
  
  • /tmp/lint/.automation/test/jscpd/good/Dockerfile
  
  
  • /tmp/lint/.dockerignore <-- this was the file that I noticed first and thought it was a dotfile issue
  
  
  • /tmp/lint/Dockerfile
  
  
  • /tmp/lint/TEMPLATES/.arm-ttk.psd1
  
  
  • /tmp/lint/TEMPLATES/.powershell-psscriptanalyzer.psd1
  
  

I definitely agree there are some files that should absolutely not be linted by super-linter, such as anything in the node_modules folder. The .git folder is actually excluded by git ls-files --others but if we use git ls-files --others we should still add more exclusions such as git ls-files --others --exclude=node_modules --exclude=vendor. Exclude patterns follow .gitignore formatting, and we can add as many as needed for things like images.

I'm not opposed to another solution, git ls-files --others with --exclude= patterns seems very promising.

Also sorry for the confusing wording above. I did mean that we should find a way to use find to locate files that are not tracked or intentionally ignored by git. I should have continued on to say that we would combine the results of the find with the results of git ls-tree in order to form a complete list of files in the working directory that could be linted, and then lint both sets based on how the user defines their environment. That seems potentially easier to achieve with git ls-files --other combines with git ls-tree.

P.S. I seem to get the same results when running git ls-tree -r --name-only HEAD and git ls-files (without --others) so it's possible that we could simplify it even more and just use git ls-files and git ls-files --others.

[admiralAwkbar]

@nemchik I agree and think this is a super valid solution

[nemchik]

One potential downside of using the git ls-* commands as i've outlined above is it does not consider running super-linter on a folder that is not part of a git repo (meaning, executing on an arbitrary folder). find did cover this.

[nemchik]

More thoughts (just to keep them here for discussion)
https://git-scm.com/docs/git-ls-files#Documentation/git-ls-files.txt---exclude-fromltfilegt
We could designate something like .super-linter-ignore as a way for users to add their own things to be ignored by super linter.

[bazzadp]

One potential downside of using the git ls-* commands as i've outlined above is it does not consider running super-linter on a folder that is not part of a git repo (meaning, executing on an arbitrary folder). find did cover this.

This was actually something I used when developing locally. I have a script that copies the files I wanted linted (plus the .github/linters folder) to a tmp folder and linted that:

npm run lint static/js

or:

npm run lint static/css static/js

It was much faster to lint just those file(s) or folder(s) as the find command was way slow due to size of the repo. Maybe git ls-* will be quicker so that's not an issue and saves me having a hacky local script. It can almost be replicated with FILTER_REGEX_INCLUDE but requires some complex regexes so not quite as easy as my script.

Anyway happy to go with git ls-* if it allows me to lint generated HTML files (the Jinja2 source templates can't be linted as HTML due to complex if/then/else branches).

We could designate something like .super-linter-ignore as a way for users to add their own things to be ignored by super linter.

Love this idea!

[ferrarimarco]

The .super-linter-ignore thing is what originally led me to implement #1185, because I liked that sort of behavior, but didn't want to re-implement that logic.

Now that we have the #1185 in place, it's trivial to use a .super-linter-ignore (or whatever) file. We just have to:

1. Configure the core.excludesFile Git configuration directive to point to the .super-linter-ignore file, which has a higher precedence than the .gitignore file. See https://git-scm.com/docs/gitignore for details. We can do this in the container. We can do this when we build the image.
2. Force IGNORE_GITIGNORED_FILES to true when .super-linter-ignore exists.
3. Add some debugging information.

This also supports symlinking .super-linter-ignore to .gitignore.

If we choose to go this route, I'd also consider:

1. Removing the FILTER_REGEX_XXXX features, because they'd be redundant.
2. Providing a default .super-linter-ignore, where we ignore what the original find command invocation ignored, after we fix what we've to fix to avoid excluding the files that @nemchik mentioned, if that is still applicable.

3. Ignore binary files by default. Here's an example function (that I use in my dotfiles), to check if git thinks that a file is binary or not.

   is_binary() {
       p=$(printf '%s\t-\t' -)
       t=$(git diff --no-index --numstat /dev/null "$1")
       case "$t" in "$p"*) return 0 ;; esac
       return 1
   }
   

[GaboFDC]

@ferrarimarco in that case, will we go back to use find? or what method will we use to list the files?
Just is important to clarify, because as it said before, when e did the change, we dropped the support for linting non-git repos, this use case is mostly for local linting, but is really related to this.
And I'm fine if we decide to drop this support, but we should be more informative about this, because as it was done, it was more of an unintended side efect.

[ferrarimarco]

No. We keep the current behavior. See my explanation above :)