Full-history mirrors are broken: trying to use a tree as a commit

[mattmccutchen]

The change to support braiding of a subdirectory (#52) changed determine_target_revision to return a tree instead of a commit, even when the remote_path is empty. But the add and update commands in full-history mode still try to merge this object, which fails. Repro script for add:

git init
git config user.name 'Test user'
git config user.email 'test@example.com'
touch dummy
git add dummy
git commit -m initial
braid add --full https://github.com/cristibalan/braid/ braid

Output:

Initialized empty Git repository in /home/user/objsheets/tools/braid/test-repo/.git/
[master (root-commit) 28f646e] initial
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 dummy
Braid: Adding mirror of 'https://github.com/cristibalan/braid'.
Braid: Resetting to '28f646e'.
Braid: Shell error: error: d9180f39cceb5ff05c122ac8326afbf27980fe0e: expected commit type, but the object dereferences to tree type

At a minimum, one would like full-history mirrors with empty remote_path to work as they did before. I'm not sure if there's a reasonable way to implement full-history mirrors of a subdirectory.

We should probably add a test for full-history mode or otherwise remove the feature. (Not to mention a test with a nonempty remote_path.)

[realityforge]

Dang. I will have a look at it later in the week. Adding tests will help stop this happening in the future. I have some local changes to tests that I will push now and will try to add some more tests soonish.

I guess the other question is - do we care for full history mode. I have never used it and see little to no value in it as a feature. Is there any valid use case you can think of where it adds some value?

[mattmccutchen]

I am happy to see full-history mode go, although I don't think I have an airtight case for it. The only potential advantage I can think of is that it provides a convenient way to archive and distribute the upstream history of the commits actually used in the downstream project if:

1. one can't trust that those commits will remain available in the upstream repository for as long as anyone cares about the downstream project (i.e., the upstream repository could go away completely or the specific commits could be removed if the upstream project rewrites history), or
2. (edit) the upstream repository contains a huge amount of data irrelevant to the downstream project and cloning the entire thing is too inefficient. (Of course, for braid update, you have to download the new upstream commit, but Braid could in principle be enhanced to download only a specified upstream branch, or even just a specified upstream commit if the server allows that.)

In terms of operating on data after it has been downloaded, I don't see any advantage of having the histories tied together as opposed to designing tools/scripts to read the upstream commit pointer out of .braids.json.

Note also that braid add for full-history mirrors (though not braid update as far as we know) was broken for half a year because of --allow-unrelated-histories and nobody complained to us.

[realityforge]

Okay removed --full option and added some basic tests for all the other commands and released as 1.0.17. Holler if there is anything else you noticed

[mattmccutchen]

Have you thought about the compatibility implications? If someone tries to update an existing full-history mirror with the new Braid, will Braid just treat it as a squashed mirror and merge the content without merging the histories? To have that happen without any notice to the user is a little goofy, though I guess it has the same end result as if Braid raised an error and directed the user to remove the mirror and add it again as squashed. What if some users on a project are using the new Braid and others are using the old Braid, or are you assuming that doesn't happen?

[realityforge]

I am largely assuming that --full is for all intents and purposes unused. Anyone using it will have their repositories "upgraded" when they move to the next version of braid. As to using mixed versions of braid I tend to assume later versions are compatible with earlier versions while versa is not necessarily true.

[mattmccutchen]

I am largely assuming that --full is for all intents and purposes unused.

OK if you're willing to take this chance. I'd be pretty upset if I were a user with a valid reason for using full-history mode and discovered sometime later that it was no longer working, though we think such users are unlikely to exist. I probably would have gone for raising an error if there is any full-history mirror and instructing the user to run a special command (not shown in the normal help) such as braid make-all-squashed, ugly as that is; in software intended for serious use, we shouldn't expect to never have to maintain extra code for compatibility.

[mattmccutchen]

I probably would have gone for raising an error if there is any full-history mirror and instructing the user to run a special command (not shown in the normal help) such as braid make-all-squashed, ugly as that is

Going back to do this per comment.