Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix exclusion of .gemspec file itself in bundle gem generated gemspec file #7488

Merged
merged 1 commit into from
Mar 5, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 4 additions & 3 deletions bundler/lib/bundler/templates/newgem/newgem.gemspec.tt
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,10 @@ Gem::Specification.new do |spec|

# Specify which files should be added to the gem when it is released.
# The `git ls-files -z` loads the files in the RubyGem that have been added into git.
spec.files = Dir.chdir(__dir__) do
`git ls-files -z`.split("\x0").reject do |f|
(File.expand_path(f) == __FILE__) ||
gemspec = File.basename(__FILE__)
spec.files = IO.popen(%w[git ls-files -z], chdir: __dir__, err: IO::NULL) do |ls|
ls.readlines("\x0", chomp: true).reject do |f|
(f == gemspec) ||
Comment on lines -30 to +33
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was already a complicated line to drop into a new gem. The change makes it even more complicated.

Should we add a helper to spec that does this? Maybe:

spec.files = spec.git_files_excluding(__FILE__, "bin", "spec")

I know this is a more involved change. We could make a temporary fix while we wait for improved syntax.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a class method?
It feels questionable to me that Gem::Specification instances should have such method.

Copy link
Contributor Author

@nobu nobu Feb 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, what version of git is required in the current rubygems?
Since git 1.9, exclude "magic signature" pathspecs are available.

excludes = %w[bin/ test/ spec/ features/ .git <%= config[:ci_config_path] %>appveyor Gemfile]

dir, gemspec = File.split(__FILE__)
excludes = [dir, *excludes].map {|e| ":^#{e}"}
spec.files = (IO.popen(*%w[git ls-files -z --], *excludes, chdir: dir) do |f|
                f.readlines("\0", chomp: true)
              end rescue [])

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, what version of git is required in the current rubygems?

I think we must support the version of git that ships with OSX. We've never made a real requirement that I know of.

@hsbt, what do you think? I don't feel strongly myself and I would be fine merging this if others are ok with it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On macOS 14.3.1:

$ /usr/bin/git --version
git version 2.39.3 (Apple Git-145)

Not sure about older versions.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I investigate git versions of debian and ubuntu.

  • Ubuntu 22.04: 2.34.1
  • Debian 12: 2.39.2

We should support git-2.34.x at least.

Copy link
Contributor Author

@nobu nobu Mar 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hsbt Thanks, git 2.x is enough for the "magic signatures".

I found this description in Xcode 12 Release Notes.

New Features
You can now specify the default branch name for new repositories in the Source Control pane of Xcode’s Preferences. This preference uses the init.defaultBranch Git config option available in Git version 2.28. (66232985)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have an official "lowest supported version" but we do ship code to make sure to play nice with git older than 1.8.5.

If I recall correctly, the motivation was that CentOS 7 shipped with a very old git version. CentOS 7 reaches its end of life by the 30th of June, I think it'd be ok to drop support after that an officially declare an official lowest supported version like 2.x.

f.start_with?(*%w[bin/ test/ spec/ features/ .git <%= config[:ci_config_path] %>appveyor Gemfile])
end
end
Expand Down