search

joeyates / imap-backup

Backup and Migrate IMAP Email Accounts
MIT License
1.33k stars 74 forks source link

get list of gem files from file system and .gitignore #106

Closed jmechnich closed 2 years ago

jmechnich commented 2 years ago

Currently, it is not possible to build a gem from the GitHub archives for each tag. This PR should fix that.

joeyates commented 2 years ago

Hi @jmechnich thanks for the PR.

I've commented on the Homebrew PR (Homebrew/homebrew-core#88089).

I'm not actually clear why this PR is necessary build the gem locally from the GitHub archive.

As an experiment, I downloaded the latest tag as a zip, and ran 

$ gem build imap-backup.gemspec
$ gem install imap-backup-3.4.0.gem

and this installed the gem.

Could you explain why these changes are necessary?

Thanks!

jmechnich commented 2 years ago

You will end up with an empty gem:

mechnich@darkstar:~/Downloads/imap-backup-3.4.0$ gem build imap-backup.gemspec 
fatal: not a git repository (or any parent up to mount point /)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
WARNING:  licenses is empty, but is recommended.  Use a license identifier from
http://spdx.org/licenses or 'Nonstandard' for a nonstandard license.
WARNING:  no files specified

The tag archive files don't contain the .git directory, so git ls-files cannot work.

joeyates commented 2 years ago

Hi @jmechnich

Thanks for your clarification.

As it is not normal practice for gems to be buildable in the way that this PR proposes, I would like to understand why this is a special case.

As I mentioned in Homebrew/homebrew-core#88089, I'm not clear why the formula doesn't simply install the gem directly, and has to build it locally. It looks like that is the normal practice in Homebrew - for example here

jmechnich commented 2 years ago

I am not an experienced Ruby coder, so I can't comment on what is supposed to be "normal". However, I expect to be able to build a software successfully from a source archive, which failed. I can understand though that the previous version might be more convenient when building from a cloned repo.

As for the homebrew formula, maybe installing the gem directly is an alternative way for implementing it. Basically, I just started out with the template for a ruby build (brew create --ruby ...), and there are other formulas that work the same way, for example travis.rb. It was not a suggested change from the reviewers though, so, again, I can't really comment on what is "normal".

From my naive point of view, it seems like a good idea to get rid of the git ls-files in order to make the source archives sort of self-consistent.

joeyates commented 2 years ago

Thanks for the additional info.

I checked the most popular gems (+Rails), and they use 4 different ways of constructing the files list:

Dir.glob + Rejection of directories + Explicit List

Dir() Mixing Globs and Explicit List

Example:

Dir["CHANGELOG.md", "MIT-LICENSE", "README.rdoc", "lib/**/*"]

Explicit list

diff-lcs (4th) rails, which has only one file

git ls + Explicit List

rspec-expectations (2nd) rspec-core (3rd) rspec-mocks (5th) rspec-support (6th) rspec (7th)

Conclusion

So, of the most popular gems, only the RSpec families does things like imap-backup.

The best practice seems to be to use Dir.glob (or Dir, which is equivalent), or an explicit list, which I think adds maintenance load.

jmechnich commented 2 years ago

@joeyates I am not sure if this is the solution that you will be happy with but here you go. Please let me know if any important files are missing.

joeyates commented 2 years ago

Hi @jmechnich

I did a manual rebase and merge via the command line.

Thanks again for the PR!

jmechnich commented 2 years ago

@joeyates I have just noticed that imap-backup.gemspec still contained the now superfluous require "rake/file_list" which leads to your CI failing. I have amended my branch accordingly but maybe you just want to remove it. Sorry for my negligence. :(