Skip to content
This repository was archived by the owner on Apr 14, 2021. It is now read-only.

Avoid printing git error when checking version on badly packaged version#6664

Merged
bundlerbot merged 1 commit intorubygems:masterfrom
greysteil:avoid-printing-git-error
Aug 22, 2018
Merged

Avoid printing git error when checking version on badly packaged version#6664
bundlerbot merged 1 commit intorubygems:masterfrom
greysteil:avoid-printing-git-error

Conversation

@greysteil
Copy link
Copy Markdown
Contributor

What was the end-user problem that led to this PR?

The problem was #6453.

What was your diagnosis of the problem?

A use had a version of Bundler packaged with Ruby 2.5.0 in RVM, but this version didn't include Bundler's git repo or any of the ivars which should have been set. As a result, Bundler was shelling out to git even though it wasn't in a git repo.

What is your fix for the problem, implemented in this PR?

My fix is to show a nicer result in this case (just say that the SHA is unknown)

Why did you choose this fix out of the possible options?

I chose this fix because ensuring Bundler is always packaged correctly by others isn't possible (although it seems weird and unlikely that a version without instance variables set would be used). Falling back to a cleaner error message is marginally nicer than what we currently have, and not much work.

Fixes #6453.


# If Bundler has been installed without its .git directory and without a
# commit instance variable then we can't determine its commits SHA.
return "unknown" unless File.directory?(".git")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this isn't quite right -- this will check whether the current working directory has a .git subdirectory


# Otherwise shell out to git.
@git_commit_sha = Dir.chdir(File.expand_path("..", __FILE__)) do
`git rev-parse --short HEAD`.strip.freeze if
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

missing conditional

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That will teach me not to test again after cleaning up! Fixed.

@colby-swandale
Copy link
Copy Markdown
Member

Thank you!

@bundlerbot r+

@bundlerbot
Copy link
Copy Markdown
Collaborator

📌 Commit bdc6d64 has been approved by colby-swandale

@colby-swandale colby-swandale added this to the 1.16.5 milestone Aug 22, 2018
@bundlerbot
Copy link
Copy Markdown
Collaborator

⌛ Testing commit bdc6d64 with merge 6dcc62e...

bundlerbot added a commit that referenced this pull request Aug 22, 2018
…ndale

Avoid printing git error when checking version on badly packaged version

### What was the end-user problem that led to this PR?

The problem was #6453.

### What was your diagnosis of the problem?

A use had a version of Bundler packaged with Ruby 2.5.0 in RVM, but this version didn't include Bundler's git repo or any of the ivars which should have been set. As a result, Bundler was shelling out to git even though it wasn't in a git repo.

### What is your fix for the problem, implemented in this PR?

My fix is to show a nicer result in this case (just say that the SHA is unknown)

### Why did you choose this fix out of the possible options?

I chose this fix because ensuring Bundler is always packaged correctly by others isn't possible (although it seems weird and unlikely that a version without instance variables set would be used). Falling back to a cleaner error message is marginally nicer than what we currently have, and not much work.

Fixes #6453.
@bundlerbot
Copy link
Copy Markdown
Collaborator

💔 Test failed - status-travis

@colby-swandale
Copy link
Copy Markdown
Member

Oops that was me.

@colby-swandale
Copy link
Copy Markdown
Member

@bundlerbot retry

@bundlerbot
Copy link
Copy Markdown
Collaborator

⌛ Testing commit bdc6d64 with merge e5e260e...

bundlerbot added a commit that referenced this pull request Aug 22, 2018
…ndale

Avoid printing git error when checking version on badly packaged version

### What was the end-user problem that led to this PR?

The problem was #6453.

### What was your diagnosis of the problem?

A use had a version of Bundler packaged with Ruby 2.5.0 in RVM, but this version didn't include Bundler's git repo or any of the ivars which should have been set. As a result, Bundler was shelling out to git even though it wasn't in a git repo.

### What is your fix for the problem, implemented in this PR?

My fix is to show a nicer result in this case (just say that the SHA is unknown)

### Why did you choose this fix out of the possible options?

