git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Rafael Ascensao <rafa.almas@gmail.com>
Cc: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
	"Git Mailing List" <git@vger.kernel.org>
Subject: Re: git complains packed-refs is not a directory when used with GIT_DIR and GIT_WORK_TREE envvars.
Date: Tue, 27 Mar 2018 02:31:38 -0400	[thread overview]
Message-ID: <20180327063137.GA24044@sigill.intra.peff.net> (raw)
In-Reply-To: <CACUQV5_3Pw+vnyyNUL4oE4tMLG_wKVdqdVk01rg4V92ufUYHHA@mail.gmail.com>

On Mon, Mar 26, 2018 at 10:27:09PM +0100, Rafael Ascensao wrote:

> One of the tools that manages PKGBUILDS for Arch Linux stores PKGBUILD
> git repos inside a cache directory for reuse.
> 
> One of the repos is triggering some unexpected behaviour that can be
> reproduced in the CLI with:
> 
>   $ GIT_DIR=spotify/.git GIT_WORK_TREE=spotify git reset HEAD
>   fatal: couldn't read spotify/.git/packed-refs: Not a directory
> [...]
> The issue seems to bisect to f57f37e2e1bf11ab4cdfd221ad47e961ba9353a0
> I can't pinpoint why this particular repo is behaving differently.

I think we're getting confused by the relative paths. Here's a related
reproduction:

  $ git init repo
  $ git -C repo commit --allow-empty -m foo
  $ GIT_DIR=repo/.git GIT_WORK_TREE=repo git reset HEAD
  $ find repo/repo
  repo/repo
  repo/repo/.git
  repo/repo/.git/logs
  repo/repo/.git/logs/HEAD
  repo/repo/.git/HEAD

Er, what? It looks like we kept looking at "repo/.git" as our git
directory, even though we should have normalized it into an absolute
path after moving into the root of the work-tree.

I can also reproduce your exact error by inserting:

  git -C repo pack-refs
  echo whatever >repo/repo

before the call to "git reset" (and then we get ENOTDIR trying to read
the packed-refs file, because the file "repo" is in the way).

Looking at f57f37e2, I think the problem is that files_ref_store_create()
saves the value of get_git_dir() at that point. But later after we
chdir for the working tree, presumably it's updated, but we continue to
use the out-dated relative path.

So one "fix" is something like this:

diff --git a/refs.c b/refs.c
index 20ba82b434..449bdf2437 100644
--- a/refs.c
+++ b/refs.c
@@ -1643,11 +1643,14 @@ static struct ref_store *ref_store_init(const char *gitdir,
 	const char *be_name = "files";
 	struct ref_storage_be *be = find_ref_storage_backend(be_name);
 	struct ref_store *refs;
+	char *abs_gitdir;
 
 	if (!be)
 		die("BUG: reference backend %s is unknown", be_name);
 
-	refs = be->init(gitdir, flags);
+	abs_gitdir = absolute_pathdup(gitdir);
+	refs = be->init(abs_gitdir, flags);
+	free(abs_gitdir);
 	return refs;
 }
 

But that really feels like we're papering over the problem. It's not
clear to me exactly what f57f37e2 is trying to accomplish, and whether
it would work for it to look call get_git_dir() whenever it needed the
path.

