Include nonconflicted files in root tree of file-conflict Git commits

[amiryal]

With the aim of better compatibility with tools that look for the state of working-copy files in the colocated Git’s HEAD, I propose to include all the would-be working copy files in the commit, in addition to the .jj-conflict-* trees that are already there for preventing GC.

This came up in a discussion about Nix flakes, and the way it breaks when HEAD@git has file conflicts. It was pointed out that:

Nix flakes actually use file contents from the working copy, but the index to determine which files to include.

Originally posted by @emilazy in https://github.com/martinvonz/jj/discussions/3970#discussioncomment-9878676

In other parts of the discussion, there has been reluctance towards updating Git’s index with the would-be working tree of HEAD@git (that is the parent of the jj working copy). When I suggested to instead change the way file-conflict commits are prepared in Git, it was pointed out that the barrier to do that is not high:

It used to be more friendly to colocated repos until commit 006c764694a2. I don't think there's much harm in changing it to better match the files in the working copy, as long as we also include the .jj-conflict-* subdirectories (they're needed in order to prevent GC).

Originally posted by @martinvonz in https://github.com/martinvonz/jj/discussions/3970#discussioncomment-9887040

[yuja]

If this is the issue of Git index, maybe we can add materialized file blobs to the index?

git::reset_head() currently resets Git index to the contents of the raw Git commit of the first parent. This wouldn't make sense if the working-copy commit was a merge or the parent had conflicts.

Of course, git commit shouldn't be used in that situation, which is probably the same for .jj-conflict-* trees.

[martinvonz]

Good point! I think you're right that the issue is only with the Git index, but maybe @amiryal can confirm.

I know we have a similar problem when the user uses git switch to update to a conflicted commit, but hopefully that doesn't confuse tooling in the same way.

[joyously]

Are the Git special cases scattered in the code, or in one place?

[ilyagr]

If this is the issue of Git index, maybe we can add materialized file blobs to the index?

I was thinking about this, and I'm conflicted about the idea.

On one hand, it would make some things a lot nicer, and I think it would solve the Nix issue. VS code might show a nicer diff for conflicted commits (I think, haven't tested it). But I'm worried about somebody hitting "Commit" in VS Code or another tool and really messing up their repo.

Currently, you need to add obviously messed up changes (removing all the .jj-conflict dirs) before you can do that. The mess being obvious and git diff looking scary makes the situation less dangerous.

I'm not sure just how bad this is: in theory, recovery is just a jj abandon away, but the user has to notice and understand the problem first.

Another idea: add the working copy to the staffing area, but don't remove the .jj-conflict dirs. This keeps the diff scary, which I think is good.

Also, any such change makes it harder to understand how jj affects the staging area now that the answer is not "never".

So, I wouldn't make this the default at least. I feel like there will be kinks.

[yuja]

but don't remove the .jj-conflict dirs.

Hmm, it might be also good to leave a README (with scalier name), or some fake conflict marker if git commit has a sanity check for that.

Also, any such change makes it harder to understand how jj affects the staging area now that the answer is not "never".

Just to be clear, jj new resets Git index, and my idea is to make it be consistent with the working directory contents materialized by jj new.

[martinvonz]

Just to be clear, jj new resets Git index, and my idea is to make it be consistent with the working directory contents materialized by jj new.

I agree with that, FWIW. I suspect it was just an oversight that we don't already do that.

[ilyagr]

or some fake conflict marker if git commit has a sanity check for that.

This is a great idea! I think any placeholder conflict marker would solve my worries. We could make the "README" itself a conflicted file (I don't know if this is a good idea, but it would look clever 😸 ).

In principle, the staging area is the one place Git understands conflicts, so with some work we might be able to actually represent all the conflicts correctly and let people resolve them with Git tools.

[amiryal]

the issue is only with the Git index, but maybe @amiryal can confirm.

Yes, when evaluating in a Git repository, Nix only considers files that are tracked by Git, and being in the index is considered as being tracked [ref].

[amiryal]

In principle, the staging area is the one place Git understands conflicts, so with some work we might be able to actually represent all the conflicts correctly and let people resolve them with Git tools.

Genius! I will work this into the issue description when I get to it, or open a new one with just this proposal.

[ilyagr]

In principle, the staging area is the one place Git understands conflicts, so with some work we might be able to actually represent all the conflicts correctly and let people resolve them with Git tools.

Genius! I will work this into the issue description when I get to it, or open a new one with just this proposal.

I thought about this a bit more, and I think this should definitely be step 2 of the plan. It would require at least one additional change to jj's Git integration, and there will be some decisions to make.

In Git, normally, the way merge conflicts are resolved is by modifying the staging area AFAIU. Currently, jj ignores any changes to the index. If we wanted jj to see the result of Git merge tools, this would have to change: jj would have to look out for changes to the index.

Now, the index is a diff on top of the parent commit AKA the Git HEAD, not the working copy commit. There's no obvious place in jj's model for jj to record any changes to the index that it detects. Two possibilities come to mind: jj could either turn changes to the index into a new commit in between the working copy and the parent, or it could squash the changes into the parent commit. Both behaviors would lead to some slight weirdness, I think.

---

Meanwhile, I still see no problem with the simpler "step 1" idea: adjust the index to jj's representation of the parent commit, and create a placeholder conflict to prevent the user from accidentally committing. The conflicted file could be named .jj-do-not-resolve-this-conflict or something. In this "step 1" scenario, jj ignores any changes to the index, just as it does now.

[martinvonz]

Just a quick not that I don't think Git's index supports more than 4 sides, but I may easily misremember that

[ilyagr]

I think it only supports what we call 2-sided conflicts with 1 base ("ours", "theirs", and "ancestor" in Git parlance):

https://docs.rs/git2/latest/git2/struct.IndexConflict.html