"knife cookbook site install" fails. Knife or Git issue?


#1

Hello,

I started of with a repository where rabbitmq_chef was installed via
"knife cookbook site install" (Chef 0.10.0). This was still a wrong
dependency for the chef-server cookbook, so I tried to install the
regular rabbitmq cookbook via “knife coobkook site install rabbitmq”,
but it failed.

All output here: https://gist.github.com/958560

After I cleaned up the local and remote chef-vendor-rabbitmq_chef
branch, re-running the rabbitmq installation worked. Can someone
elaborate if this is a Knife or a Git issue? I’m not a Git master…

Ringo


#2

Gents,

This is a bug in Chef, more specifically in CookbookSCMRepo.branch_exists?

In my case, I already had branch chef-vendor-rabbitmq_chef in my repo,
but now I wanted to add branch chef-vendor-rabbitmq. The
implementation checks the list of existing branches and matches using
the String.include? method, however:

“chef-vendor-rabbitmq_chef”.include?(“chef-vendor-rabbitmq”) => true

Filed this ticket: CHEF-2314

Ringo


#3

Fixed by https://github.com/akzhan/chef/tree/CHEF-2314, ticket is resolved
and passed to Adam Jacob.

2011/5/6 Ringo De Smet ringo.desmet@gmail.com

Gents,

This is a bug in Chef, more specifically in CookbookSCMRepo.branch_exists?

In my case, I already had branch chef-vendor-rabbitmq_chef in my repo,
but now I wanted to add branch chef-vendor-rabbitmq. The
implementation checks the list of existing branches and matches using
the String.include? method, however:

“chef-vendor-rabbitmq_chef”.include?(“chef-vendor-rabbitmq”) => true

Filed this ticket: CHEF-2314

Ringo


#4

I don’t have the Chef server code in front of me, but are you sure it’s
calling include? on the individual strings rather than the whole list?The
latter would seem to make more sense in this instance.

[“chef-vendor-rabbitmq_chef”].include?(“chef-vendor-rabbitmq”) => false

On Fri, May 6, 2011 at 9:12 AM, Ringo De Smet ringo.desmet@gmail.comwrote:

Gents,

This is a bug in Chef, more specifically in CookbookSCMRepo.branch_exists?

In my case, I already had branch chef-vendor-rabbitmq_chef in my repo,
but now I wanted to add branch chef-vendor-rabbitmq. The
implementation checks the list of existing branches and matches using
the String.include? method, however:

“chef-vendor-rabbitmq_chef”.include?(“chef-vendor-rabbitmq”) => true

Filed this ticket: CHEF-2314

Ringo


Mark J. Reed markjreed@gmail.com


#5

I have fixed server code and add spec for it. In original master branch this
spec is failed.

Mac-mini-Akzhan-Abdulin:chef akzhanabdulin$ spec
spec/unit/knife/core/cookbook_scm_repo_spec.rb
…F…

‘Chef::knife::CookbookSCMRepo determines if a the branch not exists
correctly without substring search’ FAILED
expected true to be false
./spec/unit/knife/core/cookbook_scm_repo_spec.rb:122:

Finished in 0.015377 seconds

16 examples, 1 failure

2011/5/6 Mark J. Reed markjreed@gmail.com

I don’t have the Chef server code in front of me, but are you sure it’s
calling include? on the individual strings rather than the whole list?The
latter would seem to make more sense in this instance.

[“chef-vendor-rabbitmq_chef”].include?(“chef-vendor-rabbitmq”) => false

On Fri, May 6, 2011 at 9:12 AM, Ringo De Smet ringo.desmet@gmail.comwrote:

Gents,

This is a bug in Chef, more specifically in CookbookSCMRepo.branch_exists?

In my case, I already had branch chef-vendor-rabbitmq_chef in my repo,
but now I wanted to add branch chef-vendor-rabbitmq. The
implementation checks the list of existing branches and matches using
the String.include? method, however:

“chef-vendor-rabbitmq_chef”.include?(“chef-vendor-rabbitmq”) => true

