Different git tag refs for same cookbook creates non-unique cookbooks

[sjtindell]

I still haven't tracked down exactly what is happening here, but I believe this is a use case for Batali and therefore a bug.

I have the following internal repos, names shortened

githost/sam.tindell/batali-proj
|_ Batali
githost/sam.tindell/batali-cookbook
|_ metadata.rb

The Batali file has the following contents

Batali.define do
  source 'https://supermarket.chef.io'

  cookbook 'batali-cookbook', :git => 'git@githost.nevint.com:sam.tindell/batali-cookbook.git', :ref => 'v0.0.1'
  cookbook 'batali-cookbook', :git => 'git@githost.nevint.com:sam.tindell/batali-cookbook.git', :ref => 'v0.0.2'

  discover true
end

And the batali-cookbook repo metdata file has the following contents, shortened

name             'batali-cookbook'
version          '0.0.1'
depends 'sudo'

We create a tag in batali-cookbook for v0.0.1, then bump the metdata version to 0.0.2 and create a tag v0.0.2, both tags off the master branch. In git these tags point go two separate refs.

As I understand the usage of batali, I should be able to specify these two tags in the Batali file, and then in the cookbooks directory after executing batali install -I I should get cookbooks created as follows

[batali-test/batali-proj]-> ls cookbooks
batali-cookbook-0.0.1 batali-cookbook-0.0.2 sudo-5.4.0

This works, the cookbooks are there with different names, but the tags have different refs and should have different contents in their metdata.rb files in each generated cookbook dir.

When we use Threads in the lib/batali/command/install.rb execute function, the contents of the cookbooks is identical. This has been tested with repos from Gitlab, Github, and various directory structures.

I see in the batali cache under ~/.batali/cache/git/git the two different refs are there, and they contain the different contents I want.

But when batali actually does the copy for install, rather than having different contents, batali is putting the same ref contents in both cookbook version dirs under ./cookbooks.

Here is some example output.

Notice here that the Ref for the two different versions of the batali-cookbook is the same

