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

[WIP]Add pristine command and spec #4729

Closed
wants to merge 2 commits into from
Closed
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
11 changes: 11 additions & 0 deletions lib/bundler/cli.rb
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,17 @@ def doctor
Doctor.new(options).run
end

desc "pristine [GEMNAME ...] [OPTIONS]", "Restores installed gems to pristine condition from your "
method_option "skip", :type => :array, :default => [], :banner => "Used on --all, skip if name == gem_name"
method_option "extensions", :type => :boolean, :default => false, :banner =>
"If –extensions is given (but not –all or gem names) only gems with extensions will be restored."
method_option "no-extensions", :type => :boolean, :default => false, :banner =>
"If –no-extensions is provided pristine will not attempt to restore a gem with an extension."
def pristine(*gem_list)
require "bundler/cli/pristine"
Pristine.new(options, gem_list).run
end

if Bundler.settings[:plugins]
require "bundler/cli/plugin"
desc "plugin SUBCOMMAND ...ARGS", "manage the bundler plugins"
Expand Down
98 changes: 98 additions & 0 deletions lib/bundler/cli/pristine.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
# frozen_string_literal: true
require "bundler/cli/exec"
module Bundler
class CLI::Pristine
Copy link
Member

Choose a reason for hiding this comment

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

please nest the class inside the namespace

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Didn't I already done the nesting?

Copy link
Member

Choose a reason for hiding this comment

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

never mind, this seems to be how the other CLI classes do it

class ConflictingGems < PristineError
end
class AmbiguousOption < PristineError
end
attr_reader :options, :gem_list

def initialize(options, gem_list)
@options = options
@gem_list = gem_list
end

def run
# Determine if -extensions and --no-extensions or are conflict
if options["extensions"] && options["no-extensions"]
Bundler.ui.error "You can't use --extensions and --no-extensions at the same time"
raise AmbiguousOption
end

# Raise error when Gemfile is not present
definition = begin
Bundler.definition
rescue GemfileNotFound
Bundler.ui.error "You can't pristine without a Gemfile. Please consider add Gemfile then run `bundle install` before pristine"
raise
end
# Raise error when lockfile is not present
unless Bundler.default_lockfile.exist?
Copy link
Member

Choose a reason for hiding this comment

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

why can this run install implicitly and just pristine the installed gems?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought it should be the case that the user installs dependencies via bundler then change it, and then want to pristine so he/she could "revert back" to the original state.
But on a second thought, it makes sense that the user changes local installed gems first then want to pristine.
I think a warning and ask if the user want to let us do the bundle install for him/her should be more flexible?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any thoughts on this?
@indirect @RochesterinNYC

Bundler.ui.error "You can't pristine without a Gemfile.lock. Please consider run `bundle install` before pristine"
raise GemfileLockNotFound, "Could not locate Gemfile.lock"
end

# Find the path gems and exclude them from calling gem pristine
# If gem list is empty, we need to add everything to pristine_gems, else we can use things from the gem_list
if gem_list.empty?
pristine_gems = definition.calculate_non_path_gem_list
git_gems = definition.calculate_git_gems
path_gems = definition.calculate_path_only_gems
warn_path_gems(path_gems)
pristine(pristine_gems, git_gems, true)
else
check_conflicting(gem_list, options[:skip])
pristine(gem_list)
end
end

private

def check_conflicting(gem_list, skip_gems)
conflicting_gems = skip_gems & gem_list
return if conflicting_gems.empty?
Bundler.ui.error "You can't list a gem in both GEMLIST and --skip." \
"The offending gems are: #{conflicting_gems.join(", ")}."
raise ConflictingGems
end

def pristine(gems, git_gems = nil, lazy_spec_provided = false)
gem_list = lazy_spec_provided ? gems.map(&:name) : gems
skip_gems = options[:skip]
pristine_gems = compute_pristine_gems(gem_list, skip_gems)
binding.pry
Copy link

@ruba-ruba ruba-ruba Nov 6, 2016

Choose a reason for hiding this comment

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

is pry here intentionally?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it's still a WIP.

pristine_gems.each do |gem|
command = String.new("gem pristine #{gem}")
command << " -v #{gem.version}" if lazy_spec_provided && gem.respond_to?(:version)
command << " --extensions" if options["extensions"]
command << " --no-extensions" if options["no-extensions"]
CLI.start(command.split)
end
pristine_git_gems(git_gems)
end

