Re: [PATCH 4/6] Add check_repo_format check for all major operations.
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:42:13
Junio C Hamano [off-list ref] writes:
Yes, although that is exactly what I said "this does not quite feel right" ;-). Is it hard to arrange things so that a process does exactly one check_repo_format() during its lifetime?
Let's back up a bit. I'll deal with only low-level commands
here.
First, the easiest group. The following commands do not look at
the repository (.git directory) at all.
check-ref-format get-tar-commit-id git index-pack mailinfo
mailsplit patch-id shell show-index stripspace verify-pack
We do not need to do anything special about them.
The following three use enter_repo() to the given path (either
from the end user at the command line or over the network):
daemon receive-pack upload-pack
The repo format check should be done at the same place as we
make sure enter_repo() finds the given directory a satisfactory
path. That is, just before putenv(GIT_DIR=.) in enter_repo().
The following use setup_git_directory(), which chdir()s to the
toplevel unless GIT_DIR is set.
cat-file config-set diff-files diff-index diff-stages diff-tree
ls-files name-rev rev-list rev-parse show-branch symbolic-ref
update-index update-ref
Maybe we can have a thin wrapper around setup_git_directory()
and after it returns check "${GIT_DIR-.git}" for repository
format mismatch. What to do when GIT_DIR is set? Then we can
just use it to read the config from "$GIT_DIR/config" and check
the version.
The following commands implicitly assume that they are either
run from the toplevel or GIT_DIR environment tells them where
the .git/ directory is:
apply checkout-index clone-pack commit-tree convert-objects
fetch-pack fsck-objects hash-object http-fetch http-push init-db
local-fetch ls-tree merge-base merge-index mktag pack-objects
pack-redundant peek-remote prune-packed read-tree send-pack
ssh-fetch ssh-upload tar-tree unpack-file unpack-objects
update-server-info var write-tree
With some exceptions, they are pretty much repository wide
commands, so I think it is OK for them to assume they start at
the toplevel (the Porcelain would chdir to the top for them
otherwise). The ones that take paths, namely, checkout-index,
hash-object and ls-tree, may want to use setup_git_directory()
and do the path prefixing.
That means we would need to have some way for the rest of the
commands to check if "${GIT_DIR-.git}" is of the right format
version, and call that *once* per process invocation,
perferrably at the beginning of the main().
We need an access to .git/config file to do the repository
format check anyway, which means we need setup_git_directory()
if we ever want to run them from subdirectories. And running
setup_git_directory() from the toplevel would not hurt, so
perhaps if we add setup_git_directory() at the beginning of
main() for the "implicity toplevel" class, and rewrite
setup_git_directory() like this:
static const char *setup_git_directory_main(void)
{
/* current setup_git_driectory() */
}
const char *setup_git_directory(void)
{
const char *retval = setup_git_directory_main();
check_repository_format_version(); /* dies on mismatch */
return retval;
}
it _might_ be good enough.
I said "it _might_" here, because we need to be careful. Right
now, if you run the "implicitly toplevel" commands from a
subdirectory, they fail. Some Porcelains and scripts may be
relying on that and there will be consequences if things
suddenly start not to fail but do something unexpected in higher
directories.