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

@tknerr metadata deps not honored #717

Merged
merged 5 commits into from Jul 25, 2013
Merged
Show file tree
Hide file tree
Changes from 4 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
2 changes: 1 addition & 1 deletion berkshelf.gemspec
Expand Up @@ -39,7 +39,7 @@ Gem::Specification.new do |s|
s.add_dependency 'hashie', '>= 2.0.2'
s.add_dependency 'minitar', '~> 0.5.4'
s.add_dependency 'retryable', '~> 1.3.3'
s.add_dependency 'ridley', '~> 1.3.2'
s.add_dependency 'ridley', '~> 1.4.0'
s.add_dependency 'solve', '>= 0.5.0'
s.add_dependency 'thor', '~> 0.18.0'

Expand Down
53 changes: 53 additions & 0 deletions features/install_command.feature
Expand Up @@ -407,3 +407,56 @@ Feature: install cookbooks from a Berksfile
An error occurred during Git execution:
"""
And the exit status should be "GitError"

@focus
Copy link
Contributor

Choose a reason for hiding this comment

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

gotta remove the focus bits

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 left them in until we can figure out how to fix them (i.e. this PR is incomplete)

Copy link
Contributor

Choose a reason for hiding this comment

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

gotta remove this focus tag

Scenario: transitive dependencies in metadata
Given the cookbook store contains a cookbook "fake" "1.0.0" with dependencies:
| bacon | >= 0.0.0 |
Given I write to "Berksfile" with:
"""
source "http://localhost:26210"

metadata
"""
And I write to "metadata.rb" with:
"""
depends 'fake', '1.0.0'
depends 'bacon', '0.2.0'
"""
And the Chef Server has cookbooks:
| bacon | 0.1.0 |
| bacon | 0.2.0 |
| bacon | 1.0.0 |
And the Berkshelf API server's cache is up to date
When I successfully run `berks install`
Then the cookbook store should have the cookbooks:
| bacon | 0.2.0 |
Then the output should contain:
"""
Installing bacon (0.2.0)
"""

@focus
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@reset can you take a look at this? It's still failing against master. It looks like we still trust local versions...

Copy link
Contributor

Choose a reason for hiding this comment

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

this one too

Scenario: transitive dependencies in metadata when cookbooks are downloaded
Given the cookbook store contains a cookbook "fake" "1.0.0" with dependencies:
| bacon | >= 0.0.0 |
And the cookbook store has the cookbooks:
| bacon | 1.0.0 |
| bacon | 0.3.0 |
| bacon | 0.2.0 |
And I write to "Berksfile" with:
"""
source "http://localhost:26210"

metadata
"""
And I write to "metadata.rb" with:
"""
depends 'fake', '1.0.0'
depends 'bacon', '0.2.0'
"""
When I successfully run `berks install`
Then the output should contain:
"""
Using bacon (0.2.0)
"""
16 changes: 3 additions & 13 deletions lib/berkshelf/dependency.rb
Expand Up @@ -76,6 +76,8 @@ def validate_options(options)
attr_reader :groups
# @return [Berkshelf::Location]
attr_reader :location
# @return [Solve::Version]
attr_accessor :locked_version
# @return [Solve::Constraint]
attr_accessor :version_constraint
# @return [Berkshelf::CachedCookbook]
Expand Down Expand Up @@ -131,7 +133,7 @@ def cached_cookbook
@cached_cookbook ||= if location
location.download
else
Berkshelf::CookbookStore.instance.satisfy(name, version_constraint)
Berkshelf::CookbookStore.instance.satisfy(name, Solve::Constraint.new(locked_version.to_s))
end
end

Expand All @@ -155,18 +157,6 @@ def has_group?(group)
groups.include?(group.to_sym)
end

# Get the locked version of this cookbook. First check the instance variable
# and then resort to the cached_cookbook for the version.
#
# This was formerly a delegator, but it would fail if the `@cached_cookbook`
# was nil or undefined.
#
# @return [Solve::Version, nil]
# the locked version of this cookbook
def locked_version
@locked_version ||= cached_cookbook ? cached_cookbook.version : nil
end

# The location for this dependency, such as a remote Chef Server, the
# community API, :git, or a :path location. By default, this will be the
# community API.
Expand Down
2 changes: 1 addition & 1 deletion lib/berkshelf/formatters/json.rb
Expand Up @@ -28,7 +28,7 @@ def cleanup_hook
# @param [Berkshelf::Dependency] dependency
def fetch(dependency)
cookbooks[dependency] ||= {}
cookbooks[dependency][:version] = dependency.cached_cookbook.version
cookbooks[dependency][:version] = dependency.locked_version.to_s
cookbooks[dependency][:location] = dependency.location
end

Expand Down
1 change: 1 addition & 0 deletions lib/berkshelf/installer.rb
Expand Up @@ -42,6 +42,7 @@ def run(options = {})

cached_cookbooks = resolver.resolve.collect do |name, version, dependency|
lock_deps << dependency
dependency.locked_version ||= Solve::Version.new(version)
if dependency.downloaded?
Berkshelf.formatter.use(dependency.name, dependency.cached_cookbook.version, dependency.location)
dependency.cached_cookbook
Expand Down
2 changes: 1 addition & 1 deletion spec/unit/berkshelf/dependency_spec.rb
Expand Up @@ -215,7 +215,7 @@
end

it 'includes the locked version' do
subject.stub(cached_cookbook: double('cached', version: '1.2.3'))
subject.stub(locked_version: double('cached', to_s: '1.2.3'))

expect(hash).to have_key(:locked_version)
expect(hash[:locked_version]).to eq('1.2.3')
Expand Down