From fdbdf64d87e2980e8399d1182ccb43bd0f013b16 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Tue, 11 Sep 2018 15:53:48 -0400 Subject: [PATCH] fix reversions due to undocumented and buggy git behavior * Don't use GIT_PREFIX when GIT_WORK_TREE=. because it seems git does not intend GIT_WORK_TREE to be relative to GIT_PREFIX in that case, despite GIT_WORK_TREE=.. being relative to GIT_PREFIX. * Don't use GIT_PREFIX to fix up a relative GIT_DIR, because git 2.11 sets GIT_PREFIX set to a path it's not relative to. and apparently GIT_DIR is never relative to GIT_PREFIX. Commit e50ed4ba48f93cf0addb3638a4a9605a10f17976 led us down this path by working around a git bug by relying on the barely documented GIT_PREFIX. This commit was sponsored by Trenton Cronholm on Patreon. --- CHANGELOG | 6 ++ Git/CurrentRepo.hs | 23 ++++---- ...____44___git_commit___34__blows__34__.mdwn | 2 + ..._eaafc970cce2c71ce29f136c90e04504._comment | 57 +++++++++++++++++++ 4 files changed, 78 insertions(+), 10 deletions(-) create mode 100644 doc/bugs/v6_-_under_subdir__58___git_add___34__whines__34____44___git_commit___34__blows__34__/comment_5_eaafc970cce2c71ce29f136c90e04504._comment diff --git a/CHANGELOG b/CHANGELOG index 9368f35140..4c65dbb982 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -31,6 +31,12 @@ git-annex (6.20180808) UNRELEASED; urgency=medium is configured with exporttree=yes versioning=yes, git-annex can download past versions of exported files from it. * S3: Support AWS_SESSION_TOKEN. + * Don't use GIT_PREFIX when GIT_WORK_TREE=. because it seems git + does not intend GIT_WORK_TREE to be relative to GIT_PREFIX in that + case, despite GIT_WORK_TREE=.. being relative to GIT_PREFIX. + * Don't use GIT_PREFIX to fix up a relative GIT_DIR, because + git 2.11 sets GIT_PREFIX set to a path it's not relative to. + and apparently GIT_DIR is never relative to GIT_PREFIX. * git-annex.cabal: Fix build without assistant, and some other refinements. Thanks fftehnik. diff --git a/Git/CurrentRepo.hs b/Git/CurrentRepo.hs index 2eca12fa4b..82fd0c6429 100644 --- a/Git/CurrentRepo.hs +++ b/Git/CurrentRepo.hs @@ -28,17 +28,17 @@ import Utility.Env.Set - - Also works around a git bug when running some hooks. It - runs the hooks in the top of the repository, but if GIT_WORK_TREE - - was relative, it then points to the wrong directory. In this situation - - GIT_PREFIX contains the directory that GIT_WORK_TREE (and GIT_DIR) - - are relative to. + - was relative (but not "."), it then points to the wrong directory. + - In this situation GIT_PREFIX contains the directory that + - GIT_WORK_TREE is relative to. -} get :: IO Repo get = do - prefix <- getpathenv "GIT_PREFIX" - gd <- pathenv "GIT_DIR" prefix + gd <- getpathenv "GIT_DIR" r <- configure gd =<< fromCwd + prefix <- getpathenv "GIT_PREFIX" wt <- maybe (worktree $ location r) Just - <$> pathenv "GIT_WORK_TREE" prefix + <$> getpathenvprefix "GIT_WORK_TREE" prefix case wt of Nothing -> return r Just d -> do @@ -55,10 +55,13 @@ get = do return (Just d) Nothing -> return Nothing - pathenv s Nothing = getpathenv s - pathenv s (Just prefix) = getpathenv s >>= \case - Nothing -> return Nothing - Just d -> Just <$> absPath (prefix d) + getpathenvprefix s (Just prefix) | not (null prefix) = + getpathenv s >>= \case + Nothing -> return Nothing + Just d + | d == "." -> return (Just d) + | otherwise -> Just <$> absPath (prefix d) + getpathenvprefix s _ = getpathenv s configure Nothing (Just r) = Git.Config.read r configure (Just d) _ = do diff --git a/doc/bugs/v6_-_under_subdir__58___git_add___34__whines__34____44___git_commit___34__blows__34__.mdwn b/doc/bugs/v6_-_under_subdir__58___git_add___34__whines__34____44___git_commit___34__blows__34__.mdwn index 74209966ee..3e29ab0689 100644 --- a/doc/bugs/v6_-_under_subdir__58___git_add___34__whines__34____44___git_commit___34__blows__34__.mdwn +++ b/doc/bugs/v6_-_under_subdir__58___git_add___34__whines__34____44___git_commit___34__blows__34__.mdwn @@ -47,3 +47,5 @@ git-annex version: 6.20180807+git230-gaa291acfe-1~ndall+1 also happens with 6.20180807-1 but not with 6.20170101-1+deb9u2, so it is a regression [[!meta author=yoh]] + +> [[fixed|done]] --[[Joey]] diff --git a/doc/bugs/v6_-_under_subdir__58___git_add___34__whines__34____44___git_commit___34__blows__34__/comment_5_eaafc970cce2c71ce29f136c90e04504._comment b/doc/bugs/v6_-_under_subdir__58___git_add___34__whines__34____44___git_commit___34__blows__34__/comment_5_eaafc970cce2c71ce29f136c90e04504._comment new file mode 100644 index 0000000000..acb36873f3 --- /dev/null +++ b/doc/bugs/v6_-_under_subdir__58___git_add___34__whines__34____44___git_commit___34__blows__34__/comment_5_eaafc970cce2c71ce29f136c90e04504._comment @@ -0,0 +1,57 @@ +[[!comment format=mdwn + username="joey" + subject="""comment 5""" + date="2018-09-11T18:51:07Z" + content=""" +Unfortunately git 2.11.0 has the bug that I made git-annex look at +`GIT_PREFIX` to avoid. + + root@darkstar:/tmp/repo/foo/bar# git --work-tree=../.. ls-files --modified /tmp/repo/ + BASH_EXECUTION_STRING='set | grep GIT_ >&2; pwd >&2; git-annex smudge --clean '\''subdir/file'\''' + GIT_DIR=/tmp/repo/.git + GIT_PREFIX=foo/bar/ + GIT_WORK_TREE=../.. + /tmp/repo + +So, it's not as simple as not looking at `GIT_PREFIX` at all with this +version of git. When `GIT_WORK_TREE` is relative, and `GIT_PREFIX` is set, +it needs to combine them to get the actual path to the work tree. + +Perhaps git-annex should only use `GIT_PREFIX` +for fixup of relative `GIT_WORK_TREE`, but not for `GIT_DIR`. I've so +far only seen it be needed for `GIT_WORK_TREE`; I only made it also +be used for `GIT_DIR` out of an assumption git would be consistent in its +bugginess. + +Oh hell, here's another way it fails, with git 2.19: + + joey@darkstar:/tmp/repo/subdir/foo> git --work-tree=../.. status + BASH_EXECUTION_STRING='set | grep GIT_ >&2; pwd >&2; git-annex smudge --clean '\''subdir/file'\''' + GIT_DIR=/tmp/repo/.git + GIT_EXEC_PATH=/usr/lib/git-core + GIT_MERGE_AUTOEDIT=no + GIT_PAGER= + GIT_PREFIX=subdir/foo/ + GIT_WORK_TREE=. + /tmp/repo + git-annex: subdir/file: getFileStatus: does not exist (No such file or directory) + git-annex: smudge: 1 failed + +That one does not involve `GIT_DIR`, instead `GIT_WORK_TREE` is not relative to +`GIT_PREFIX` so the workaround that assumes it is breaks. I guess that we +could say that `GIT_DIR` is not relative to some directory in this case, +so the `GIT_PREFIX` is irrelevant. + +Kind of feels like git's behavior is so inconsistent and ill-specified +that trying to work around the bugs in it is likely to just be a neverending +source of bugs. + +Unfortunately the git devs have so far ignored my bug report despite it +having an easy test case needing nothing more than a simple shell script to +reproduce. And there are plenty of differently broken git versions out there +even if they eventually fix it, so it seems git-annex has to deal with this +mess somehow. + +I'm going with not using `GIT_PREFIX` for `GIT_DIR` and not for +`GIT_WORK_TREE=.` and hope that's enough. +"""]]