Initial stab at enabling git submodule support in between meals

[irabinovitch]

This is an initial stab at enabling git submodule support in between meals. Still doing a bit more testing but wanted to start the PR to get any feedback/concerns.

[odcinek]

Why?

[irabinovitch]

To handle the case of a path being removed and re-added as a submodule. Before reversing the order it was only picking up the delete, and not the addition.

[odcinek]

Can you provide an example git output? And incorporate that into the test cases? I've got a feeling this fix is incidental and possibly breaking other stuff.

[odcinek]

Looks sane. Could you squash into single commit?

[irabinovitch]

I believe these tests are broken on master as well.

[odcinek]

Nope, they are not. .reverse breaks stuff.

[irabinovitch]

Sorry for the delay was at ChefConf last week.

When you move an existing cookbook to a submodule the git history looks like:

A       .gitmodules
A       chef/cookbooks/foo
D       chef/cookbooks/foo/metadata.rb
D       chef/cookbooks/foo/recipes/default.rb
D       chef/cookbooks/foo/README.md

Between_Meals does detect all of the events and lists them properly in the Changes hash. However the above sequence results in a delete of the foo cookbook from the chef server, not an add/update

 DEBUG: [cookbook] chef/cookbooks/foo meaningful? [(?-mix:^chef\/cookbooks\/([^\/]+)\/.*)]:  

This is due to:

• the change list deletes original metadata.rb but does not
• meaningful_cookbook_file? returns false for chef/cookbooks/foo as it does not match against the regex in lib/between_meals/changes/cookbook.rb).

While the current hacks (reverse and relaxed regex) do hide this issue for our use case, they're not ideal as you've pointed out. My current thought is to add a gd_revision file for each submodule.

[jaymzh]

Meanwhile if you move a cookbook, it'll show up as:

D chef/cookbooks/one/foo/metadata.rb
...
A chef/cookbooks/two/foo/metadata.rb
...

So not only are they sub-optimal, they'll break the common case. But moreover, we are doing all deletes before adds:

https://github.com/facebook/grocery-delivery/blob/master/bin/grocery-delivery#L130

So the problem here actually isn't the order (I'm confused as to how the .reverse even helps), it seems like the real problem is that we're not recognizing the add at all... is there a more verbose option to git log that will show the submodule changes in more detail?

[odcinek]

@irabinovitch - this might be what you want

$ git submodule foreach git status --porcelain
Entering 'foo'
 A       metadata.rb
 A       recipes/default.rb
 A       README.md

somewhere around
https://github.com/facebook/between-meals/blob/master/lib/between_meals/repo/git.rb#L115

[odcinek]

afaik the ordering of output is crucial here.

all the delete actions must go first. rename is a delete and add, in that order. moving a cookbook to a submodule should not be any different - explicit delete, followed by an add.

[jaymzh]

@irabinovitch ?

[jaymzh]

Pingaling?

[jaymzh]

OK, closing after 3 months of no activity. If you get time to work on this again, please feel free to re-open.