Filed this ticket: CHEF-2314

Ringo


Mark J. Reed markjreed@gmail.com


#6

There I have added branch ‘chef-vendor-absent-new’

And try to look for branch chef-vendor-absent. branch_exists? returns wrong
true value.

2011/5/6 Akzhan Abdulin akzhan.abdulin@gmail.com

I have fixed server code and add spec for it. In original master branch
this spec is failed.

Mac-mini-Akzhan-Abdulin:chef akzhanabdulin$ spec
spec/unit/knife/core/cookbook_scm_repo_spec.rb
…F…

‘Chef::knife::CookbookSCMRepo determines if a the branch not exists
correctly without substring search’ FAILED
expected true to be false
./spec/unit/knife/core/cookbook_scm_repo_spec.rb:122:

Finished in 0.015377 seconds

16 examples, 1 failure

2011/5/6 Mark J. Reed markjreed@gmail.com

I don’t have the Chef server code in front of me, but are you sure it’s

calling include? on the individual strings rather than the whole list?The
latter would seem to make more sense in this instance.

[“chef-vendor-rabbitmq_chef”].include?(“chef-vendor-rabbitmq”) => false

On Fri, May 6, 2011 at 9:12 AM, Ringo De Smet ringo.desmet@gmail.comwrote:

Gents,

This is a bug in Chef, more specifically in
CookbookSCMRepo.branch_exists?

In my case, I already had branch chef-vendor-rabbitmq_chef in my repo,
but now I wanted to add branch chef-vendor-rabbitmq. The
implementation checks the list of existing branches and matches using
the String.include? method, however:

“chef-vendor-rabbitmq_chef”.include?(“chef-vendor-rabbitmq”) => true

Filed this ticket: CHEF-2314

Ringo


Mark J. Reed markjreed@gmail.com


#7

Mark,

On 6 May 2011 15:59, Mark J. Reed markjreed@gmail.com wrote:

I don’t have the Chef server code in front of me, but are you sure it’s
calling include? on the individual strings rather than the whole list?The
latter would seem to make more sense in this instance.
[“chef-vendor-rabbitmq_chef”].include?(“chef-vendor-rabbitmq”) => false

It is not a collection include that is used. Here is the faulty code:

git(“branch --no-color”).stdout.lines.any? {|l| l.include?(branch_name) }

The include? is invoked on any element of the collection, which
results in a string compare.

Ringo


#8

By the way, a lot of tests written in fair manner with tests of
"is_boolean_function?.should be_true/false". These tests write very
noniformative failure notices when spec failed.

I have rewritten spec for CHEF-2314 using “should/should_not
be_boolean_function”. This syntax provides more informative failure notices.

2011/5/6 Akzhan Abdulin akzhan.abdulin@gmail.com

There I have added branch ‘chef-vendor-absent-new’

And try to look for branch chef-vendor-absent. branch_exists? returns wrong
true value.

2011/5/6 Akzhan Abdulin akzhan.abdulin@gmail.com

I have fixed server code and add spec for it. In original master branch
this spec is failed.

Mac-mini-Akzhan-Abdulin:chef akzhanabdulin$ spec
spec/unit/knife/core/cookbook_scm_repo_spec.rb
…F…

‘Chef::knife::CookbookSCMRepo determines if a the branch not exists
correctly without substring search’ FAILED
expected true to be false
./spec/unit/knife/core/cookbook_scm_repo_spec.rb:122:

Finished in 0.015377 seconds

16 examples, 1 failure

2011/5/6 Mark J. Reed markjreed@gmail.com

I don’t have the Chef server code in front of me, but are you sure it’s

calling include? on the individual strings rather than the whole list?The
latter would seem to make more sense in this instance.

[“chef-vendor-rabbitmq_chef”].include?(“chef-vendor-rabbitmq”) => false

On Fri, May 6, 2011 at 9:12 AM, Ringo De Smet ringo.desmet@gmail.comwrote:

Gents,

This is a bug in Chef, more specifically in
CookbookSCMRepo.branch_exists?

