0.62: always inspects 0 files when using Cygwin git

[agross]

Windows supports two git variants:

• git for Windows which understands Windows paths
• Cygwin git, which works with unix paths

Description

• rubocop 0.62 only includes files for analysis that git considers part of the repository (Exclude files in .gitignore #595)
• rubocop invokes git ls-files <path> ... to get those files
• The path follows standard Ruby conventions (forward slashes on Windows, e.g. D:/projects/foo)
• Cygwin git does understand this path and returns no files and exits with status of 0 (success)

  TargetFinder.ls_git_files("D:/projects/foo")
  #=> []

  • Cygwin mounts Windows drives under /cygdrive by default, e.g. /cygdrive/d/projects/foo
  • Cygwin users may elect to change this mountpoint to e.g. /d/projects/foo
  • Cygwin git would understand those paths referring to a mountpoint
• rubocop then checks whether a file enumerated from the file system is included in the empty git_files array
• because the array is empty any file is considered as ignored

Discussion

I see that the root cause is git which should (perhaps?) error out if ls-files is invoked with a directory that does not exist.

$ uname
CYGWIN_NT-10.0
$ git ls-files D:/does/not/exist && echo should not happen
should not happen

---

Expected behavior

All files are inspected.

Perhaps treat git_files.empty? like git_files.nil?.

Actual behavior

No files are inspected effectively disabling rubocop.

Steps to reproduce the problem

1. Install Windows
2. Install Windows Ruby
3. Install Cygwin including the git package
4. Run Cygwin shell

   $ mkdir test
   $ cd test
   $ touch file.rb
   $ rubocop
   Inspecting 1 file
   1 file inspected, no offenses detected
   $ git init
   Initialized empty Git repository in /home/agross/test/.git/
   $ rubocop
   Inspecting 0 files
   0 files inspected, no offenses detected

git/RuboCop version

$ git --version
git version 2.17.0
$ bundle exec rubocop -V
0.62.0 (using Parser 2.5.3.0, running on ruby 2.6.0 x64-mingw32)

[bbatsov]

Thanks for the detailed report! I'm still not sure whether we'd fix or remove the git integration.

@AlexWayfer I guess now you understand why I was so skeptical of adding this feature. 😆

[agross]

A fix for the "run rubocop in the current directory" case is to replace base_dir = Dir.pwd with base_dir = '.' here.

It isn't as it breaks cops.

[agross]

git ls-files <path> will error if <path> is outside of the current repository, effectively disabling the new feature for invocations like rubocop ../somewhere/outside/this/repo.

I continued testing on macOS which shows more problems unrelated to Cygwin:

If you run rubocop only for a subdirectory of the current repo you will also get 0 files to be analyzed. I tried 0.60 and 0.62, where only 0.60 analyzes files.

This issue stems from the fact that git ls-files returns files relative to the current working directory. ls_git_files then File.joins these with the base_dir:

1. cd path/to/repo
2. Run rubocop lib
3. git ls-files lib returns %w[lib/foo.rb lib/bar.rb]
4. The array is mapped to %w[lib/lib/foo.rb lib/lib/bar.rb]
5. Enumerated files in the form of lib/foo.rb are tested against the mapped array always yielding false

To summarize:

• rubocop ../somewhere/outside/this/repo will not consider ignored files if ../somewhere/outside/this/repo is a git repo with ignored ruby files
• rubocop subdir will not consider any files if run inside a git repo (same behavior as in the OP)

[agross]

Another problem exists if you haven't added sh to your $PATH on Windows: 56c4cd0#r31888653

I'd say on a typical Windows machine this is the default. You might argue that developers likely run git. But your Windows-based CI server might not. Still, you might want to run rubocop there against your code that was transferred to the server by other means.

[thomthom]

I've had problems with having git's bash tools in my path historically. Conflicting with other tools. So I've opted out of it.

Having to have bash tools available introduce a workflow problem because of this.

If I install bash tools I run into problems with other tools I need for my work. But if I don't then I cannot use rubocop in VSCode and RubyMine etc. I'd be limited to running a separate bash console which then severely limits the usefulness of RuboCop.

I'm maintaining the rubocop-sketchup extension as a tool for developers who develop extensions for SketchUp's Ruby API. SketchUp is a desktop application where the wast majority of users are on Windows - thus also our API users.

Using git to enhance the exclude filter is fine by me. But requiring bash to check if git is available feels heavy handed. I understand it simplifies code by not having branches based on platform, but at the expense of Windows users. (It's already a bit of a pain to contribute to rubocop from Windows because the tests expect Unix line-endings - while at the same time checking out the git repo the default git behavious is to normalize to Windows endings.)

I realize Windows isn't the typical platform for Ruby developers, but I make my plea for having these Windows platform issues reconsidered.

@agross details issues with Cygwin that I'm not experience with. So I'm afraid I'm short on help there.

I was looking briefly at the usage of sh to detect git. At first I though it would be possible to use where git - which in the classic terminal returns a list of paths from PATH where it is found. But from a quick test in PowerShell it doesn't appear to work there. And powershell is being pushed hard from MS, being default terminal in VSCode.

Ideally I'd prefer that usage of rubocop reverted to being more flexible under Windows without having to have these additional constraints.

[agross]

I was looking briefly at the usage of sh to detect git. At first I though it would be possible to use where git

Instead of checking for the git executable by running sh (which will raise Errno::ENOENT if it's not found - which is to be expected on Windows) rubocop could catch Errno::ENOENT when running Open3.capture3. This would remove the dependency on sh.

All other problems (Cygwin and subdirs) would persist nonetheless.

[abrthel]

Instead of catching the Errno::ENOENT error for sh maybe something like system('git --version') would be better. If the return value isn't nil then you know it's available.

[bbatsov]

That's reverted on master. We're dropping the git integration in the next release.

[AlexWayfer]

I apologize to all of you about problems with git integration. I'll try to be smarter and more careful.