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

Bug fix for .git submodule files #7

Closed
wants to merge 1 commit into from

Conversation

kirtfitzpatrick
Copy link
Collaborator

No description provided.

@@ -43,8 +43,10 @@ def validate_changes_on_the_server_not_in_the_repo
path = cookbook_path.join("#{manifest_record["path"]}")

if path.exist?
checksum = checksum_cookbook_file(path)
messages << "#{manifest_record['path']}" if checksum != manifest_record['checksum']
if path.basename.to_s != '.git'
Copy link
Contributor

Choose a reason for hiding this comment

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

why not make the first if a compound condition test a là if path.exist? && path.basename.to_s != '.git'? </nit>

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't know how to reproduce that submodule bug, but if you move the condition you end up in the else branch and it's probably not what you want.

In that case I would use a guard clause instead:

if path.exist?
  next if path.basename.to_s == '.git'

  checksum = checksum_cookbook_file(path)
  messages << "#{manifest_record['path']}" if checksum != manifest_record['checksum']
else

@gregkare
Copy link
Collaborator

gregkare commented Jan 6, 2014

@kirtfitzpatrick Hey, can you give me more information about that submodule bug, like a log? Thanks!

@jonlives
Copy link

@gregkare I think we might be running into this too, we're seeing the following in a submodule:

ERROR: Errno::EISDIR: Is a directory - /var/hudson/.hudson/workspace/chef-server-git-sync/cookbooks/foo/.git

Oddly though, it's only recently started happening - we've had that submodule ever since we've been using knife-inspect.

@gregkare
Copy link
Collaborator

@jonlives I'll take a look at this tonight

@jonlives
Copy link

Thank y'kindly sir!

@gregkare
Copy link
Collaborator

Oops, I completely forgot about this, sorry! I will look at it this week-end.

@gregkare
Copy link
Collaborator

gregkare commented Aug 2, 2014

@jonlives Hey, I'm really sorry I took so long! Can you try https://github.com/bmarini/knife-inspect/tree/submodule_bug and tell me if it fixes your issue? I haven't been able to reproduce it yet so it's a stab in the dark...

@jonlives
Copy link

@gregkare that looks like it does the trick, thanks muchly!

@gregkare
Copy link
Collaborator

@jonlives Perfect, I'll release a new version of the gem with that fix then

@jonlives
Copy link

Lovely stuff.

@gregkare gregkare closed this in fd81c6d Aug 11, 2014
@gregkare
Copy link
Collaborator

I've released 0.9.0 including this fix: https://rubygems.org/gems/knife-inspect/versions/0.9.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants