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

wrong path in case of workdirs #1972

Closed
daschuer opened this issue Jul 11, 2021 · 12 comments · Fixed by #2252
Closed

wrong path in case of workdirs #1972

daschuer opened this issue Jul 11, 2021 · 12 comments · Fixed by #2252

Comments

@daschuer
Copy link
Contributor

When working with work dirs the conflict check is using the wrong path. This results in a weird error message during a commit with the base worktree in conflict.

[INFO] Checking merge-conflict files only.
An unexpected error has occurred: CalledProcessError: command: ('/usr/lib/git-core/git', 'diff', '--name-only', '--no-ext-diff', '-z', '-m', 'e8fb6f96dfe86b454ceb6913a3e02f88a9103565', 'HEAD', 'MERGE_HEAD')
return code: 128
expected return code: 0
stdout: (none)
stderr:
    fatal: mehrdeutiges Argument 'MERGE_HEAD': unbekannter Commit oder Pfad existiert nicht
    im Arbeitsverzeichnis
    Benutzen Sie '--', um Pfade und Commits zu trennen, ähnlich wie:
    'git <Befehl> [<Commit>...] -- [<Datei>...]'
    
Check the log at /home/sperry/.cache/pre-commit/pre-commit.log
@daschuer
Copy link
Contributor Author

It is a possible regression from #832 fixing #831

It looks like we need tow get_git_dir() functions one for the git dir for the regular usage and one for the common git dir used during install.

daschuer added a commit to daschuer/pre-commit that referenced this issue Jul 11, 2021
This fixes the conflicted state check when using work trees. pre-commit#1972
daschuer added a commit to daschuer/pre-commit that referenced this issue Jul 11, 2021
This fixes the conflicted state check when using work trees. pre-commit#1972
@asottile
Copy link
Member

you're missing: a reproduction of your error and a test in the pr demonstrating the change

@daschuer
Copy link
Contributor Author

The issue can reproduced like that:

  • create a second worktee from the default worktree that contains the common .git folder
  • create a conflicting merge in the origin worktee
  • don't resolve the conflicts
  • change to the other worktree folder
  • change a file
  • commit
  • the error message posted above appears.

I have no clue how to create a test for that. Can you help out here? Is there an other test that can be taken as a template?

@asottile
Copy link
Member

there's a handful of relevant similar tests in

def test_get_root_bare_worktree(tmpdir):
and https://github.com/pre-commit/pre-commit/blob/835f9c65e93cdaa517fb87a4cd42edb4f9b084a3/tests/staged_files_only_test.py

I believe the latter on is relevant to the error you posted above

@daschuer
Copy link
Contributor Author

daschuer commented Aug 6, 2021

The test is in place.

daschuer added a commit to daschuer/pre-commit that referenced this issue Aug 6, 2021
This fixes the conflicted state check when using work trees. pre-commit#1972
daschuer added a commit to daschuer/pre-commit that referenced this issue Aug 6, 2021
This fixes the conflicted state check when using work trees. pre-commit#1972
@daschuer
Copy link
Contributor Author

Since #1973 has been closed, is this issue solved?

@asottile
Copy link
Member

the pr was incomplete and stale

@daschuer
Copy link
Contributor Author

Do you have plans to fix the issue yourself?

@asottile
Copy link
Member

not really -- I don't use the feature

@daschuer
Copy link
Contributor Author

So we are back to the PR right? I think I need your help to fix. What is to do to put it into a merge-able state?

@daschuer
Copy link
Contributor Author

How to fix the failing CI?

@asottile
Copy link
Member

well the CI is green without your change so something you changed broke things -- you'll have to do some debugging to figure out why

daschuer added a commit to daschuer/pre-commit that referenced this issue Feb 17, 2022
This fixes the conflicted state check when using work trees. pre-commit#1972
asottile pushed a commit to daschuer/pre-commit that referenced this issue Apr 2, 2022
This fixes the conflicted state check when using work trees. pre-commit#1972
saravankrish pushed a commit to saravankrish/pre-commit that referenced this issue Jul 7, 2023
This fixes the conflicted state check when using work trees. pre-commit#1972
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants