Braid interacting with files outside braided directory

[realityforge]

The latest version of braid (1.0.4) with the changes from @mattmccutchen has started to cause conflicts with files outside the braided directory. The scenario seems to come about when you have multiple braids into a single repository. So it is not uncommon for our projects to set up a braids for a set of projects such as;

• vendor/tools/tool1
• vendor/tools/tool2
• vendor/tools/tool3

And frequently when you delete several files from tool1 and then braid update vendor/tools/tool1 you will end up with conflicts in directories vendor/tools/tool2 or vendor/tools/tool3 as braid tries to delete them. (Presumably the hash collides?)

This only happens in braid (1.0.4) - @mattmccutchen do you have any ideas about what would be causing this and/or how to fix it?

[mattmccutchen]

No ideas off the top of my head. I'll be happy to investigate if you can provide specific steps to reproduce. I tried to recreate the scenario you described and the update worked fine.

[realityforge]

I believe the following script can reproduce it.

#!/bin/sh

rm -rf braid_test
mkdir braid_test

cd braid_test
echo "2.3.1" > .ruby-version
echo "source 'https://rubygems.org'" > Gemfile
echo "gem 'braid', '= 1.0.4'" >> Gemfile
bundle install

git init
git add .
git commit -m "Initial checking"

braid add --revision=ff580dcff88f909daaf558d0ed164c37ffac14d7 https://github.com/realityforge/domgen.git vendor/tools/domgen
braid add --revision=146c749ebc85b948be703a5d1fb4b95a6d15f5c9 https://github.com/realityforge/dbt.git vendor/tools/dbt
braid add --revision=199fee2f79e4636c84e3390a106fbd35249ea46a https://github.com/stocksoftware/way_of_stock.git vendor/docs/way_of_stock
braid add --revision=c10a6e92fa280a5613367e92eb47535479cee63d https://github.com/realityforge/buildr_plus.git vendor/tools/buildr_plus

git rm vendor/tools/buildr_plus/lib/buildr_plus/projects/rails.rb vendor/tools/buildr_plus/lib/buildr_plus/features/rails.rb vendor/tools/buildr_plus/lib/buildr_plus/features/itest.rb  vendor/tools/buildr_plus/lib/buildr_plus/features/calendar_date_select.rb
git commit -m "Test commit"

braid update --head vendor/tools/buildr_plus/ 

[mattmccutchen]

Thanks. I see, git's rename detection is unintentionally triggering because of the way braid sets up the trees to merge after my change (#38). Before #38, we used:

Tree	Mirror	Elsewhere
base	old upstream lib	local rest
local	local lib	local rest
remote	new upstream lib	local rest

After #38, we use:

Tree	Mirror	Elsewhere
base	old upstream lib	empty
local	local lib	local rest
remote	new upstream lib	empty

I thought it was a harmless optimization to skip reading the "local rest" content into the index when generating the base and remote trees. But if a file X in "old upstream lib" (in your example, vendor/tools/buildr_plus/lib/buildr_plus/features/calendar_date_select.rb) is deleted in "local lib", then git can falsely detect a rename from X to some unrelated but similar-looking file in "local rest" in the local tree (in your example, vendor/tools/domgen/lib/domgen/ruby/helper.rb).

I'll write a PR to read the "local rest" content into the base and remote trees, so we should have exactly the same merge semantics as before. The fix itself is 2 lines, but I want to do a little code cleanup and add a test.

[realityforge]

Great work. I released your code and most of my tests seem to work however the above script with version updated to 1.0.5 still produces a conflict that I can not understand. Any ideas?

[realityforge]

BTW Are you interested in coming onboard to help out with the project directly?

[mattmccutchen]

the above script with version updated to 1.0.5 still produces a conflict that I can not understand. Any ideas?

This was triggered by the recently pushed realityforge/buildr_plus@8cf5978 and is unrelated to the original issue; the behavior is the same with 1.0.3. Between realityforge/buildr_plus@8cf5978 and realityforge/buildr_plus@c10a6e9, lib/buildr_plus/projects/rails.rb was deleted and lib/buildr_plus/features/kinjen.rb was created; the files have the same long license header and very little code, so git falsely detects a rename based on the default 50% similarity threshold. You can even see this in the GitHub diff viewer. This rename conflicts with the local deletion of lib/buildr_plus/projects/rails.rb.

This has nothing to do with Braid: you would get the same conflict if you simply forked buildr_plus at c10a6e9, made the same git rm, and tried to merge. The only thing Braid could do better here is display the original error message from git merge-recursive:

CONFLICT (rename/delete): vendor/tools/buildr_plus/lib/buildr_plus/features/kinjen.rb deleted in HEAD and renamed in 8cf59781ca75297853c34ef3276824457cef1841. Version 8cf59781ca75297853c34ef3276824457cef1841 of vendor/tools/buildr_plus/lib/buildr_plus/features/kinjen.rb left in tree.

That message is still missing one piece of information: the filename that git thinks was renamed to vendor/tools/buildr_plus/lib/buildr_plus/features/kinjen.rb. Ideally git would include all relevant filenames in the message; I can request that as an enhancement on the git mailing list, but it will take me too long to do it myself and I don't know if anyone else will be motivated to do it. Absent such an enhancement, the user can find out the details of what git thinks happened by manually diffing the old and new revisions of the mirror and diffing the local project against the old revision with braid diff, as applicable.

[realityforge]

Righto. Thanks for the explanation.

I will reclose this issue then ;)

[mattmccutchen]

BTW:

Ideally git would include all relevant filenames in the message; I can request that as an enhancement on the git mailing list, but it will take me too long to do it myself and I don't know if anyone else will be motivated to do it.

I started on the implementation myself and it turned out to be not as hard as I expected, assuming the maintainers are satisfied with the way I wrote the test. Here's the thread.

[mattmccutchen]

BTW Are you interested in coming onboard to help out with the project directly?

I appreciate the invitation! I guess it depends on just what you mean by this. I'm not promising at this point to work on issues that don't interest me personally. If you'd like me to be able to make changes or merge other people's changes in the event you're unavailable, I'm open to that, but I certainly prefer to have your review.

[realityforge]

Mostly I am hoping you continue contributing to the project in whatever way works for you. So yes if you wanted to make changes directly or help review pull requests if I am unavailable then that is great. I will send off a message to the original owner of the project...