In my case, I already had branch chef-vendor-rabbitmq_chef in my repo,
but now I wanted to add branch chef-vendor-rabbitmq. The
implementation checks the list of existing branches and matches using
the String.include? method, however:

“chef-vendor-rabbitmq_chef”.include?(“chef-vendor-rabbitmq”) => true

Filed this ticket: CHEF-2314

Ringo


Mark J. Reed markjreed@gmail.com


#9

Now failure notice in master branch looks like:

expected branch_exists?(“chef-vendor-absent”) to return false, got true

2011/5/6 Akzhan Abdulin akzhan.abdulin@gmail.com

By the way, a lot of tests written in fair manner with tests of
"is_boolean_function?.should be_true/false". These tests write very
noniformative failure notices when spec failed.

I have rewritten spec for CHEF-2314 using “should/should_not
be_boolean_function”. This syntax provides more informative failure notices.

2011/5/6 Akzhan Abdulin akzhan.abdulin@gmail.com

There I have added branch ‘chef-vendor-absent-new’

And try to look for branch chef-vendor-absent. branch_exists? returns
wrong true value.

2011/5/6 Akzhan Abdulin akzhan.abdulin@gmail.com

I have fixed server code and add spec for it. In original master branch
this spec is failed.

Mac-mini-Akzhan-Abdulin:chef akzhanabdulin$ spec
spec/unit/knife/core/cookbook_scm_repo_spec.rb
…F…

‘Chef::knife::CookbookSCMRepo determines if a the branch not exists
correctly without substring search’ FAILED
expected true to be false
./spec/unit/knife/core/cookbook_scm_repo_spec.rb:122:

Finished in 0.015377 seconds

16 examples, 1 failure

2011/5/6 Mark J. Reed markjreed@gmail.com

I don’t have the Chef server code in front of me, but are you sure it’s

calling include? on the individual strings rather than the whole list?The
latter would seem to make more sense in this instance.

[“chef-vendor-rabbitmq_chef”].include?(“chef-vendor-rabbitmq”) => false

On Fri, May 6, 2011 at 9:12 AM, Ringo De Smet ringo.desmet@gmail.comwrote:

Gents,

This is a bug in Chef, more specifically in
CookbookSCMRepo.branch_exists?

In my case, I already had branch chef-vendor-rabbitmq_chef in my repo,
but now I wanted to add branch chef-vendor-rabbitmq. The
implementation checks the list of existing branches and matches using
the String.include? method, however:

“chef-vendor-rabbitmq_chef”.include?(“chef-vendor-rabbitmq”) => true

Filed this ticket: CHEF-2314

Ringo


Mark J. Reed markjreed@gmail.com


#10

Akzhan,

On 6 May 2011 15:59, Akzhan Abdulin akzhan.abdulin@gmail.com wrote:

Fixed by https://github.com/akzhan/chef/tree/CHEF-2314, ticket is resolved
and passed to Adam Jacob.

As Mark indicated, wouldn’t the fix be easier if we fall back to
ary.include? instead?
http://www.ruby-doc.org/core/classes/Array.html#M000265

git(“branch --no-color”).stdout.lines.include?(branch_name)

Ringo


#11

Take a note that output of git branch --no-color looks like:
$ git branch --no-color
0.9-stable
CHEF-1669-0.9-stable
CHEF-1956
CHEF-1956-0.9-stable
CHEF-2123
CHEF-2207

  • CHEF-2314
    master

Take a note that one of the lines marked as current branch. So we must use
not strict matching but regexp matching.

2011/5/6 Ringo De Smet ringo.desmet@gmail.com

Akzhan,

On 6 May 2011 15:59, Akzhan Abdulin akzhan.abdulin@gmail.com wrote:

Fixed by https://github.com/akzhan/chef/tree/CHEF-2314, ticket is
resolved
and passed to Adam Jacob.

As Mark indicated, wouldn’t the fix be easier if we fall back to
ary.include? instead?
http://www.ruby-doc.org/core/classes/Array.html#M000265

git(“branch --no-color”).stdout.lines.include?(branch_name)

Ringo