Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion jekyll-github-metadata.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ Gem::Specification.new do |spec|
spec.files = `git ls-files -z`.split("\x0").grep(%r!^(lib|bin)/!)
spec.require_paths = ["lib"]

spec.add_runtime_dependency "jekyll", ENV["JEKYLL_VERSION"] ? "~> #{ENV["JEKYLL_VERSION"]}" : "~> 3.1"
spec.add_runtime_dependency "jekyll", ENV["JEKYLL_VERSION"] ? "~> #{ENV["JEKYLL_VERSION"]}" : "~> 3.4"
spec.add_runtime_dependency "octokit", "~> 4.0", "!= 4.4.0"

spec.add_development_dependency "bundler", "~> 1.5"
Expand Down
14 changes: 5 additions & 9 deletions lib/jekyll-github-metadata/repository_finder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,16 +45,12 @@ def nwo_from_config
repo if repo && repo.is_a?(String) && repo.include?("/")
end

def git_exe_path
ENV["PATH"].to_s
.split(File::PATH_SEPARATOR)
.map { |path| File.join(path, "git") }
.find { |path| File.exist?(path) }
end

def git_remotes
return [] if git_exe_path.nil?
`#{git_exe_path} remote --verbose`.to_s.strip.split("\n")
_process, output = Jekyll::Utils::Exec.run("git", "remote", "--verbose")
output.to_s.strip.split("\n")
rescue Errno::ENOENT => e
Jekyll.logger.warn "Not Installed:", e.message
[]
end

def git_remote_url
Expand Down
8 changes: 5 additions & 3 deletions spec/repository_finder_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,14 @@
end

context "when git doesn't exist" do
before(:each) { @old_path = ENV.delete("PATH").to_s.split(File::PATH_SEPARATOR) }
after(:each) { ENV["PATH"] = @old_path.join(File::PATH_SEPARATOR) }
before(:each) do
@old_path = ENV["PATH"]
ENV["PATH"] = ""
Copy link
Member

Choose a reason for hiding this comment

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

?

Copy link
Member

Choose a reason for hiding this comment

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

The PATH is "reset" to an empty string to simulate the scenario where git is not installed.. (especially in CI environments)

end
after(:each) { ENV["PATH"] = @old_path }

it "fails with a nice error message" do
allow(subject).to receive(:git_remote_url).and_call_original
expect(subject.send(:git_exe_path)).to eql(nil)
Copy link
Member

Choose a reason for hiding this comment

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

I think we still need to be able to run git? If we're relying on Jekyll::Utils::Exec, then we'll definitely need to make sure that the exe can be found, right?

Copy link
Member

Choose a reason for hiding this comment

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

Relying on Jekyll::Utils::Exec is like running system "git" in a begin..rescue block..
If the utility method doesn't find the executable, an Errno::ENOENT is raised which is rescued to output a warning to the terminal..

Copy link
Member

Choose a reason for hiding this comment

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

Ok, so we can remove git_exe_path?

Copy link
Member

Choose a reason for hiding this comment

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

Yep. In fact, this PR has already removed it..

expect(subject.send(:git_remote_url)).to be_empty
end
end
Expand Down