-Peff

  parent reply	other threads:[~2018-03-27  6:31 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-26 21:27 git complains packed-refs is not a directory when used with GIT_DIR and GIT_WORK_TREE envvars Rafael Ascensao
2018-03-26 21:44 ` Ævar Arnfjörð Bjarmason
2018-03-27  6:31 ` Jeff King [this message]
2018-03-27 14:56   ` Duy Nguyen
2018-03-27 16:47     ` Jeff King
2018-03-27 17:09       ` Duy Nguyen
2018-03-27 17:30         ` Duy Nguyen
2018-03-28  9:52           ` Jeff King
2018-03-28 10:10             ` Duy Nguyen
2018-03-28 17:36               ` Jeff King
2018-03-28 17:38                 ` [PATCH 1/4] set_git_dir: die when setenv() fails Jeff King
2018-03-28 17:40                 ` [PATCH 2/4] add chdir-notify API Jeff King
2018-03-28 17:58                   ` Eric Sunshine
2018-03-28 18:02                     ` Jeff King
2018-03-29 14:53                   ` Duy Nguyen
2018-03-29 17:48                     ` Jeff King
2018-03-29 18:12                       ` Duy Nguyen
2018-03-28 17:42                 ` [PATCH 3/4] set_work_tree: use chdir_notify Jeff King
2018-03-29 17:02                   ` Duy Nguyen
2018-03-29 17:23                     ` Duy Nguyen
2018-03-29 17:50                       ` Jeff King
2018-03-29 17:50                     ` Jeff King
2018-03-29 18:01                       ` Duy Nguyen
2018-03-30 17:23                         ` Jeff King
2018-03-28 17:43                 ` [PATCH 4/4] refs: use chdir_notify to update cached relative paths Jeff King
2018-03-30 18:34                 ` [PATCH v2 0/5] re-parenting relative directories after chdir Jeff King
2018-03-30 18:34                   ` [PATCH v2 1/5] set_git_dir: die when setenv() fails Jeff King
2018-03-30 18:34                   ` [PATCH v2 2/5] trace.c: export trace_setup_key Jeff King
2018-03-30 19:46                     ` Junio C Hamano
2018-03-30 19:47                       ` Jeff King
2018-03-30 19:50                         ` Junio C Hamano
2018-03-30 19:54                           ` Jeff King
2018-03-30 18:35                   ` [PATCH v2 3/5] add chdir-notify API Jeff King
2018-03-30 18:35                   ` [PATCH v2 4/5] set_work_tree: use chdir_notify Jeff King
2018-03-30 18:35                   ` [PATCH v2 5/5] refs: use chdir_notify to update cached relative paths Jeff King
2018-03-30 19:36                   ` [PATCH v2 0/5] re-parenting relative directories after chdir Duy Nguyen
2018-03-28  9:47         ` git complains packed-refs is not a directory when used with GIT_DIR and GIT_WORK_TREE envvars Jeff King
2018-03-28 17:55           ` [PATCH 0/8] " Nguyễn Thái Ngọc Duy
2018-03-28 17:55             ` [PATCH 1/8] strbuf.c: add strbuf_ensure_trailing_dr_sep() Nguyễn Thái Ngọc Duy
2018-03-28 17:55             ` [PATCH 2/8] strbuf.c: reintroduce get_pwd_cwd() (with strbuf_ prefix) Nguyễn Thái Ngọc Duy
2018-03-28 18:02               ` Stefan Beller
2018-03-28 18:05                 ` Duy Nguyen
2018-03-28 17:55             ` [PATCH 3/8] trace.c: export trace_setup_key Nguyễn Thái Ngọc Duy
2018-03-28 17:55             ` [PATCH 4/8] setup.c: introduce setup_adjust_path() Nguyễn Thái Ngọc Duy
2018-03-28 17:55             ` [PATCH 5/8] setup.c: allow other code to be notified when $CWD moves Nguyễn Thái Ngọc Duy
2018-03-28 17:55             ` [PATCH 6/8] environment.c: adjust env containing relpath when $CWD is moved Nguyễn Thái Ngọc Duy
2018-03-28 18:30               ` Jeff King
2018-03-28 18:45                 ` Duy Nguyen
2018-03-28 17:55             ` [PATCH 7/8] repository: adjust repo paths when $CWD moves Nguyễn Thái Ngọc Duy
2018-03-28 17:55             ` [PATCH 8/8] refs: adjust main " Nguyễn Thái Ngọc Duy
2018-03-28 18:19             ` [PATCH 0/8] Re: git complains packed-refs is not a directory when used with GIT_DIR and GIT_WORK_TREE envvars Jeff King
2018-03-29 14:57               ` Duy Nguyen
2018-03-30 17:21                 ` Jeff King
2018-03-28 22:24             ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180327063137.GA24044@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=pclouds@gmail.com \
    --cc=rafa.almas@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).