On Tue, Sep 24, 2013 at 12:06:43PM -0700, Jonathan Nieder wrote:
> Part of the underlying problem is that unlike 'git init', 'git clone'
> does not accept a --shared=(true|false|group|...) option. Worse, it
> does accept a --shared option, with a completely different meaning.
> No better option names are coming immediately to mind, but perhaps
> someone on the list will have ideas that could let 'git clone' and
> 'git init' use the same --share-repository=group option?
Perhaps calling it something like --permissions would be good (IMHO,
that is more descriptive than "--shared" for "git init"). Then both can
respect it, and "init" maintains "--shared" for backwards compatibility.
Calling it --shared-repository does match the config name, but I do not
find that name especially descriptive. But perhaps it is good enough,
and consistency with the existing name is certainly a plus.
> Another problem is that once the configuration is written, it is past
> the point that some of the sharedrepository setting's effect would
> have occured. The repository, including worktree, object dirs, and so
> on has already been created without g+w and setgid bits set.
Yes. This is as-documented for "clone -c", which claims to act after the
repository is initialized. That being said, I think it is less confusing
for the user for them to take effect as early as possible, so the user
doesn't have to worry.
I cannot think off-hand of any case where the user would actually want
the delayed effect.
> Worse, when we write the new configuration and then re-read it, we
> skip reading some repository-specific configuration
> (core.{repositoryformatversion,sharedrepository,bare,worktree})
> on the assumption that we should already know what their values would
> be. That assumption is now wrong.
Yes, I think this is simply a bug in 84054f7 (clone: accept config
options on the command line), which assumed that any effects it had
would be picked up by the git_config() call.
> I wonder if something like the following (just a sketch, completely
> untested) would make sense.
>
> diff --git i/builtin/clone.c w/builtin/clone.c
> index 7ac677d..145a974 100644
> --- i/builtin/clone.c
> +++ w/builtin/clone.c
> @@ -856,7 +856,11 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
> init_db(option_template, INIT_DB_QUIET);
> write_config(&option_config);
>
> + if (option_bare)
> + git_config_set("core.bare", "true");
> +
> git_config(git_default_config, NULL);
> + check_repository_format();
If we call check_repository_format() again, we will pick up the new
config. But I think we would still have to go back and fix the paths
created by init_db.
It may be cleaner to teach init_db to add the initial config (and
optionally add "init -c" for testing, though I do not think anyone
particularly cares in the real world).
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html