Re: [PATCH v3 2/3] config: add hashtable for config parsing & retrieval

4 messages, 3 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH v3 2/3] config: add hashtable for config parsing & retrieval

From: Junio C Hamano <hidden>
Date: 2016-06-15 23:01:44

Ramsay Jones [off-list ref] writes:
quoted
+static struct hashmap *get_config_cache(void)
+{
+	static struct hashmap config_cache;
+	if (!hashmap_initialized) {
+		config_cache_init(&config_cache);
+		hashmap_initialized = 1;
+		git_config(config_cache_callback, NULL);
+	}
+	return &config_cache;
+}
[I have not been following this series at all (sorry I haven't had
the time to spare), so take these comments with a very big pinch of
salt! ie just ignore me if it's already been discussed etc. ;-) ]

The 'git config' command can be used to read arbitrary files (so long
as they conform to the config syntax). For example, see the --file and
--blob options to git-config. At present, I think only scripted commands
use this facility (eg git-submodule). Noting the singleton config_cache,
what happens when git-submodule becomes a C builtin, or indeed any other
C builtin wants to take advantage of the new code when processing a non-
standard config file?
Yup, that is a very good point.  There needs an infrastructure to
tie a set of files (i.e. the standard one being the chain of
system-global /etc/gitconfig to repo-specific .git/config, and any
custom one that can be specified by the caller like submodule code)
to a separate hashmap; a future built-in submodule code would use
two hashmaps aka "config-caches", one to manage the usual
"configuration" and the other to manage the contents of the
.gitmodules file.

When we are operating across repositories (think "recursive
checkout"), we somehow have to deal with multiple sets of
configuration, one that applies to the top-level superproject and
others that apply to individual submodule repositories.

Even there is currently one caller, the "git config" command
implementation, if one is designing the new API, the design
shouldn't be tied to a singleton limitation, I would have to say, so
that future callers do not have to throw away everything done in
this series and start from scratch.

Re: [PATCH v3 2/3] config: add hashtable for config parsing & retrieval

From: Tanay Abhra <hidden>
Date: 2016-06-15 23:01:44

On 6/24/2014 4:55 AM, Junio C Hamano wrote:
Ramsay Jones [off-list ref] writes:
quoted
quoted
+static struct hashmap *get_config_cache(void)
+{
+	static struct hashmap config_cache;
+	if (!hashmap_initialized) {
+		config_cache_init(&config_cache);
+		hashmap_initialized = 1;
+		git_config(config_cache_callback, NULL);
+	}
+	return &config_cache;
+}
[I have not been following this series at all (sorry I haven't had
the time to spare), so take these comments with a very big pinch of
salt! ie just ignore me if it's already been discussed etc. ;-) ]

The 'git config' command can be used to read arbitrary files (so long
as they conform to the config syntax). For example, see the --file and
--blob options to git-config. At present, I think only scripted commands
use this facility (eg git-submodule). Noting the singleton config_cache,
what happens when git-submodule becomes a C builtin, or indeed any other
C builtin wants to take advantage of the new code when processing a non-
standard config file?
Yup, that is a very good point.  There needs an infrastructure to
tie a set of files (i.e. the standard one being the chain of
system-global /etc/gitconfig to repo-specific .git/config, and any
custom one that can be specified by the caller like submodule code)
to a separate hashmap; a future built-in submodule code would use
two hashmaps aka "config-caches", one to manage the usual
"configuration" and the other to manage the contents of the
.gitmodules file.

When we are operating across repositories (think "recursive
checkout"), we somehow have to deal with multiple sets of
configuration, one that applies to the top-level superproject and
others that apply to individual submodule repositories.

Even there is currently one caller, the "git config" command
implementation, if one is designing the new API, the design
shouldn't be tied to a singleton limitation, I would have to say, so
that future callers do not have to throw away everything done in
this series and start from scratch.
What changes should I implement in this series? Should I add new
user facing API adding to the singleton callers which are already in
this series.

"submodule lookup API" was intoduced by Heiko Voigt in [1], which is in
"cooking" now. His series already allows looking up local configuration
by passing in the null_sha1 for gitmodule or commit sha1. What else can
I add to my implementation, please suggest.

Thanks,
Tanay Abhra

Re: [PATCH v3 2/3] config: add hashtable for config parsing & retrieval

From: Tanay Abhra <hidden>
Date: 2016-06-15 23:01:44

On 6/24/2014 4:55 AM, Junio C Hamano wrote:
Ramsay Jones [off-list ref] writes:
quoted
quoted
+static struct hashmap *get_config_cache(void)
+{
+	static struct hashmap config_cache;
+	if (!hashmap_initialized) {
+		config_cache_init(&config_cache);
+		hashmap_initialized = 1;
+		git_config(config_cache_callback, NULL);
+	}
+	return &config_cache;
+}
[I have not been following this series at all (sorry I haven't had
the time to spare), so take these comments with a very big pinch of
salt! ie just ignore me if it's already been discussed etc. ;-) ]

The 'git config' command can be used to read arbitrary files (so long
as they conform to the config syntax). For example, see the --file and
--blob options to git-config. At present, I think only scripted commands
use this facility (eg git-submodule). Noting the singleton config_cache,
what happens when git-submodule becomes a C builtin, or indeed any other
C builtin wants to take advantage of the new code when processing a non-
standard config file?
Yup, that is a very good point.  There needs an infrastructure to
tie a set of files (i.e. the standard one being the chain of
system-global /etc/gitconfig to repo-specific .git/config, and any
custom one that can be specified by the caller like submodule code)
to a separate hashmap; a future built-in submodule code would use
two hashmaps aka "config-caches", one to manage the usual
"configuration" and the other to manage the contents of the
.gitmodules file.

When we are operating across repositories (think "recursive
checkout"), we somehow have to deal with multiple sets of
configuration, one that applies to the top-level superproject and
others that apply to individual submodule repositories.

Even there is currently one caller, the "git config" command
implementation, if one is designing the new API, the design
shouldn't be tied to a singleton limitation, I would have to say, so
that future callers do not have to throw away everything done in
this series and start from scratch.
Sorry for the repost, missed the link.

What changes should I implement in this series? Should I add new
user facing API adding to the singleton callers which are already in
this series.

"submodule lookup API" was intoduced by Heiko Voigt in [1], which is in
"cooking" now. His series already allows looking up local configuration
by passing in the null_sha1 for gitmodule or commit sha1. What else can
I add to my implementation, please suggest.

[1] http://thread.gmane.org/gmane.comp.version-control.git/250811

Thanks,
Tanay Abhra

Re: [PATCH v3 2/3] config: add hashtable for config parsing & retrieval

From: Ramsay Jones <hidden>
Date: 2016-06-15 23:01:44

On 24/06/14 00:25, Junio C Hamano wrote:
Ramsay Jones [off-list ref] writes:
quoted
quoted
+static struct hashmap *get_config_cache(void)
+{
+	static struct hashmap config_cache;
+	if (!hashmap_initialized) {
+		config_cache_init(&config_cache);
+		hashmap_initialized = 1;
+		git_config(config_cache_callback, NULL);
+	}
+	return &config_cache;
+}
[I have not been following this series at all (sorry I haven't had
the time to spare), so take these comments with a very big pinch of
salt! ie just ignore me if it's already been discussed etc. ;-) ]

The 'git config' command can be used to read arbitrary files (so long
as they conform to the config syntax). For example, see the --file and
--blob options to git-config. At present, I think only scripted commands
use this facility (eg git-submodule). Noting the singleton config_cache,
what happens when git-submodule becomes a C builtin, or indeed any other
C builtin wants to take advantage of the new code when processing a non-
standard config file?
Yup, that is a very good point.  There needs an infrastructure to
tie a set of files (i.e. the standard one being the chain of
system-global /etc/gitconfig to repo-specific .git/config, and any
custom one that can be specified by the caller like submodule code)
to a separate hashmap; a future built-in submodule code would use
two hashmaps aka "config-caches", one to manage the usual
"configuration" and the other to manage the contents of the
.gitmodules file.
I had expected to see one hash table per file/blob, with the three
standard config hash tables linked together to implement the scope/
priority rules. (Well, these could be merged into one, as the current
code does, since that makes handling "multi" keys slightly easier).

ie when looking up a symbol in the .git/config, if not found then
follow the link to parent scope (~/.gitconfig) and search there ...

Of course, other uses (I think) represent only a single scope, so they
would not have any parent scopes to link to.
When we are operating across repositories (think "recursive
checkout"), we somehow have to deal with multiple sets of
configuration, one that applies to the top-level superproject and
others that apply to individual submodule repositories.
Here, you could push the new .git/config onto a stack while linking
to the parent (~/.gitconfig) scope. So, the lookup code starts from
the stack top, etc, ...
Even there is currently one caller, the "git config" command
implementation, if one is designing the new API, the design
shouldn't be tied to a singleton limitation, I would have to say, so
that future callers do not have to throw away everything done in
this series and start from scratch.
.
I would like to stress that, despite the above, I have not given
any great thought to this project. So please take my comments with
a big pinch of salt. I was just scrolling through the patch in my
email client and was surprised to see the singleton ...

ATB,
Ramsay Jones
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help