I chose this fix because ensuring Bundler is always packaged correctly by others isn't possible (although it seems weird and unlikely that a version without instance variables set would be used). Falling back to a cleaner error message is marginally nicer than what we currently have, and not much work.

Fixes #6453.
@bundlerbot
Copy link
Copy Markdown
Collaborator

☀️ Test successful - status-travis
Approved by: colby-swandale
Pushing e5e260e to master...

@bundlerbot bundlerbot merged commit bdc6d64 into rubygems:master Aug 22, 2018
@greysteil greysteil deleted the avoid-printing-git-error branch September 12, 2018 11:41
colby-swandale pushed a commit that referenced this pull request Sep 14, 2018
…ndale

Avoid printing git error when checking version on badly packaged version

### What was the end-user problem that led to this PR?

The problem was #6453.

### What was your diagnosis of the problem?

A use had a version of Bundler packaged with Ruby 2.5.0 in RVM, but this version didn't include Bundler's git repo or any of the ivars which should have been set. As a result, Bundler was shelling out to git even though it wasn't in a git repo.

### What is your fix for the problem, implemented in this PR?

My fix is to show a nicer result in this case (just say that the SHA is unknown)

### Why did you choose this fix out of the possible options?

I chose this fix because ensuring Bundler is always packaged correctly by others isn't possible (although it seems weird and unlikely that a version without instance variables set would be used). Falling back to a cleaner error message is marginally nicer than what we currently have, and not much work.

Fixes #6453.

(cherry picked from commit e5e260e)
colby-swandale pushed a commit that referenced this pull request Sep 18, 2018
* 1-16-stable:
  Version 1.16.5 with changelog
  scope TruffleRuby platform specs to be RubyGems >= 2.1.0
  Auto merge of #6689 - bundler:colby/fix-bundler-load-error, r=colby-swandale
  Auto merge of #6695 - bundler:segiddins/6684-gvp-prefer-non-pres, r=colby-swandale
  Auto merge of #6693 - eregon:truffleruby, r=colby-swandale
  Auto merge of #6692 - eregon:simplify-autoload-require-deprecate, r=segiddins
  Auto merge of #6688 - voxik:check-search, r=colby-swandale
  Auto merge of #6682 - bundler:bundle_env_formatting, r=colby-swandale
  Auto merge of #6675 - MaxLap:master, r=greysteil
  Auto merge of #6669 - ChrisBr:fix_dep_proxy, r=segiddins
  Auto merge of #6664 - greysteil:avoid-printing-git-error, r=colby-swandale
ghost pushed a commit that referenced this pull request Oct 12, 2018
6733: Check for file or folder when checking for git hash in build metadata r=colby-swandale a=colby-swandale

Thanks so much for the contribution!
To make reviewing this PR a bit easier, please fill out answers to the following questions.

### What was the end-user problem that led to this PR?

A change that was introduced in #6664 is breaking RubyGem's specs because the Build Metadata is not being generated correctly.

This is breaking because the change is checking for the `.git` to determine if the git hash should be fetched for the version string. This folder does not exist in the submodule in RubyGems' repo

### What was your diagnosis of the problem?

See https://travis-ci.org/rubygems/rubygems/jobs/437576972

### What is your fix for the problem, implemented in this PR?

Check if `.git` exists either as a file or folder

Co-authored-by: Colby Swandale <me@colby.fyi>
ghost pushed a commit that referenced this pull request Oct 20, 2018
6737: when running from source, find submodule shas too r=indirect a=indirect

We recently merged #6664 to prevent Bundler from (wrongly) claiming the
git commit of any parent directory when it is run from source. However,
Bundler is also run from source as a submodule in RubyGems, and in that
case it does not have its own git directory.

This commit resolves the failure in version_spec.rb that only appears
when the Bundler tests are run from a submodule, by explicitly checking
for that submodule, and using `git ls-tree` to get the sha if so.

Arguably, this is a bugfix compared to the previous behavior, which
would blindly print the current sha from the _RubyGems_ repo, while
claiming that it was the sha of Bundler.

Co-authored-by: Andre Arko <andre@arko.net>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bundle version v1.16.1 prints strange "fatal: Not a git repository ...' message

4 participants