git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Duy Nguyen <pclouds@gmail.com>
To: Jeff King <peff@peff.net>
Cc: Rafael Ascensao <rafa.almas@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 16:56:00 +0200	[thread overview]
Message-ID: <CACsJy8DP53Og1crS1bLoJf6w8cJhjGKy=ggfbsqzJ6AU4eNhPA@mail.gmail.com> (raw)
In-Reply-To: <20180327063137.GA24044@sigill.intra.peff.net>

On Tue, Mar 27, 2018 at 8:31 AM, Jeff King <peff@peff.net> wrote:
>...
>
> But that really feels like we're papering over the problem.

It is papering over the problem in my opinion. setup_work_tree() is
involved here and when it moves cwd around, it re-init git-dir (and
all other paths). Before my patch we call git_path() only when we need
it and git-dir is likely already updated by setup_work_tree(). After
this patch, the path is set in stone before setup_work_tree() kicks
in. Once it moves cwd, relative paths all become invalid.

The way setup_work_tree() does it is just bad to me. When it calls
set_git_dir() again, the assumption is the new path is exactly the
same as the old one. The only difference is relative vs absolute. But
this is super hard to see from set_git_dir implementation. The new
struct repository design also inherits this (i.e. allowing to call
set_git_dir multiple times, which really does not make sense), but
this won't fly long term. When cwd moves, all cached relative paths
must be adjusted, we need a better mechanism to tell everybody that,
not just do it for $GIT_DIR and related paths.

I am planning to fix this. This is part of the "setup cleanup" effort
to keep repository.c design less messy and hopefully unify how the
main repo and submodule repo are created/set up. But frankly that may
take a long while before I could submit anything substantial (I also
have the "report multiple worktree's HEADs correctly and make fsck
count all HEADs" task, which is not small and to me have higher
priority).

So I would not mind papering over it right now (with an understanding
that absolute path pays some more overhead in path walking, which was
th reason we tried to avoid it in setup code). A slightly better patch
is trigger this path absolutization from setup_work_tree(), near
set_git_dir(). But then you face some ugliness there: how to find out
all ref stores to update, or just update the main ref store alone.

> 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
-- 
Duy

  reply	other threads:[~2018-03-27 14:56 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
2018-03-27 14:56   ` Duy Nguyen [this message]
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='CACsJy8DP53Og1crS1bLoJf6w8cJhjGKy=ggfbsqzJ6AU4eNhPA@mail.gmail.com' \
    --to=pclouds@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    --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).