Drop `git` in gemspec

[utkarsh2102]

Hi @JonRowe & @pirj,

Thanks for your great work on this! ❤️

Whilst maintaining this in Debian, we found that this library relies on git to produce a list of files, which can be achieved by a plain Ruby alternative. Using git in gemspec is problematic in general. We're working with the bundler team to fix this there itself.

It'd be great if we can drop git from gemspec here, at least! If you concur, I'd be happy to raise a PR doing the same! Please let me know what you think! 😄

[JonRowe]

We're happy to consider an alternative providing it produces the same output, in particular the point of using git is to avoid a gem spec accidentally including uncommitted files during a release, given that everyone releasing rspec up until now has had git installed this has never been a problem.

[pirj]

Hi!

I understand your concern about building deb packages. Is it problematic to add a git dependency for building them? Or are you building outside of a git repository?

This was also done this way to avoid publishing a gem with extra files that don't really belong to the git repository, e.g. bundler binstubs etc that are in the .gitignore file. I glanced over our repos's gemspec files, and don't really see any technical obstacles for using Dir[] instead of git shell-out. Dir[] seems to be supported on Ruby 1.8.6+, this shouldn't be a problem. Did you plan to change this in this repository only, or all of them? rspec-collection_matchers differs slightly.

avoid a gem spec accidentally including uncommitted files during a release

As the maintainer, @JonRowe has a much better view of the problem. I've just heard it's not that straightforward as it seems.

[pirj]

RuboCop's opinion on this topic https://docs.rubocop.org/rubocop-packaging/cops_packaging.html#packaginggemspecgit

[utkarsh2102]

Hello 👋🏻

We're happy to consider an alternative providing it produces the same output, in particular the point of using git is to avoid a gem spec accidentally including uncommitted files during a release, given that everyone releasing rspec up until now has had git installed this has never been a problem.

Is it problematic to add a git dependency for building them? Or are you building outside of a git repository? This was also done this way to avoid publishing a gem with extra files that don't really belong to the git repository, e.g. bundler binstubs etc that are in the .gitignore file.

Ah, so bundler now doesn't let you release if you've got uncommited changes in your repository. So that's good to go! See the rationale at https://docs.rubocop.org/rubocop-packaging/cops_packaging.html#gemspec-git-rationale 😄

I glanced over our repos's gemspec files, and don't really see any technical obstacles for using Dir[] instead of git shell-out. Dir[] seems to be supported on Ruby 1.8.6+, this shouldn't be a problem.

Yep, so I would propose something like this: Dir["lib/**/*"].reject { |f| File.directory?(f) }

To see if the output produced is same:

irb(main):001:0> `git ls-files -- lib/*`.split("\n").sort == Dir["lib/**/*"].reject { |f| File.directory?(f) }.sort
=> true

Please let me know if you're happy with this? 😄

Did you plan to change this in this repository only, or all of them? rspec-collection_matchers differs slightly.

I'll be willing and very happy to bring about this change in the libraries that I maintain in Debian at least! And rspec-collection_matchers shouldn't be a problem at all, I have a patch in mind already, so it should be okay as well!

RuboCop's opinion on this topic https://docs.rubocop.org/rubocop-packaging/cops_packaging.html#packaginggemspecgit

Eeee, glad that you mentioned that! ^.^ This was written to help the downstream maintenance without compromising the experience of the upstream maintainers. So it also helps other distributions then Debian! \o/

[JonRowe]

Ideally we also need to enforce its a committed file, or at the very least that there are no uncommitted files.

[utkarsh2102]

Okay, so there are two ways here:

• Respecting the .gitignore file: It should be simple, something like: Rake::FileList["lib/**/*"].exclude(*File.read('.gitignore').split).reject { |f| File.directory?(f) }

• Explicitly mentioning the files in the repository: s.files = File.read("Manifest.txt").split where, Manifest.txt contains the list of all the files available in this git repository, generate via a Rake task where we could use git ls-files.

What do you think? Do you have a preference? I'll be happy to submit a PR based on this^

[pirj]

Just a note that .gitignore can contain comments. That doesn't seem to affect us/be our case though.

I've checked the diff between your approach with FileList on rspec-core, and the only difference was the missing lib/rspec/core/project_initializer/.rspec.

[utkarsh2102]

Hi @pirj,

I've checked the diff between your approach with FileList on rspec-core, and the only difference was the missing lib/rspec/core/project_initializer/.rspec.

Ah, Rake::FileList cannot yet parse the ... it seems 😞

Perhaps, using Manifest.txt is the best way forward here?

[pirj]

Dir['*'] that doesn't return "hidden" files that start with a dot, but Dir['.*'] does, but only those starting with the dot. AFAIR it's a nix thing. `Dir['', '.*']will return both, and I guessFileList` behaves exactly the same way.

PS Frankly, any code-based solution is good, if it doesn't corner us into manually keeping the list of files in a file.

[utkarsh2102]

Eeeeeks, I misunderstood your second last comment. That should be easily fixed! 😅 I'll work on a PR today!

[utkarsh2102]

Opened #2384, which should be good to go!

irb(main):001:0> require "rake/file_list"
=> true
irb(main):002:0> `git ls-files -- lib/*`.split("\n").sort == Rake::FileList["lib/**/{.*,*}"].exclude(*File.read(".gitignore").split).reject { |f| File.directory?(f) }
=> true

[JonRowe]

Closing as stale. Any Ruby replacement for the git commands needs to provide the same safeguards as the git commands, or allow for the git commands to be used when git is available.