Re: Moving initialization of log_all_ref_updates

["Shawn O. Pearce" <spearce@spearce.org>]

Junio C Hamano <junkio@cox.net> wrote:
> The patches to prevent Porcelainish that require working tree
> from doing any damage in a bare repository make a lot of sense,
> and I want to make the is_bare_git_dir() function more reliable.
> 
> In order to allow the repository owner override the heuristic
> implemented in is_bare_git_dir() if/when it misidentifies a
> particular repository, it would make sense to introduce a new
> configuration variable "[core] bare = true/false", and make
> is_bare_git_dir() notice it.

Yes, this is an excellent idea.
 
> The scripts would do a 'repo-config --bool --get core.bare' and
> iff the command fails (i.e. there is no such variable in the
> configuration file), it would do the heuristic you implemented
> in your RFC patch [*1*].

Which will come up right most of the time.  And when it doesn't,
core.bare is there to bail the user out.  I like it.

Do we want to consider having init-db/clone set core.bare based on
what they are being asked to do?

> However, setup_git_env() which is called a lot earlier than we
> even read from the repository configuration currently makes a
> call to is_bare_git_dir(), in order to change the default
> setting for log_all_ref_updates.  It somehow feels that this is
> a hack, and I am considering the following patch.  What do you
> think?

Ack'd.  There's little we can do here then to change the order things
get invoked.  That's harder and uglier to change than to just declare
"we don't know what this right now" and guess later.  I'd go with
the patch you have now.  Maybe post 1.5.0 we can cleanup some of the
init order to get the config file parsing for at least some really
very core properties to occur at the same time as setup_git_env().

One thing I don't like about the config file parsing is that
applications can easily skip doing the core.* property setup.
With my sp/mmap changes this means core.packedGit{Limit,WindowSize}
may not be changed away from their defaults, and yet the application
will perform pack file access.  IMHO core.* is core; if you are
going after pack files or loose objects you better also recognize
and honor what core.* says!
 
> By the way, [*1*] is another thing I hate about the current
> config mechanism.  "git-repo-config --get" does not know what
> the possible configuration variables are, let alone what the
> default values for them are.  It allows us not to maintain a
> centralized configuration table, which makes it easy to
> introduce ad-hoc variables and gives a warm fuzzy feeling of
> being modular, but my feeling is that it is turning out to be a
> rather high price to pay for scripts.

I was thinking the other day (after reading an earlier email from
you stating the exact same thing) that perhaps we should do a
config registry within the repository.  E.g. if you want to access
a property in the config file you should also load its documentation
into a file like config.desc:

  $ cat .git/config.desc
  [core "bare"]
    default = 
    package = core-git
    documentation = "Indicates ...."

  [gui "fontdiff"]
    default = -family VT100 -size 9 -weight normal
    package = git-gui
    documentation = "The font for display of a file diff."

I'm not entirely happy with the above format; its just an example.

Obviously we can't change the current repo-config behavior, but
we could add a new "--declared" option which requires that the
property being get/set is also described in .git/config.desc,
failing if it isn't.

-- 
Shawn.