{"thread":{"id":"14018","subject":"[PATCH 0/4] Add 'core.fsyncobjectfiles' config option","startedAt":"2008-06-18T22:29:48Z","lastAt":"2008-06-19T02:10:51Z","messageCount":16,"participants":["Linus Torvalds","Jeff King","Eric Raible","Junio C Hamano","Daniel Barkalow"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"80254","messageId":"alpine.LFD.1.10.0806181524490.2907@woody.linux-foundation.org","threadId":"14018","inReplyTo":null,"subject":"[PATCH 0/4] Add 'core.fsyncobjectfiles' config option","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-06-18T22:29:48Z","receivedAt":"2008-06-18T22:29:48Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nSo these four patches end up adding support for conditionally enabling \nfsync() on loose object creation in the .gitconfig file with something \nlike\n\n\t[core]\n\t\tFsyncObjectFiles = true\n\nwhich can be useful on filesystems that don't already guarantee data \nconsistency for other reasons (whether due to ordered writes or due to \nfull data journalling).\n\nActually, just the last one adds the fairly trivial feature, the three \nfirst patches are just cleanups of the config parsing that I needed in \norder to not gouge my eyes out when looking at the code. The config file \nparser is kind of ad-hoc and people have added more and more options to it \nwithout ever trying to clean it up, and I refuse to do that.\n\nThe full patch-series is\n\n  Split up default \"core\" config parsing into helper routine\n  Split up default \"user\" config parsing into helper routine\n  Split up default \"i18n\" and \"branch\" config parsing into helper\n    routines\n  Add config option to enable 'fsync()' of object files\n\nresulting in the following diffstat:\n\n Documentation/config.txt |    8 ++++\n cache.h                  |    1 +\n config.c                 |   82 ++++++++++++++++++++++++++++++++++-----------\n environment.c            |    1 +\n sha1_file.c              |    3 +-\n 5 files changed, 74 insertions(+), 21 deletions(-)\n\nand the patches themselves will follow..\n\n\t\t\tLinus\n"},{"id":"80255","messageId":"alpine.LFD.1.10.0806181529570.2907@woody.linux-foundation.org","threadId":"14018","inReplyTo":"alpine.LFD.1.10.0806181524490.2907@woody.linux-foundation.org","subject":"[PATCH 1/4] Split up default \"core\" config parsing into helper routine","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-06-18T22:30:35Z","receivedAt":"2008-06-18T22:30:35Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nFrom: Linus Torvalds <torvalds@linux-foundation.org>\nDate: Wed, 18 Jun 2008 14:37:18 -0700\nSubject: [PATCH 1/4] Split up default \"core\" config parsing into helper routine\n\nIt makes the code a bit easier to read, and in theory a bit faster too\n(no need to compare all the different \"core.*\" strings against non-core\nconfig options).\n\nThe config system really should get something of a complete overhaul,\nbut in the absense of that, this at least improves on it a tiny bit.\n\nSigned-off-by: Linus Torvalds <torvalds@linux-foundation.org>\n---\n config.c |   42 ++++++++++++++++++++++++++----------------\n 1 files changed, 26 insertions(+), 16 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex c2f2bbb..c3597e0 100644\n--- a/config.c\n+++ b/config.c\n@@ -332,7 +332,7 @@ int git_config_string(const char **dest, const char *var, const char *value)\n \treturn 0;\n }\n \n-int git_default_config(const char *var, const char *value, void *dummy)\n+static int git_default_core_config(const char *var, const char *value)\n {\n \t/* This needs a better name */\n \tif (!strcmp(var, \"core.filemode\")) {\n@@ -444,6 +444,31 @@ int git_default_config(const char *var, const char *value, void *dummy)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(var, \"core.pager\"))\n+\t\treturn git_config_string(&pager_program, var, value);\n+\n+\tif (!strcmp(var, \"core.editor\"))\n+\t\treturn git_config_string(&editor_program, var, value);\n+\n+\tif (!strcmp(var, \"core.excludesfile\"))\n+\t\treturn git_config_string(&excludes_file, var, value);\n+\n+\tif (!strcmp(var, \"core.whitespace\")) {\n+\t\tif (!value)\n+\t\t\treturn config_error_nonbool(var);\n+\t\twhitespace_rule_cfg = parse_whitespace_rule(value);\n+\t\treturn 0;\n+\t}\n+\n+\t/* Add other config variables here and to Documentation/config.txt. */\n+\treturn 0;\n+}\n+\n+int git_default_config(const char *var, const char *value, void *dummy)\n+{\n+\tif (!prefixcmp(var, \"core.\"))\n+\t\treturn git_default_core_config(var, value);\n+\n \tif (!strcmp(var, \"user.name\")) {\n \t\tif (!value)\n \t\t\treturn config_error_nonbool(var);\n@@ -473,21 +498,6 @@ int git_default_config(const char *var, const char *value, void *dummy)\n \t\treturn 0;\n \t}\n \n-\tif (!strcmp(var, \"core.pager\"))\n-\t\treturn git_config_string(&pager_program, var, value);\n-\n-\tif (!strcmp(var, \"core.editor\"))\n-\t\treturn git_config_string(&editor_program, var, value);\n-\n-\tif (!strcmp(var, \"core.excludesfile\"))\n-\t\treturn git_config_string(&excludes_file, var, value);\n-\n-\tif (!strcmp(var, \"core.whitespace\")) {\n-\t\tif (!value)\n-\t\t\treturn config_error_nonbool(var);\n-\t\twhitespace_rule_cfg = parse_whitespace_rule(value);\n-\t\treturn 0;\n-\t}\n \tif (!strcmp(var, \"branch.autosetupmerge\")) {\n \t\tif (value && !strcasecmp(value, \"always\")) {\n \t\t\tgit_branch_track = BRANCH_TRACK_ALWAYS;\n-- \n1.5.6.rc3.7.g336d0.dirty\n"},{"id":"80258","messageId":"alpine.LFD.1.10.0806181530420.2907@woody.linux-foundation.org","threadId":"14018","inReplyTo":"alpine.LFD.1.10.0806181529570.2907@woody.linux-foundation.org","subject":"[PATCH 2/4] Split up default \"user\" config parsing into helper routine","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-06-18T22:31:08Z","receivedAt":"2008-06-18T22:31:08Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nFrom: Linus Torvalds <torvalds@linux-foundation.org>\nDate: Wed, 18 Jun 2008 14:40:35 -0700\nSubject: [PATCH 2/4] Split up default \"user\" config parsing into helper routine\n\nThis follows the example of the \"core\" config, and splits out the\ndefault \"user\" config option parsing into a helper routine.\n\nSigned-off-by: Linus Torvalds <torvalds@linux-foundation.org>\n---\n config.c |   17 +++++++++++++----\n 1 files changed, 13 insertions(+), 4 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex c3597e0..ee7642b 100644\n--- a/config.c\n+++ b/config.c\n@@ -464,11 +464,8 @@ static int git_default_core_config(const char *var, const char *value)\n \treturn 0;\n }\n \n-int git_default_config(const char *var, const char *value, void *dummy)\n+static int git_default_user_config(const char *var, const char *value)\n {\n-\tif (!prefixcmp(var, \"core.\"))\n-\t\treturn git_default_core_config(var, value);\n-\n \tif (!strcmp(var, \"user.name\")) {\n \t\tif (!value)\n \t\t\treturn config_error_nonbool(var);\n@@ -487,6 +484,18 @@ int git_default_config(const char *var, const char *value, void *dummy)\n \t\treturn 0;\n \t}\n \n+\t/* Add other config variables here and to Documentation/config.txt. */\n+\treturn 0;\n+}\n+\n+int git_default_config(const char *var, const char *value, void *dummy)\n+{\n+\tif (!prefixcmp(var, \"core.\"))\n+\t\treturn git_default_core_config(var, value);\n+\n+\tif (!prefixcmp(var, \"user.\"))\n+\t\treturn git_default_user_config(var, value);\n+\n \tif (!strcmp(var, \"i18n.commitencoding\"))\n \t\treturn git_config_string(&git_commit_encoding, var, value);\n \n-- \n1.5.6.rc3.7.g336d0.dirty\n"},{"id":"80256","messageId":"alpine.LFD.1.10.0806181531120.2907@woody.linux-foundation.org","threadId":"14018","inReplyTo":"alpine.LFD.1.10.0806181530420.2907@woody.linux-foundation.org","subject":"[PATCH 3/4] Split up default \"i18n\" and \"branch\" config parsing into helper routines","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-06-18T22:31:32Z","receivedAt":"2008-06-18T22:31:32Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nFrom: Linus Torvalds <torvalds@linux-foundation.org>\nDate: Wed, 18 Jun 2008 15:00:11 -0700\nSubject: [PATCH 3/4] Split up default \"i18n\" and \"branch\" config parsing into helper routines\n\n.. just to finish it off.  We'll leave the pager color config alone,\nsince it is such an odd-ball special case anyway.\n\nSigned-off-by: Linus Torvalds <torvalds@linux-foundation.org>\n---\n config.c |   40 +++++++++++++++++++++++++++++-----------\n 1 files changed, 29 insertions(+), 11 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex ee7642b..9d14a74 100644\n--- a/config.c\n+++ b/config.c\n@@ -488,25 +488,20 @@ static int git_default_user_config(const char *var, const char *value)\n \treturn 0;\n }\n \n-int git_default_config(const char *var, const char *value, void *dummy)\n+static int git_default_i18n_config(const char *var, const char *value)\n {\n-\tif (!prefixcmp(var, \"core.\"))\n-\t\treturn git_default_core_config(var, value);\n-\n-\tif (!prefixcmp(var, \"user.\"))\n-\t\treturn git_default_user_config(var, value);\n-\n \tif (!strcmp(var, \"i18n.commitencoding\"))\n \t\treturn git_config_string(&git_commit_encoding, var, value);\n \n \tif (!strcmp(var, \"i18n.logoutputencoding\"))\n \t\treturn git_config_string(&git_log_output_encoding, var, value);\n \n-\tif (!strcmp(var, \"pager.color\") || !strcmp(var, \"color.pager\")) {\n-\t\tpager_use_color = git_config_bool(var,value);\n-\t\treturn 0;\n-\t}\n+\t/* Add other config variables here and to Documentation/config.txt. */\n+\treturn 0;\n+}\n \n+static int git_default_branch_config(const char *var, const char *value)\n+{\n \tif (!strcmp(var, \"branch.autosetupmerge\")) {\n \t\tif (value && !strcasecmp(value, \"always\")) {\n \t\t\tgit_branch_track = BRANCH_TRACK_ALWAYS;\n@@ -535,6 +530,29 @@ int git_default_config(const char *var, const char *value, void *dummy)\n \treturn 0;\n }\n \n+int git_default_config(const char *var, const char *value, void *dummy)\n+{\n+\tif (!prefixcmp(var, \"core.\"))\n+\t\treturn git_default_core_config(var, value);\n+\n+\tif (!prefixcmp(var, \"user.\"))\n+\t\treturn git_default_user_config(var, value);\n+\n+\tif (!prefixcmp(var, \"i18n.\"))\n+\t\treturn git_default_i18n_config(var, value);\n+\n+\tif (!prefixcmp(var, \"branch.\"))\n+\t\treturn git_default_branch_config(var, value);\n+\n+\tif (!strcmp(var, \"pager.color\") || !strcmp(var, \"color.pager\")) {\n+\t\tpager_use_color = git_config_bool(var,value);\n+\t\treturn 0;\n+\t}\n+\n+\t/* Add other config variables here and to Documentation/config.txt. */\n+\treturn 0;\n+}\n+\n int git_config_from_file(config_fn_t fn, const char *filename, void *data)\n {\n \tint ret;\n-- \n1.5.6.rc3.7.g336d0.dirty\n"},{"id":"80257","messageId":"alpine.LFD.1.10.0806181531370.2907@woody.linux-foundation.org","threadId":"14018","inReplyTo":"alpine.LFD.1.10.0806181531120.2907@woody.linux-foundation.org","subject":"[PATCH 4/4] Add config option to enable 'fsync()' of object files","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-06-18T22:32:24Z","receivedAt":"2008-06-18T22:32:24Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nFrom: Linus Torvalds <torvalds@linux-foundation.org>\nDate: Wed, 18 Jun 2008 15:18:44 -0700\nSubject: [PATCH 4/4] Add config option to enable 'fsync()' of object files\n\nAs explained in the documentation[*] this is totally useless on\nfilesystems that do ordered/journalled data writes, but it can be a\nuseful safety feature on filesystems like HFS+ that only journal the\nmetadata, not the actual file contents.\n\nIt defaults to off, although we could presumably in theory some day\nauto-enable it on a per-filesystem basis.\n\n[*] Yes, I updated the docs for the thing.  Hell really _has_ frozen\n    over, and the four horsemen are probably just beyond the horizon.\n    EVERYBODY PANIC!\n\nSigned-off-by: Linus Torvalds <torvalds@linux-foundation.org>\n---\n\nOk, this is the actual real and trivial patch. \n\n Documentation/config.txt |    8 ++++++++\n cache.h                  |    1 +\n config.c                 |    5 +++++\n environment.c            |    1 +\n sha1_file.c              |    3 ++-\n 5 files changed, 17 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 5331b45..01689f1 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -372,6 +372,14 @@ core.whitespace::\n   does not trigger if the character before such a carriage-return\n   is not a whitespace (not enabled by default).\n \n+core.fsyncobjectfiles::\n+\tThis boolean will enable 'fsync()' when writing object files. \n++\n+This is a total waste of time and effort on a filesystem that orders\n+data writes properly, but can be useful for filesystems that do not use\n+journalling (traditional UNIX filesystems) or that only journal metadata\n+and not file contents (OS X's HFS+, or Linux ext3 with \"data=writeback\").\n+\n alias.*::\n \tCommand aliases for the linkgit:git[1] command wrapper - e.g.\n \tafter defining \"alias.last = cat-file commit HEAD\", the invocation\ndiff --git a/cache.h b/cache.h\nindex 81b7e17..01c8502 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -435,6 +435,7 @@ extern size_t packed_git_window_size;\n extern size_t packed_git_limit;\n extern size_t delta_base_cache_limit;\n extern int auto_crlf;\n+extern int fsync_object_files;\n \n enum safe_crlf {\n \tSAFE_CRLF_FALSE = 0,\ndiff --git a/config.c b/config.c\nindex 9d14a74..b2d5b4e 100644\n--- a/config.c\n+++ b/config.c\n@@ -460,6 +460,11 @@ static int git_default_core_config(const char *var, const char *value)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(var, \"core.fsyncobjectfiles\")) {\n+\t\tfsync_object_files = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n \t/* Add other config variables here and to Documentation/config.txt. */\n \treturn 0;\n }\ndiff --git a/environment.c b/environment.c\nindex 73feb2d..d5c3e29 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -29,6 +29,7 @@ const char *apply_default_whitespace;\n int zlib_compression_level = Z_BEST_SPEED;\n int core_compression_level;\n int core_compression_seen;\n+int fsync_object_files;\n size_t packed_git_window_size = DEFAULT_PACKED_GIT_WINDOW_SIZE;\n size_t packed_git_limit = DEFAULT_PACKED_GIT_LIMIT;\n size_t delta_base_cache_limit = 16 * 1024 * 1024;\ndiff --git a/sha1_file.c b/sha1_file.c\nindex e2a65c5..c34ac6d 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -2083,7 +2083,8 @@ int hash_sha1_file(const void *buf, unsigned long len, const char *type,\n /* Finalize a file on disk, and close it. */\n static void close_sha1_file(int fd)\n {\n-\t/* For safe-mode, we could fsync_or_die(fd, \"sha1 file\") here */\n+\tif (fsync_object_files)\n+\t\tfsync_or_die(fd, \"sha1 file\");\n \tfchmod(fd, 0444);\n \tif (close(fd) != 0)\n \t\tdie(\"unable to write sha1 file\");\n-- \n1.5.6.rc3.7.g336d0.dirty\n"},{"id":"80260","messageId":"20080618224919.GA22599@sigill.intra.peff.net","threadId":"14018","inReplyTo":"alpine.LFD.1.10.0806181529570.2907@woody.linux-foundation.org","subject":"Re: [PATCH 1/4] Split up default \"core\" config parsing into helper routine","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-06-18T22:49:19Z","receivedAt":"2008-06-18T22:49:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 18, 2008 at 03:30:35PM -0700, Linus Torvalds wrote:\n\n> It makes the code a bit easier to read, and in theory a bit faster too\n> (no need to compare all the different \"core.*\" strings against non-core\n> config options).\n\nMaybe it would be easier still to read (and unmeasurably more efficient)\nto actually do it like:\n\n        if (!prefixcmp(var, \"core.\"))\n                return git_default_core_config(var+5, value);\n...\nint git_default_core_config(const char *var, const char *value)\n{\n        if (!strcmp(var, \"pager\"))\n                ...\n\n> The config system really should get something of a complete overhaul,\n> but in the absense of that, this at least improves on it a tiny bit.\n\nI was curious a while ago and instrumented git_config to write the PID\nto a tempfile each time it was called. Most git programs parse the\nconfig files (.git/config, ~/.gitconfig, /etc/gitconfig) three times\neach, with some doing it as many as five times.\n\nMost of the config functions are simply \"if this key, then set this\nvalue\". I wonder if it would be simpler to just load the whole thing at\nonce, using a table similar to parseopt.\n\nThen we could do useful things like say \"you specified core.foobar, but\nthere is no such variable.\" I know we can't know all values, since some\nnon-git programs put values in the config, but I don't think it's\nunreasonable for us to claim all of core.*, especially if it helps us\ncatch simple configuration errors.\n\n-Peff\n"},{"id":"80261","messageId":"alpine.LFD.1.10.0806181552590.2907@woody.linux-foundation.org","threadId":"14018","inReplyTo":"20080618224919.GA22599@sigill.intra.peff.net","subject":"Re: [PATCH 1/4] Split up default \"core\" config parsing into helper routine","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-06-18T22:58:13Z","receivedAt":"2008-06-18T22:58:13Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 18 Jun 2008, Jeff King wrote:\n> \n> Maybe it would be easier still to read (and unmeasurably more efficient)\n> to actually do it like:\n> \n>         if (!prefixcmp(var, \"core.\"))\n>                 return git_default_core_config(var+5, value);\n\nI considered it, but I think that screws up error reporting (ie if some \nvalue is unparseable, it would then print out the wrong variable name).\n\nIt would also have made the patches much less obvious. So it's a \"future \nenhancement\" thing.\n\n> I was curious a while ago and instrumented git_config to write the PID\n> to a tempfile each time it was called. Most git programs parse the\n> config files (.git/config, ~/.gitconfig, /etc/gitconfig) three times\n> each, with some doing it as many as five times.\n\nYeah, I know. I love the config file format (quite frankly, anybody who \nthinks XML and friends are sane is a total moron and should be shot before \nthey reproduce), but the whole parsing code was a really quick hack. \n\nI've several times wanted to rewrite it so that it does something smarter \n(parse it once, save it in a nice data structure), but let's face it, the \nupside is rather small. So I've never really ended up having the energy.\n\n> Then we could do useful things like say \"you specified core.foobar, but\n> there is no such variable.\"\n\nNo. We could already do that (just add it to the end of \ngit_default_core_config - it should be called last even if there was a \nchain), but avoid doing that very much on purpose.\n\nWhy? Because it's really irritating to have a parser that complains about \nnewer values (or old deprecated ones) that don't matter for that version \nof the program. \n\n\t\t\tLinus\n"},{"id":"80262","messageId":"20080618231316.GB23053@sigill.intra.peff.net","threadId":"14018","inReplyTo":"alpine.LFD.1.10.0806181552590.2907@woody.linux-foundation.org","subject":"Re: [PATCH 1/4] Split up default \"core\" config parsing into helper routine","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-06-18T23:13:16Z","receivedAt":"2008-06-18T23:13:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 18, 2008 at 03:58:13PM -0700, Linus Torvalds wrote:\n\n> >         if (!prefixcmp(var, \"core.\"))\n> >                 return git_default_core_config(var+5, value);\n> \n> I considered it, but I think that screws up error reporting (ie if some \n> value is unparseable, it would then print out the wrong variable name).\n\nGood point.\n\n> Yeah, I know. I love the config file format (quite frankly, anybody who \n> thinks XML and friends are sane is a total moron and should be shot before \n> they reproduce), but the whole parsing code was a really quick hack. \n\nI actually find the hierarchy a bit nonsensical. We talk about\ncore.pager and branch.master.remote, but the config file doesn't look\nlike that. So I buy that it's\n\n[core]\n  pager\n\nbut then it seems kind of arbitrary to have\n\n[branch \"master\"]\n  remote\n\ninstead of\n\n[branch]\n  master.remote\n\nor\n\n[branch]\n  [master]\n    remote\n\nwhich is of course impossible because we don't have a \"close section\"\nsyntax element.\n\nIt seems like the '.' implies hierarchy, but the syntax of the file\ndoesn't really follow it. But that's a minor issue, and really not worth\nthe pain it would take to change at this point.\n\n> I've several times wanted to rewrite it so that it does something smarter \n> (parse it once, save it in a nice data structure), but let's face it, the \n> upside is rather small. So I've never really ended up having the energy.\n\nI think we need to refactor what happens at the beginning of the 'git'\nwrapper, and this may be part of it. But as it stands now, the worktree\nsetup is a minefield of bugs (e.g., calling a command via an alias\nversus directly can get you different results for whether you have a\nworktree!), and a simple patch I did to make pager use configurable blew\nup in my face due to implicit constraints in when we look at the config\nfile (i.e., it mostly worked, but reading the config file implied doing\na setup_git_directory, which then screwed up a later invocation of\nsetup_git_directory).\n\nSo I think those things need cleaning up, and refactoring the config\nreading could probably be part of that. I might try to look at it in the\nnext release cycle.\n\n> > Then we could do useful things like say \"you specified core.foobar, but\n> > there is no such variable.\"\n> \n> No. We could already do that (just add it to the end of \n> git_default_core_config - it should be called last even if there was a \n> chain), but avoid doing that very much on purpose.\n> \n> Why? Because it's really irritating to have a parser that complains about \n> newer values (or old deprecated ones) that don't matter for that version \n> of the program. \n\nEven a \"check my config for bogosity\" program would be nice, and then it\nwouldn't bug you all the time. But you _can't_ do it now, because the\ncallback mechanism only looks at a subset of the values. IOW, there is\nno single function that you call that represents every config value;\ndifferent programs use different callbacks.\n\nIn the case of just core.*, though, you might be able to get away with\nit (I don't know how closely we've adhered to \"all of core.* is in\ngit_default_config).\n\n-Peff\n"},{"id":"80264","messageId":"alpine.LFD.1.10.0806181632080.2907@woody.linux-foundation.org","threadId":"14018","inReplyTo":"20080618231316.GB23053@sigill.intra.peff.net","subject":"Re: [PATCH 1/4] Split up default \"core\" config parsing into helper routine","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-06-18T23:34:49Z","receivedAt":"2008-06-18T23:34:49Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 18 Jun 2008, Jeff King wrote:\n> \n> It seems like the '.' implies hierarchy, but the syntax of the file\n> doesn't really follow it. But that's a minor issue, and really not worth\n> the pain it would take to change at this point.\n\nHierarchy is idiotic in a human-readable file. It's not how people work.\n\nAnd you're looking at the wrong part. You're looking at the _code_ part, \nwhich is not the primary thing. The primary thing is the config file \nsyntax. And\n\n\t[branch \"mybranch\"]\n\t\turl = xyz\n\nis a hell of a lot more readable than any alternatives I've ever seen, and \nno, there is no hierarchy _anywhere_ there, and shouldn't be.\n\nForget about \"branch.mybranch.url\". It has no meaning. It's not what you \nare supposed to ever use as a human (it's purely for scripting). It's not \nworth even thinking about.\n\n\t\t\tLinus\n"},{"id":"80270","messageId":"20080619000842.GA25117@sigill.intra.peff.net","threadId":"14018","inReplyTo":"alpine.LFD.1.10.0806181632080.2907@woody.linux-foundation.org","subject":"Re: [PATCH 1/4] Split up default \"core\" config parsing into helper routine","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-06-19T00:08:42Z","receivedAt":"2008-06-19T00:08:42Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 18, 2008 at 04:34:49PM -0700, Linus Torvalds wrote:\n\n> Forget about \"branch.mybranch.url\". It has no meaning. It's not what you \n> are supposed to ever use as a human (it's purely for scripting). It's not \n> worth even thinking about.\n\nHave you read \"man git-config\" lately?\n\n-Peff\n"},{"id":"80271","messageId":"loom.20080619T000837-669@post.gmane.org","threadId":"14018","inReplyTo":"alpine.LFD.1.10.0806181632080.2907@woody.linux-foundation.org","subject":"Re: [PATCH 1/4] Split up default \"core\" config parsing into helper routine","fromName":"Eric Raible","fromEmail":"raible@gmail.com","sentAt":"2008-06-19T00:18:49Z","receivedAt":"2008-06-19T00:18:49Z","isPatch":true,"sender":{"key":"raible@gmail.com","avatar":null},"body":"Linus Torvalds <torvalds <at> linux-foundation.org> writes:\n\n> Forget about \"branch.mybranch.url\". It has no meaning. It's not what you \n> are supposed to ever use as a human (it's purely for scripting). It's not \n> worth even thinking about.\n> \n> \t\t\tLinus\n\nBut it's hard to forget about it when the 'git pull' error\nmessage that begins with \"You asked me to pull without ...\"\nmentions these specifically:\n  branch.${curr_branch}.remote = <nickname>\"\n  branch.${curr_branch}.merge = <remote-ref>\"\n  remote.<nickname>.url = <url>\"\n  remote.<nickname>.fetch = <refspec>\"\n\nIt seems to me it would be much more helpful if that message\ngave some sample git config command lines rather than the\nresulting \"scripting-only\" names.\n\n- Eric\n"},{"id":"80272","messageId":"alpine.LFD.1.10.0806181721580.2907@woody.linux-foundation.org","threadId":"14018","inReplyTo":"20080619000842.GA25117@sigill.intra.peff.net","subject":"Re: [PATCH 1/4] Split up default \"core\" config parsing into helper routine","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-06-19T00:23:21Z","receivedAt":"2008-06-19T00:23:21Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 18 Jun 2008, Jeff King wrote:\n> \n> Have you read \"man git-config\" lately?\n\nThat whole program is there just for scripting. Of _course_ it talks about \nthe machine format!\n\nNobody should ever use \"git-config\" normally. You should fire up your \neditor and just edit the damn file in place. But no, we don't have a \nman-page for that.\n\n\t\tLinus\n"},{"id":"80274","messageId":"alpine.LFD.1.10.0806181726550.2907@woody.linux-foundation.org","threadId":"14018","inReplyTo":"alpine.LFD.1.10.0806181721580.2907@woody.linux-foundation.org","subject":"Re: [PATCH 1/4] Split up default \"core\" config parsing into helper routine","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-06-19T00:28:19Z","receivedAt":"2008-06-19T00:28:19Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 18 Jun 2008, Linus Torvalds wrote:\n> \n> Nobody should ever use \"git-config\" normally. You should fire up your \n> editor and just edit the damn file in place. But no, we don't have a \n> man-page for that.\n\nBtw, one reason people end up talking about git-config is that it's easier \nto tell *other* people to \"run this command\" than it is to tell them to \n\"edit this file so it looks like <xyz>\".\n\nAnd I think that's understandable, but still very sad. That file was \ndesigned to be edited by humans, and it's actually much easier to edit \nthat way than to do lots of \"git config\" runs.\n\n\t\t\tLinus\n"},{"id":"80275","messageId":"7vzlpiw9gn.fsf@gitster.siamese.dyndns.org","threadId":"14018","inReplyTo":"alpine.LFD.1.10.0806181721580.2907@woody.linux-foundation.org","subject":"Re: [PATCH 1/4] Split up default \"core\" config parsing into helper routine","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-19T00:32:24Z","receivedAt":"2008-06-19T00:32:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> On Wed, 18 Jun 2008, Jeff King wrote:\n>> \n>> Have you read \"man git-config\" lately?\n>\n> That whole program is there just for scripting. Of _course_ it talks about \n> the machine format!\n>\n> Nobody should ever use \"git-config\" normally. You should fire up your \n> editor and just edit the damn file in place. But no, we don't have a \n> man-page for that.\n\nI was asked to proofread a git book somebody is starting to write, and the\nfirst thing the book taught was to use \"config user.name\".\n\nI gave a suggestion that we should not teach config as the first thing but\ninstead we should show editing the $HOME/.gitconfig file manually, to give\nthe feeling that what git deals with is approachable, but I realize the\nsuggestion is not convincing when our own Documentation/gittutorial.txt\ndoes it fairly early in it, too.\n"},{"id":"80280","messageId":"20080619012310.GA25307@sigill.intra.peff.net","threadId":"14018","inReplyTo":"alpine.LFD.1.10.0806181721580.2907@woody.linux-foundation.org","subject":"Re: [PATCH 1/4] Split up default \"core\" config parsing into helper routine","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-06-19T01:23:10Z","receivedAt":"2008-06-19T01:23:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 18, 2008 at 05:23:21PM -0700, Linus Torvalds wrote:\n\n> > Have you read \"man git-config\" lately?\n> \n> That whole program is there just for scripting. Of _course_ it talks about \n> the machine format!\n\nForget the command git-config; all of the config _variables_ are\ndocumented in \"foo.bar\" form. Name one place in all of the documentation\nwhere the user can find out about core.pager, but the \"foo.bar\" form is\nnot used.\n\n-Peff\n"},{"id":"80282","messageId":"alpine.LNX.1.00.0806182202030.19665@iabervon.org","threadId":"14018","inReplyTo":"alpine.LFD.1.10.0806181726550.2907@woody.linux-foundation.org","subject":"Re: [PATCH 1/4] Split up default \"core\" config parsing into helper routine","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-06-19T02:10:51Z","receivedAt":"2008-06-19T02:10:51Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Wed, 18 Jun 2008, Linus Torvalds wrote:\n\n> On Wed, 18 Jun 2008, Linus Torvalds wrote:\n> > \n> > Nobody should ever use \"git-config\" normally. You should fire up your \n> > editor and just edit the damn file in place. But no, we don't have a \n> > man-page for that.\n> \n> Btw, one reason people end up talking about git-config is that it's easier \n> to tell *other* people to \"run this command\" than it is to tell them to \n> \"edit this file so it looks like <xyz>\".\n\nThe other factor is that you sometimes want to tell somebody one option \nsetting, and you're using a medium where there's a comprehension cost to \nnewlines. So 'remote.origin.url blah' is more clear than '[remote \n\"origin\"]\n\turl = blah' in those contexts. And we don't seem to encourage \n'[remote origin] url = blah', even though it turns out to work fine (at \nleast, I didn't know until I just now tried it that I could suggest it).\n\n\t-Daniel\n*This .sig left intentionally blank*\n"}]}