Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge conflicts on update disappear in git mergetool or some IDEs #1833

Open
borispf opened this issue Oct 24, 2024 · 6 comments · May be fixed by #1907
Open

Merge conflicts on update disappear in git mergetool or some IDEs #1833

borispf opened this issue Oct 24, 2024 · 6 comments · May be fixed by #1907
Labels
bug triage Trying to make sure if this is valid or not

Comments

@borispf
Copy link

borispf commented Oct 24, 2024

Describe the problem

When running copier update with inline conflict markers, if there is a conflict the file in the worktree will have conflict markers as expected, git status will report the file as both modified, but if you open the file with git mergetool, or the built-in merge editors in VS Code or PyCharm the conflict has disappeared and only the local change is shown form all three-files in the three-way merge.

I believe the problem was introduced in #1396, and is related to the index having the same version of the file three times.

Template

The problem occurred with a work-internal template that I unfortunately cannot share, but I believe I have recreated the issue in the copier test suite.

To Reproduce

  1. git checkout 6bba7614a32a4023f11d8170dd97232b3006d5be
  2. Modify tests/test_updatediff.py::test_conflicted_files_are_marked_unmerged, appending:
        print(git("ls-files", "--stage"))
        print(git("cat-file", "-p", "f163f4b09aad0c495f8f58112ea4aabc9900afe7"))
  1. Run that one test with pytest raw output enabled:
PYTEST_XDIST_AUTO_NUM_WORKERS=0 poetry run pytest -s 'tests/test_updatediff.py::test_conflicted_files_are_marked_unmerged[README.md]'

Logs

[11:58]PYTEST_XDIST_AUTO_NUM_WORKERS=0 poetry run pytest -s 'tests/test_updatediff.py::test_conflicted_files_are_marked_unmerged[README.md]'
========================================================================================= test session starts =========================================================================================
platform darwin -- Python 3.12.7, pytest-8.3.3, pluggy-1.5.0
rootdir: /Users/P00922026/src/gh/copier-org/copier
configfile: pyproject.toml
plugins: cov-5.0.0, gitconfig-0.7.0, xdist-3.6.1
collected 1 item

tests/test_updatediff.py
Copying from template version 1
    create  README.md
    create  .copier-answers.yml


Updating to template version 2
100644 5b0fbcceaec7a16ecde36f5ea5121720b26409c4 0	.copier-answers.yml
100644 f163f4b09aad0c495f8f58112ea4aabc9900afe7 1	README.md
100644 f163f4b09aad0c495f8f58112ea4aabc9900afe7 2	README.md
100644 f163f4b09aad0c495f8f58112ea4aabc9900afe7 3	README.md

upstream version 1 + downstream
.

Expected behavior

All three versions of README.md in git ls-files --stage should have different git hashes.

Screenshots/screencasts/logs

No response

Operating system

macOS

Operating system distribution and version

Sonoma 14.6.1 (23G93)

Copier version

6bba761

Python version

CPython 3.12.7

Installation method

local build

Additional context

No response

@borispf borispf added bug triage Trying to make sure if this is valid or not labels Oct 24, 2024
@pawamoy
Copy link
Contributor

pawamoy commented Oct 24, 2024

the conflict has disappeared and only the local change is shown form all three-files in the three-way merge

Do you mean that there isn't any conflict marker in the file anymore? If that's the case, I believe we have fixed this in #1813.

About the file hashes:

100644 f163f4b09aad0c495f8f58112ea4aabc9900afe7 1	README.md
100644 f163f4b09aad0c495f8f58112ea4aabc9900afe7 2	README.md
100644 f163f4b09aad0c495f8f58112ea4aabc9900afe7 3	README.md

It's possible that I took a shortcut. I'm not particularly knowledgeable of Git's internals, so I did the easy thing and just updated the index with all three staging modes using the same hash. Should we instead hash three different temporary files, one for each possible conflict resolution?

@pawamoy
Copy link
Contributor

pawamoy commented Oct 24, 2024

Seeing that you point at a commit from yesterday as your Copier version, I worry the fix I mention has no effect on your issue 😅 Waiting for your confirmation. However I'm not sure to fully understand your issue yet.

@borispf
Copy link
Author

borispf commented Oct 25, 2024

Yeah, rereading it was very unclear. The conflict markers are there in the file, but git mergetool or the vscode merge editor I think use that hashes you see with git ls-files --stage to know what the original files were in the merge. And so with all the hashes the same the conflict disappears (screenshot at the end shows the problem).

