Thread (1 message) 1 message, 1 author, 2016-06-15

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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help