From: Madhu <enometh@meer.net>
To: gitster@pobox.com
Cc: git@vger.kernel.org
Subject: Re: [PATCH] init: don't reset core.filemode on git-new-workdirs.
Date: Mon, 22 Mar 2021 14:34:37 +0530 (IST) [thread overview]
Message-ID: <20210322.143437.212295420302618690.enometh@meer.net> (raw)
In-Reply-To: <xmqq7dlz94by.fsf@gitster.g>
* Junio C Hamano <gitster@pobox.com> <xmqq7dlz94by.fsf@gitster.g>
Wrote on Sun, 21 Mar 2021 21:53:37 -0700
> There are two points we should consider.
>
> * Historically, we've used .git/config as the sample file to check,
> but that was not because we wanted to make sure we can chmod the
> config file, but because we knew the file has to be there. If
> .git/config is sometimes a symbolic link, and if chmod test
> requires a regular file, we do not have to use .git/config as the
> sample file. We could instead switch to use a different,
> temporary, file. After all, the symlink check I pointed out in
> the message you are responding to uses a brand new temporary
> filename for that, and there is no reason why we shouldn't do the
> same with a regular file for the filemode test.
>
> * If running "git init" in an already functioning repository can be
> a useful way to "re-initialize" and/or "correct" various aspect
> of the repository (e.g. perhaps core.filemode is incorrectly set
> originally and running "git init" again corrects it), we would
> want to allow that in a normal repository as well as in a
> repository that is created by new-workdir the same way. Or if it
> is not useful and we want "re-initialize" not to touch the
> filemode, we would want to skip the check in a normal repository
> as well as in a new-workdir repository the same way. That is why
> "if symlink, then skip" is wrong---it targets the new-workdir
> case specifically.
I'd say it doesn't (target the new-workdir case specifically). The
lstat test on `config' is incorrect if `config' (or a new file) is not
a regular file, so I think it should be fixed anyway. The new-workdir
case happens to be handled correctly once this is fixed.
> I personally do not have a strong opinion either way, but to me, it
> seems that "does the filesystem support filemode?" and "does the
> filesystem support symbolic link?" are at about the same level and
> should be treated similarly unless there is a good reason not to.
> And the symlink check is never done in "reinit" case, so perhaps
> when "git init" is run again in an already functioning repository,
> we should not muck with the filemode, either.
I'd think so (on the last point). While it is understandable to
expect consistent re-initing behaviour (which is why the spurious
git-init was there) the expectations may not hold if the git directory
and work tree are on different filesystems - there is more scope for
making wrong inferences.
Typically checkout worktrees with new-workdir in /dev/shm for
advantages in the low overhead.
I do have repositories where .git/config is a symlink to a config.A
and I have other config.B files - all for the same repo but with
different upstreams. (I know better ways to do it but why should I be
prevented from doing this)
> A natural conclusion of the line of thought is that we can move the
> "check filemode trustability" block (from the comment to concluding
> git_config_set()) inside the "if (!reinit)" that happens a bit later
> and we'd be fine---as an existing normal repository, as well as what
> new-workdir creates, won't have to do the "let's chmod +x/-x the
> config file and see what happens" code at all (perhaps the attached
> patch, which hasn't even been compile tested).
>
> On the other hand, if it is worth "fixing" the filemode setting
> while re-initializing, we probably should switch to use a temporary
> file instead of 'config'. And we may want to reconsider the placement
> of the "is symlink supported?" check---which may also have to be
> redone to "fix" its existing value.
I don't think the posted patch (snipped) would work as reinit is
always 1 and we are always a candidate for reiniting - I may be
missing something.
Using a new file for the filemode test would be a natural
improvement. But I dont see the added complexity as a win. I can
imagine other scenarios that need to be fixed: calling git-init on a
worktree prepared by git-new-worktree (not git-new-workdir) on a FAT
fs with .git on ext2. Calling git-init on a worktree prepared by
calling git-new-worktree with a parent which is checked out by
git-new-workdir. (The parent has a symlinked config). The
possibilities for fun endless :)
next prev parent reply other threads:[~2021-03-22 9:06 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-21 12:28 [PATCH] init: don't reset core.filemode on git-new-workdirs Madhu
2021-03-21 21:58 ` Junio C Hamano
2021-03-22 2:40 ` Madhu
2021-03-22 4:53 ` Junio C Hamano
2021-03-22 9:04 ` Madhu [this message]
2021-03-22 18:02 ` Junio C Hamano
2021-03-23 3:57 ` Madhu
2021-03-23 6:39 ` Junio C Hamano
2021-03-23 16:53 ` Torsten Bögershausen
2021-03-23 17:45 ` Junio C Hamano
2021-03-23 20:31 ` Torsten Bögershausen
2021-03-23 20:50 ` Junio C Hamano
2021-06-18 4:18 ` Madhu
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=20210322.143437.212295420302618690.enometh@meer.net \
--to=enometh@meer.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.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).