[batali-test/batali-proj]-> rm -rf cookbooks; rm -rf ~/.batali; rm batali.manifest; batali resolve -I; batali install -I --debug
[Batali]: Loading sources... complete!
[Batali]: Performing infrastructure path resolution.
[Batali]: Resolving dependency constraints... complete!
[Batali]: Writing infrastructure manifest file... complete!
[Batali]: Infrastructure manifest solution:
batali-cookbook <0.0.1, 0.0.2>
sudo <5.4.0>
[Batali]: Readying installation destination... complete!
[Batali]: Installing cookbooks... 
[DEBUG]: Thread: 70142260907860
UnitInspect: <Batali::Unit:70142256724900 [{"name"=>"sudo", "dependencies"=>[], "version"=>#<Batali::UnitVersion "5.4.0">, "source"=><Batali::Source::Site:70142256441580 [{"type"=>"Batali::Source::Site", "url"=>"https://supermarket.chef.io:443/api/v1/cookbooks/sudo/versions/5.4.0/download", "version"=>"5.4.0"}]>}]>
UnitSource: <Batali::Source::Site:70142256441580>
AssetPath: /Users/sam.tindell/.batali/cache/site/remote_site/aHR0cHM6Ly9zdXBlcm1hcmtldC5jaGVmLmlvOjQ0My9hcGkvdjEvY29va2Jvb2tzL3N1ZG8vdmVyc2lvbnMvNS40LjAvZG93bmxvYWQ=/sudo
FinalPath: cookbooks/sudo-5.4.0

[DEBUG]: Thread: 70142260908040
UnitInspect: <Batali::Unit:70142256751260 [{"name"=>"batali-cookbook", "dependencies"=>[<Batali::UnitDependency type=:runtime name="sudo" requirements="> 0">], "version"=>#<Batali::UnitVersion "**0.0.1**">, "source"=><Batali::Source::Git:70142285328780 [{"type"=>"Batali::Source::Git", "url"=>"git@githost.nevint.com:sam.tindell/batali-cookbook.git", "ref"=>"**96977253ff672da5cef92ce7a7cebc30f8e12cea**", "subdirectory"=>nil, "path"=>"/Users/sam.tindell/.batali/cache/git/git/96977253ff672da5cef92ce7a7cebc30f8e12cea"}]>}]>
UnitSource: <Batali::Source::Git:70142285328780>
AssetPath: /var/folders/bz/kk0v4vb54958z6p25n4xfk7h0000gq/T/d20180627-33337-1pkz6uf
FinalPath: cookbooks/batali-cookbook-0.0.1

[DEBUG]: Thread: 70142260908360
UnitInspect: <Batali::Unit:70142262074580 [{"name"=>"batali-cookbook", "dependencies"=>[<Batali::UnitDependency type=:runtime name="sudo" requirements="> 0">], "version"=>#<Batali::UnitVersion "0.0.2">, "source"=><Batali::Source::Git:70142256754800 [{"type"=>"Batali::Source::Git", "url"=>"git@githost.nevint.com:sam.tindell/batali-cookbook.git", "ref"=>"**96977253ff672da5cef92ce7a7cebc30f8e12cea**", "subdirectory"=>nil, "path"=>"/Users/sam.tindell/.batali/cache/git/git/96977253ff672da5cef92ce7a7cebc30f8e12cea"}]>}]>
UnitSource: <Batali::Source::Git:70142256754800>
AssetPath: /var/folders/bz/kk0v4vb54958z6p25n4xfk7h0000gq/T/d20180627-33337-1glr0al
FinalPath: cookbooks/batali-cookbook-0.0.2
complete!

Different folders are being created under /var/folders, but the contents of them is the same.

When we remove the Thread.new do and end.map(&:join) block wrapping the install code and let it run in the single main thread, we see the following where the refs are different as they should be.

[batali-test/batali-proj]-> rm -rf cookbooks; rm -rf ~/.batali; rm batali.manifest; batali resolve -I; batali install -I --debug
[Batali]: Loading sources... complete!
[Batali]: Performing infrastructure path resolution.
[Batali]: Resolving dependency constraints... complete!
[Batali]: Writing infrastructure manifest file... complete!
[Batali]: Infrastructure manifest solution:
batali-cookbook <0.0.1, 0.0.2>
sudo <5.4.0>
[Batali]: Readying installation destination... complete!
[Batali]: Installing cookbooks... 
[DEBUG]: Thread: 70315678054060
UnitInspect: <Batali::Unit:70315674507560 [{"name"=>"batali-cookbook", "dependencies"=>[<Batali::UnitDependency type=:runtime name="sudo" requirements="> 0">], "version"=>#<Batali::UnitVersion **"0.0.2**">, "source"=><Batali::Source::Git:70315698845260 [{"type"=>"Batali::Source::Git", "url"=>"git@githost.nevint.com:sam.tindell/batali-cookbook.git", "ref"=>"**96977253ff672da5cef92ce7a7cebc30f8e12cea**", "subdirectory"=>nil, "path"=>"/Users/sam.tindell/.batali/cache/git/git/96977253ff672da5cef92ce7a7cebc30f8e12cea"}]>}]>
UnitSource: <Batali::Source::Git:70315698845260>
AssetPath: /var/folders/bz/kk0v4vb54958z6p25n4xfk7h0000gq/T/d20180627-33044-197oxgw
FinalPath: cookbooks/batali-cookbook-0.0.2

[DEBUG]: Thread: 70315678054060
UnitInspect: <Batali::Unit:70315698841700 [{"name"=>"batali-cookbook", "dependencies"=>[<Batali::UnitDependency type=:runtime name="sudo" requirements="> 0">], "version"=>#<Batali::UnitVersion "**0.0.1**">, "source"=><Batali::Source::Git:70315675097840 [{"type"=>"Batali::Source::Git", "url"=>"git@githost.nevint.com:sam.tindell/batali-cookbook.git", "ref"=>"**89604365d27a67a753f833eef8e30da08d9a3b26**", "subdirectory"=>nil, "path"=>"/Users/sam.tindell/.batali/cache/git/git/89604365d27a67a753f833eef8e30da08d9a3b26"}]>}]>
UnitSource: <Batali::Source::Git:70315675097840>
AssetPath: /var/folders/bz/kk0v4vb54958z6p25n4xfk7h0000gq/T/d20180627-33044-pl92a1
FinalPath: cookbooks/batali-cookbook-0.0.1

[DEBUG]: Thread: 70315678054060
UnitInspect: <Batali::Unit:70315675091780 [{"name"=>"sudo", "dependencies"=>[], "version"=>#<Batali::UnitVersion "5.4.0">, "source"=><Batali::Source::Site:70315685743140 [{"type"=>"Batali::Source::Site", "url"=>"https://supermarket.chef.io:443/api/v1/cookbooks/sudo/versions/5.4.0/download", "version"=>"5.4.0"}]>}]>
UnitSource: <Batali::Source::Site:70315685743140>
AssetPath: /Users/sam.tindell/.batali/cache/site/remote_site/aHR0cHM6Ly9zdXBlcm1hcmtldC5jaGVmLmlvOjQ0My9hcGkvdjEvY29va2Jvb2tzL3N1ZG8vdmVyc2lvbnMvNS40LjAvZG93bmxvYWQ=/sudo
FinalPath: cookbooks/sudo-5.4.0
complete!

What is going wrong here? Is it the threading or the end.map.(&:join) operation? I do not know enough Ruby to track it down fully.

[sjtindell]

If we change lib/batali/git.rb
self.ref = git.log.first.sha
to
self.ref = ref

It works. This indicates a possible problem with threads trying to set the git log sha here via checkout and running into each other? Or perhaps the first sha in the log doesn't change at the right time?

Is there any possible negative side effect of using the ref, which in this case gets set to a tag e.g. v0.0.1 or v0.0.2?

[chrisroberts]

@sjtindell Hi and thanks for the report! I believe the modification in #105 will resolve the issue. If you are able, can you give it a spin and see if it fixes things up for you?

[sjtindell]

Thanks Chris, I appreciate the timely response. But still seeing the same issue with the test repos I posted above.

[sjtindell]

When I add the following to ref_dup in lib/batali/git.rb

    def ref_dup
      git = ::Git.open(base_path)
      git.checkout(ref)
      git.pull("origin", ref)
      puts("#{Thread.current.object_id} RefDup_ref: #{ref}")
      puts("#{Thread.current.object_id} RefDup_sha: #{git.log.first.sha}")
      self.ref = git.log.first.sha
      self.path = Utility.join_path(cache_path, "git", ref)
      unless File.directory?(path)
        FileUtils.mkdir_p(path)
        FileUtils.cp_r(Utility.join_path(base_path, "."), path)
        FileUtils.rm_rf(Utility.join_path(path, ".git"))
      end
      path
    end

I see the following on execution. The function is being called many times, and the two refs are correct on the first outputs from each thread, but then there are more calls and the threads are still getting the wrong ref.

[batali-test/batali-proj]-> rm -rf cookbooks; rm -rf ~/.batali; rm batali.manifest; batali resolve -I; batali install -I --debug
[Batali]: Loading sources... 70221658535620 RefDup_ref: v0.0.1
70221658535620 RefDup_sha: 89604365d27a67a753f833eef8e30da08d9a3b26
70221658535620 RefDup_ref: v0.0.2
70221658535620 RefDup_sha: 96977253ff672da5cef92ce7a7cebc30f8e12cea
complete!
[Batali]: Performing infrastructure path resolution.
[Batali]: Resolving dependency constraints... complete!
[Batali]: Writing infrastructure manifest file... complete!
[Batali]: Infrastructure manifest solution:
batali-cookbook <0.0.1, 0.0.2>
sudo <5.4.0>
[Batali]: Readying installation destination... complete!
[Batali]: Installing cookbooks... [DEBUG]: Starting unit install for: sudo<5.4.0>
[DEBUG]: Starting unit install for: batali-cookbook<0.0.2>
[DEBUG]: Starting unit install for: batali-cookbook<0.0.1>
[DEBUG]: Cache directory to persist cookbooks: /Users/sam.tindell/.batali/cache
[DEBUG]: Cache directory to persist cookbooks: /Users/sam.tindell/.batali/cache
[DEBUG]: Cache directory to persist cookbooks: /Users/sam.tindell/.batali/cache
[DEBUG]: Completed unit install for: sudo<5.4.0>
70159852808140 RefDup_ref: 96977253ff672da5cef92ce7a7cebc30f8e12cea
70159852807920 RefDup_ref: 89604365d27a67a753f833eef8e30da08d9a3b26
70159852808140 RefDup_sha: 96977253ff672da5cef92ce7a7cebc30f8e12cea
70159852807920 RefDup_sha: 96977253ff672da5cef92ce7a7cebc30f8e12cea
70159852808140 RefDup_ref: 96977253ff672da5cef92ce7a7cebc30f8e12cea
70159852807920 RefDup_ref: 96977253ff672da5cef92ce7a7cebc30f8e12cea
70159852808140 RefDup_sha: 96977253ff672da5cef92ce7a7cebc30f8e12cea
70159852807920 RefDup_sha: 96977253ff672da5cef92ce7a7cebc30f8e12cea
[DEBUG]: Completed unit install for: batali-cookbook<0.0.2>
[DEBUG]: Completed unit install for: batali-cookbook<0.0.1>
complete!
[batali-test/batali-proj]->
[batali-test/batali-proj]-> python test_metadata_match.py cookbooks
batali-cookbook-0.0.1 Mismatch! Exiting...

[sjtindell]

I created a PR to your fix branch, it appears to work if we use a class variable for the lock.

#106