def warn_path_gems(path_gems)
# Warn about the path gems
return if path_gems.empty?
message = String.new
message << "At this moment, Bundler cannot prstine the following gems:"
path_gems.each do |gem|
message << "* #{gem.name} at #{gem.source.path}"
end
Bundler.ui.warn(message)
end

def pristine_git_gems(gems)
# Pristine git gems
end

def compute_pristine_gems(gems, skip_gems)
if gems.is_a?(SpecSet)
gems.reject {|gem| skip_gems.include? gem.name }
else
gems - skip_gems
end
end
end
end
25 changes: 23 additions & 2 deletions lib/bundler/definition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ class Definition
:gem_version_promoter,
:locked_deps,
:locked_gems,
:locked_specs,
:platforms,
:requires,
:ruby_version
Expand Down Expand Up @@ -413,8 +414,8 @@ def ensure_equivalent_gemfile_and_lockfile(explicit_flag = false)

# Check if it is possible that the source is only changed thing
if (new_deps.empty? && deleted_deps.empty?) && (!new_sources.empty? && !deleted_sources.empty?)
new_sources.reject! {|source| source.is_a_path? && source.path.exist? }
deleted_sources.reject! {|source| source.is_a_path? && source.path.exist? }
new_sources.reject! {|source| source.a_path? && source.path.exist? }
deleted_sources.reject! {|source| source.a_path? && source.path.exist? }
end

if @locked_sources != gemfile_sources
Expand Down Expand Up @@ -486,6 +487,26 @@ def remove_platform(platform)
raise InvalidOption, "Unable to remove the platform `#{platform}` since the only platforms are #{@platforms.join ", "}"
end

def any_path_sources?
!sources.path_sources.empty?
end

def any_git_sources?
!sources.git_sources.empty?
end

def calculate_non_path_gem_list
SpecSet.new(@locked_specs.reject {|locked_spec| locked_spec.source && locked_spec.source.a_path? })
end

def calculate_git_gems
@locked_deps.select {|locked_spec| locked_spec.source && locked_spec.source.a_git? }
end

def calculate_path_only_gems
@locked_deps.select {|locked_spec| locked_spec.source && locked_spec.source.a_path? && !locked_spec.source.a_git? }
end

attr_reader :sources
private :sources

Expand Down
1 change: 1 addition & 0 deletions lib/bundler/errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ class LockfileError < BundlerError; status_code(20); end
class CyclicDependencyError < BundlerError; status_code(21); end
class GemfileLockNotFound < BundlerError; status_code(22); end
class PluginError < BundlerError; status_code(29); end
class PristineError < BundlerError; status_code(30); end
class GemfileEvalError < GemfileError; end
class MarshalError < StandardError; end

Expand Down
8 changes: 8 additions & 0 deletions lib/bundler/source.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,5 +33,13 @@ def can_lock?(spec)
def include?(other)
other == self
end

def a_path?
instance_of?(Path)
end

def a_git?
instance_of?(Git)
end
end
end
4 changes: 0 additions & 4 deletions lib/bundler/source/path.rb
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,6 @@ def root
Bundler.root
end

def is_a_path?
instance_of?(Path)
end

private

def expanded_path
Expand Down
36 changes: 36 additions & 0 deletions spec/commands/pristine_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# frozen_string_literal: true
require "spec_helper"
describe "bundle pristine" do
context "when the Gemfile is missing" do
it "should provide useful information" do
bundle "pristine"
expect(out).to include "You can't pristine without a Gemfile. Please consider add Gemfile"
expect(out).to include "Could not locate Gemfile"
end
end

context "when the Gemfile is present and Gemfile.lock is missing" do
before :each do
gemfile <<-G
source "file://#{gem_repo1}"
gem "rack", "1.0.0"
gem "foo", "1.2.3"
G
end
it "should provide useful information" do
bundle "pristine"
expect(out).to include "You can't pristine without a Gemfile.lock. Please consider run `bundle install`"
expect(out).to include "Could not locate Gemfile.lock"
end
end

context "when both the Gemfile and the Gemfile.lock is present" do
before :each do
install_gemfile <<-G
source "file://#{gem_repo1}"
gem "rack", "1.0.0"
gem "foo", "1.2.3"
G
end
end
end