I've tried to make a clear reproduction using a test template where only one file (version.txt) changes between versions: https://github.com/borispf/test-template

# In a new directory, copy v1.0 of the template.
git init
copier copy --vcs-ref 1.0 https://github.com/borispf/test-template.git .
git add .
git commit -m v1

# Make a conflicting change.
echo version 2 > version.txt
git add .
git commit -m v2

# Then do a template update to generate the conflict
copier update

Then some relevant output:

# version.txt correctly shown as both modified.
$ git status
On branch main
Unmerged paths:
  (use "git restore --staged <file>..." to unstage)
  (use "git add <file>..." to mark resolution)
	both modified:   version.txt

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   .copier-answers.yml

no changes added to commit (use "git add" and/or "git commit -a")

# version.txt has the expected conflict markers.
$ cat version.txt
<<<<<<< before updating
version 2
=======
version 1.1
>>>>>>> after updating

# ls-files shows that version.txt has different versions, but the they all have the same sha1?
$ git ls-files --stage
100644 10f1cf1b6e306695e7572a28faa7077bcadbfcd1 0	.copier-answers.yml
100644 1f7a7a472abf3dd9643fd615f6da379c4acb3e3a 1	version.txt
100644 1f7a7a472abf3dd9643fd615f6da379c4acb3e3a 2	version.txt
100644 1f7a7a472abf3dd9643fd615f6da379c4acb3e3a 3	version.txt

# seems like git thinks all there files are just the v2.
$ git cat-file -p 1f7a7a472abf3dd9643fd615f6da379c4acb3e3a
version 2

And now in VSCode's merge editor you can see the issue:

Screenshot 2024-10-25 at 22 19 45

Click on Merge Editor bottom right

All of the changes or merge conflicts are gone.

Screenshot 2024-10-25 at 22 19 50

Similar in vimdiff, all of the source files are the same, at least the result still shows the merge conflict:

Screenshot 2024-10-25 at 22 47 46

This is the repo zipped up if it helps: test.zip.

I hope that's a little clearer.

@pawamoy
Copy link
Contributor

pawamoy commented Oct 25, 2024

That's much clearer, thanks.

I never use VSCode's "Merge Editor", only the "Source Control" tab.

Not sure if that's fixable. We could try and set the right hashes for each mode, but that doesn't mean the files would be there for merge tools to see them? My Git-fu is lacking here 🤷

@pawamoy
Copy link
Contributor

pawamoy commented Dec 26, 2024

Confirming the issue in a different way: after a copier update, my README.md looks like this:


<<<<<<< before updating
=======
[![documentation](https://img.shields.io/badge/docs-mkdocs-708FCC.svg?style=flat)](https://pawamoy.github.io/pawamoy-testing/)
[![gitter](https://badges.gitter.im/join%20chat.svg)](https://app.gitter.im/#/room/#pawamoy-testing:gitter.im)

Testing this great template

## Installation

This project is available to sponsors only, through my Insiders program.
See Insiders [explanation](https://pawamoy.github.io/pawamoy-testing/insiders/)
and [installation instructions](https://pawamoy.github.io/pawamoy-testing/insiders/installation/).
>>>>>>> after updating

Yet, whether I run git checkout --ours README.md or git checkout --theirs README.md, all I get is an empty file, which corresponds to the contents before the update (the "ours" contents).

Coming back to this:

Should we instead hash three different temporary files, one for each possible conflict resolution?

...and looking at the code, the 3-way merge is done like this:

git(
    "merge-file",
    "-L",
    "before updating",
    "-L",
    "last update",
    "-L",
    "after updating",
    fname,
    Path(old_copy) / fname,
    Path(new_copy) / fname,
    retcode=None,
)

...so we do have files on the disk, and wouldn't need to re-create temporary ones. Maybe I just have to use these files (Path(old_copy) / fname and Path(new_copy) / fname) to get proper hashes, which will help Git actually understand the situation?

EDIT: the hashes are blob ids, not file hashes. Not sure how to retrieve them from Git, if they even exist. Posted this SO question: https://stackoverflow.com/questions/79309642/git-checkout-ours-theirs-after-git-merge-files-with-external-files.

@pawamoy
Copy link
Contributor

pawamoy commented Dec 26, 2024

Got a fantastic answer on SO again, sending a PR 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug triage Trying to make sure if this is valid or not
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants