Drop git ls-files in gemspec

[utkarsh2102]

Avoid using git to produce lists of files. Downstreams often need to build your package in an environment that does not have git (on purpose). Use some pure Ruby alternative, like Dir or Dir.glob.

Simultaneously, add rubocop-packaging as a dependency.

Signed-off-by: Utkarsh Gupta <utkarsh@debian.org>

[timoschilling]

Avoid using git to produce lists of files. Downstreams often need to build your package in an environment that does not have git (on purpose).

Can you give an example for such a case.

Use some pure Ruby alternative, like Dir or Dir.glob.

`git ls-files LICENSE docs lib`.split("\n") and Dir['docs/**/*', 'lib/**/*', 'LICENSE'] aren't do the same. There are reasons for using git ls-files, take a look on the history of the bundler template file if you want to know more about the why.

Simultaneously, add rubocop-packaging as a dependency.

I don't like to add such undocumented gem which only has a README from an generator.

[deivid-rodriguez]

Hi @timoschilling! Nice seeing you around :)

Just to give you some context. I've been recently in touch with Debian ruby packagers and we're collaborating to try to make things easier for OS packagers while not compromising the experience for upstream maintainers. In particular, @utkarsh2102 is working as part of a GSoC program to build a tool to enforce a set of best practices that upstream maintainers can follow to make lifes of packagers easier. That is how rubocop-packaging is born. It's in a very early state (only one rule so far, no git in gemspecs 😃), so it's fair enough if you don't want to adopt it right now 👍.

Regarding git ls-files, there's not only Debian or other OS packaging examples, but also a couple others that come to mind:

• ruby-core as part of their CI system runs their test suite against an unpackaged ruby tarball which doesn't have a .git folders. That means they needed to overwrite the bundler gemspec to not use git. Actually, not anymore since we removed git from bundler's gemspec.

• If you build your application on a bare docker image without git, and you are pointing to a git sourced gem that uses git on its gemspec, you'll get No such file or directory - git ls-files (Errno::ENOENT) warnings all around. For example, if you use gem "arbre", git: "https://github.com/activeadmin/arbre" in your Gemfile. (NOTE: As per a private comment, you do need git to install this kind of Gemfile, but you don't necessarily need git to run it).

Originally, git ls-files inside the default gemspec template was designed so that users publishing their first gem wouldn't unintentionally publish artifacts to it. However, in our case we're not beginners anymore and arbre doesn't have a C extension or anything, so the risk of anything like this happening is very low. Also note that bundler won't let you release if you have uncommitted stuff in your working repository, so the risk is even lower.

How about we add a spec to make sure that the Dir.glob clause in the gemspec exactly matches the what git ls-files would do? That way we stay on the safe side, while moving the git dependency out of the gemspec. Something like the spec I added in https://github.com/rubygems/bundler/pull/6985/files#diff-0612364e18e1f39125551ebb93623a1fR240-R251 to bundler.

By the way, @utkarsh2102, the Dir.glob will need a bit of extra tweaking to filter out directories to be 100% equivalent:

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

[deivid-rodriguez]

I don't like to add such undocumented gem which only has a README from an generator.

By the way, this is the gem. Just linking because I suspect you may have found something similar in another location, since the README is definitely documented 😅.

[timoschilling]

I found that exactly that repo, I just missed some summary about what does the cops are checking.

[deivid-rodriguez]

Oh, ok, you're right! The README definitely needs a link to a list of rules and documentation for them, thanks for letting us know ❤️.

[utkarsh2102]

Hello,

Thanks, @deivid-rodriguez, for that explanation, I should've done a better job myself at explaining this PR in the first place :/
Also, I've fixed this as per your suggestion, thanks for that, too ❤️

And thanks for the feedback, @timoschilling, that certainly helps. I'll make the README more detailed 😄

[deivid-rodriguez]

Thanks so much @timoschilling and @utkarsh2102!! 💜