{"thread":{"id":"38571","subject":"[PATCH] push: allow --follow-tags to be set by config push.followTags","startedAt":"2015-02-16T03:01:30Z","lastAt":"2015-03-14T22:08:22Z","messageCount":29,"participants":["Dave Olszewski","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"256117","messageId":"1424055690-32631-1-git-send-email-cxreg@pobox.com","threadId":"38571","inReplyTo":null,"subject":"[PATCH] push: allow --follow-tags to be set by config push.followTags","fromName":"Dave Olszewski","fromEmail":"cxreg@pobox.com","sentAt":"2015-02-16T03:01:30Z","receivedAt":"2015-02-16T03:01:30Z","isPatch":true,"sender":{"key":"cxreg@pobox.com","avatar":"https://avatars.githubusercontent.com/u/55474?v=4"},"body":"Signed-off-by: Dave Olszewski \n---\n Documentation/config.txt | 6 ++++++\n Documentation/git-push.txt | 5 ++++-\n builtin/push.c | 5 +++++\n cache.h | 1 +\n config.c | 5 +++++\n contrib/completion/git-completion.bash | 1 +\n environment.c | 1 +\n transport.c | 2 +-\n 8 files changed, 24 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex ae6791d..e01d21c 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2079,6 +2079,12 @@ new default).\n \n --\n \n+push.followTags::\n+\tIf set to true enable '--follow-tags' option by default. You\n+\tmay override this configuration at time of push by specifying\n+\t'--no-follow-tags'.\n+\n+\n rebase.stat::\n \tWhether to show a diffstat of what changed upstream since the last\n \trebase. False by default.\ndiff --git a/Documentation/git-push.txt b/Documentation/git-push.txt\nindex ea97576..caa187b 100644\n--- a/Documentation/git-push.txt\n+++ b/Documentation/git-push.txt\n@@ -128,7 +128,10 @@ already exists on the remote side.\n \tPush all the refs that would be pushed without this option,\n \tand also push annotated tags in `refs/tags` that are missing\n \tfrom the remote but are pointing at commit-ish that are\n-\treachable from the refs being pushed.\n+\treachable from the refs being pushed. This can also be specified\n+\twith configuration variable 'push.followTags'. For more\n+\tinformation, see 'push.followTags' in linkgit:git-config[1].\n+\n \n --signed::\n \tGPG-sign the push request to update refs on the receiving\ndiff --git a/builtin/push.c b/builtin/push.c\nindex fc771a9..47f0119 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -525,6 +525,11 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n \n \tpacket_trace_identity(\"push\");\n \tgit_config(git_push_config, NULL);\n+\n+\t/* set TRANSPORT_PUSH_FOLLOW_TAGS in flags so that --no-follow-tags may unset it */\n+\tif (push_follow_tags)\n+\t\tflags |= TRANSPORT_PUSH_FOLLOW_TAGS;\n+\n \targc = parse_options(argc, argv, prefix, options, push_usage, 0);\n \n \tif (deleterefs && (tags || (flags & (TRANSPORT_PUSH_ALL | TRANSPORT_PUSH_MIRROR))))\ndiff --git a/cache.h b/cache.h\nindex f704af5..9318189 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -648,6 +648,7 @@ enum push_default_type {\n extern enum branch_track git_branch_track;\n extern enum rebase_setup_type autorebase;\n extern enum push_default_type push_default;\n+extern int push_follow_tags;\n \n enum object_creation_mode {\n \tOBJECT_CREATION_USES_HARDLINKS = 0,\ndiff --git a/config.c b/config.c\nindex e5e64dc..cb237cd 100644\n--- a/config.c\n+++ b/config.c\n@@ -977,6 +977,11 @@ static int git_default_push_config(const char *var, const char *value)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(var, \"push.followtags\")) {\n+\t\tpush_follow_tags = 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/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex c21190d..cffb2b8 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -2188,6 +2188,7 @@ _git_config ()\n \t\tpull.octopus\n \t\tpull.twohead\n \t\tpush.default\n+\t\tpush.followTags\n \t\trebase.autosquash\n \t\trebase.stat\n \t\treceive.autogc\ndiff --git a/environment.c b/environment.c\nindex 1ade5c9..aef9587 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -52,6 +52,7 @@ unsigned whitespace_rule_cfg = WS_DEFAULT_RULE;\n enum branch_track git_branch_track = BRANCH_TRACK_REMOTE;\n enum rebase_setup_type autorebase = AUTOREBASE_NEVER;\n enum push_default_type push_default = PUSH_DEFAULT_UNSPECIFIED;\n+int push_follow_tags = 0;\n #ifndef OBJECT_CREATION_MODE\n #define OBJECT_CREATION_MODE OBJECT_CREATION_USES_HARDLINKS\n #endif\ndiff --git a/transport.c b/transport.c\nindex 0694a7c..ff5f63d 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -1148,7 +1148,7 @@ int transport_push(struct transport *transport,\n \t\t\tmatch_flags |= MATCH_REFS_MIRROR;\n \t\tif (flags & TRANSPORT_PUSH_PRUNE)\n \t\t\tmatch_flags |= MATCH_REFS_PRUNE;\n-\t\tif (flags & TRANSPORT_PUSH_FOLLOW_TAGS)\n+\t\tif ((flags & TRANSPORT_PUSH_FOLLOW_TAGS))\n \t\t\tmatch_flags |= MATCH_REFS_FOLLOW_TAGS;\n \n \t\tif (match_push_refs(local_refs, &remote_refs,\n-- \n2.1.4\n"},{"id":"256120","messageId":"20150216052049.GA5031@peff.net","threadId":"38571","inReplyTo":"1424055690-32631-1-git-send-email-cxreg@pobox.com","subject":"Re: [PATCH] push: allow --follow-tags to be set by config push.followTags","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-02-16T05:20:49Z","receivedAt":"2015-02-16T05:20:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Feb 15, 2015 at 07:01:30PM -0800, Dave Olszewski wrote:\n\n> +push.followTags::\n> +\tIf set to true enable '--follow-tags' option by default. You\n> +\tmay override this configuration at time of push by specifying\n> +\t'--no-follow-tags'.\n\nThanks, this is something I've considered implementing myself, as I have\none repo that is frequently migrating tags from one remote to another,\nand I often forget to specify the option.\n\n> diff --git a/builtin/push.c b/builtin/push.c\n> index fc771a9..47f0119 100644\n> --- a/builtin/push.c\n> +++ b/builtin/push.c\n> @@ -525,6 +525,11 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n> \n> \tpacket_trace_identity(\"push\");\n> \tgit_config(git_push_config, NULL);\n> +\n> +\t/* set TRANSPORT_PUSH_FOLLOW_TAGS in flags so that --no-follow-tags may unset it */\n> +\tif (push_follow_tags)\n> +\t\tflags |= TRANSPORT_PUSH_FOLLOW_TAGS;\n\nYou can see above that we use git_push_config to load our config...\n\n> --- a/config.c\n> +++ b/config.c\n> @@ -977,6 +977,11 @@ static int git_default_push_config(const char *var, const char *value)\n> \t\treturn 0;\n> \t}\n> \n> +\tif (!strcmp(var, \"push.followtags\")) {\n> +\t\tpush_follow_tags = git_config_bool(var, value);\n> +\t\treturn 0;\n> +\t}\n\nBut here you are adding to git_default_push_config, which is in another\nfile.\n\nI'm trying to figure out why git_default_push_config exists at all. The\nmajor difference from git_push_config is that the \"default\" variant will\nget loaded for _all_ commands, not just \"push\". So if it affected\nvariables that were used by other commands, it would be needed. But all\nit sets is push_default, which seems to be specific to builtin/push.c.\n\nSo I suspect it can be removed entirely, and folded into\ngit_config_push. But that's outside the scope of your patch.\n\nWhat _is_ in the scope of your patch is that I think the new option you\nare adding could go into git_push_config; it is definitely only about\nthe push command itself. And then you could declare it as:\n\n static int push_follow_tags;\n\nwithout having to worry about making it an extern that is available\neverywhere.\n\n> diff --git a/transport.c b/transport.c\n> index 0694a7c..ff5f63d 100644\n> --- a/transport.c\n> +++ b/transport.c\n> @@ -1148,7 +1148,7 @@ int transport_push(struct transport *transport,\n> \t\t\tmatch_flags |= MATCH_REFS_MIRROR;\n> \t\tif (flags & TRANSPORT_PUSH_PRUNE)\n> \t\t\tmatch_flags |= MATCH_REFS_PRUNE;\n> -\t\tif (flags & TRANSPORT_PUSH_FOLLOW_TAGS)\n> +\t\tif ((flags & TRANSPORT_PUSH_FOLLOW_TAGS))\n> \t\t\tmatch_flags |= MATCH_REFS_FOLLOW_TAGS;\n\nThis looks like just noise in the diff (I guess leftover from some\ndebugging you were doing). Is that correct?\n\n-Peff\n"},{"id":"256121","messageId":"20150216054550.GA24611@peff.net","threadId":"38571","inReplyTo":"20150216052049.GA5031@peff.net","subject":"[PATCH 0/2] clean up push config callbacks","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-02-16T05:45:50Z","receivedAt":"2015-02-16T05:45:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 16, 2015 at 12:20:49AM -0500, Jeff King wrote:\n\n> But here you are adding to git_default_push_config, which is in another\n> file.\n> \n> I'm trying to figure out why git_default_push_config exists at all. The\n> major difference from git_push_config is that the \"default\" variant will\n> get loaded for _all_ commands, not just \"push\". So if it affected\n> variables that were used by other commands, it would be needed. But all\n> it sets is push_default, which seems to be specific to builtin/push.c.\n> \n> So I suspect it can be removed entirely, and folded into\n> git_config_push. But that's outside the scope of your patch.\n\nHere's that cleanup, plus another one I noticed while doing it.\n\n [1/2]: git_push_config: drop cargo-culted wt_status pointer\n [2/2]: builtin/push.c: make push_default a static variable\n\n-Peff\n"},{"id":"256122","messageId":"20150216054629.GA25088@peff.net","threadId":"38571","inReplyTo":"20150216054550.GA24611@peff.net","subject":"[PATCH 1/2] git_push_config: drop cargo-culted wt_status pointer","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-02-16T05:46:30Z","receivedAt":"2015-02-16T05:46:30Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The push config callback does not expect any incoming data\nvia the void pointer. And if it did, it would certainly not\nbe a \"struct wt_status\". This probably got picked up\naccidentally in b945901 (push: heed user.signingkey for\nsigned pushes, 2014-10-22), which copied the template for\nthe config callback from builtin/commit.c.\n\nSigned-off-by: Jeff King \n---\n builtin/push.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex fc771a9..aa9334c 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -473,13 +473,12 @@ static int option_parse_recurse_submodules(const struct option *opt,\n \n static int git_push_config(const char *k, const char *v, void *cb)\n {\n-\tstruct wt_status *s = cb;\n \tint status;\n \n \tstatus = git_gpg_config(k, v, NULL);\n \tif (status)\n \t\treturn status;\n-\treturn git_default_config(k, v, s);\n+\treturn git_default_config(k, v, NULL);\n }\n \n int cmd_push(int argc, const char **argv, const char *prefix)\n-- \n2.3.0.rc1.287.g761fd19\n"},{"id":"256123","messageId":"20150216054754.GB25088@peff.net","threadId":"38571","inReplyTo":"20150216054550.GA24611@peff.net","subject":"[PATCH 2/2] builtin/push.c: make push_default a static variable","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-02-16T05:47:54Z","receivedAt":"2015-02-16T05:47:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When the \"push_default\" flag was originally added, it was\nmade globally visible to all code. This might have been\nuseful if other commands or library calls ended up depending\non it, but as it turns out, only builtin/push.c cares.\n\nLet's make it a static variable in builtin/push.c. Since it\nis no longer globally visible, it only needs to be set\ninside that function. That means we can drop the\ngit_push_default_config function (which is called from\ngit_default_config for all commands) and just set it as part\nof git_push_config.\n\nThat in turn makes it easier for people adding new config to\ngit-push to know which callback function to add to (since\nthere is only one now).\n\nSigned-off-by: Jeff King \n---\nWe know this is safe because no other callers needed tweaked when the\nvariable went out of scope. :) It would only be a bad idea if we\nwere planning on having other code in the future depend on push_default\n(e.g., the code in remote.c to find the push destination). But it does\nnot seem to have needed that in the intervening years, so it's probably\nfine to do this cleanup now.\n\n builtin/push.c | 33 +++++++++++++++++++++++++++++++++\n cache.h | 10 ----------\n config.c | 32 --------------------------------\n environment.c | 1 -\n 4 files changed, 33 insertions(+), 43 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex aa9334c..ab99f4c 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -23,6 +23,15 @@ static int progress = -1;\n \n static struct push_cas_option cas;\n \n+static enum push_default_type {\n+\tPUSH_DEFAULT_NOTHING = 0,\n+\tPUSH_DEFAULT_MATCHING,\n+\tPUSH_DEFAULT_SIMPLE,\n+\tPUSH_DEFAULT_UPSTREAM,\n+\tPUSH_DEFAULT_CURRENT,\n+\tPUSH_DEFAULT_UNSPECIFIED\n+} push_default = PUSH_DEFAULT_UNSPECIFIED;\n+\n static const char **refspec;\n static int refspec_nr;\n static int refspec_alloc;\n@@ -478,6 +487,30 @@ static int git_push_config(const char *k, const char *v, void *cb)\n \tstatus = git_gpg_config(k, v, NULL);\n \tif (status)\n \t\treturn status;\n+\n+\tif (!strcmp(k, \"push.default\")) {\n+\t\tif (!v)\n+\t\t\treturn config_error_nonbool(k);\n+\t\telse if (!strcmp(v, \"nothing\"))\n+\t\t\tpush_default = PUSH_DEFAULT_NOTHING;\n+\t\telse if (!strcmp(v, \"matching\"))\n+\t\t\tpush_default = PUSH_DEFAULT_MATCHING;\n+\t\telse if (!strcmp(v, \"simple\"))\n+\t\t\tpush_default = PUSH_DEFAULT_SIMPLE;\n+\t\telse if (!strcmp(v, \"upstream\"))\n+\t\t\tpush_default = PUSH_DEFAULT_UPSTREAM;\n+\t\telse if (!strcmp(v, \"tracking\")) /* deprecated */\n+\t\t\tpush_default = PUSH_DEFAULT_UPSTREAM;\n+\t\telse if (!strcmp(v, \"current\"))\n+\t\t\tpush_default = PUSH_DEFAULT_CURRENT;\n+\t\telse {\n+\t\t\terror(\"Malformed value for %s: %s\", k, v);\n+\t\t\treturn error(\"Must be one of nothing, matching, simple, \"\n+\t\t\t\t \"upstream or current.\");\n+\t\t}\n+\t\treturn 0;\n+\t}\n+\n \treturn git_default_config(k, v, NULL);\n }\n \ndiff --git a/cache.h b/cache.h\nindex f704af5..5394546 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -636,18 +636,8 @@ enum rebase_setup_type {\n \tAUTOREBASE_ALWAYS\n };\n \n-enum push_default_type {\n-\tPUSH_DEFAULT_NOTHING = 0,\n-\tPUSH_DEFAULT_MATCHING,\n-\tPUSH_DEFAULT_SIMPLE,\n-\tPUSH_DEFAULT_UPSTREAM,\n-\tPUSH_DEFAULT_CURRENT,\n-\tPUSH_DEFAULT_UNSPECIFIED\n-};\n-\n extern enum branch_track git_branch_track;\n extern enum rebase_setup_type autorebase;\n-extern enum push_default_type push_default;\n \n enum object_creation_mode {\n \tOBJECT_CREATION_USES_HARDLINKS = 0,\ndiff --git a/config.c b/config.c\nindex e5e64dc..5782442 100644\n--- a/config.c\n+++ b/config.c\n@@ -952,35 +952,6 @@ static int git_default_branch_config(const char *var, const char *value)\n \treturn 0;\n }\n \n-static int git_default_push_config(const char *var, const char *value)\n-{\n-\tif (!strcmp(var, \"push.default\")) {\n-\t\tif (!value)\n-\t\t\treturn config_error_nonbool(var);\n-\t\telse if (!strcmp(value, \"nothing\"))\n-\t\t\tpush_default = PUSH_DEFAULT_NOTHING;\n-\t\telse if (!strcmp(value, \"matching\"))\n-\t\t\tpush_default = PUSH_DEFAULT_MATCHING;\n-\t\telse if (!strcmp(value, \"simple\"))\n-\t\t\tpush_default = PUSH_DEFAULT_SIMPLE;\n-\t\telse if (!strcmp(value, \"upstream\"))\n-\t\t\tpush_default = PUSH_DEFAULT_UPSTREAM;\n-\t\telse if (!strcmp(value, \"tracking\")) /* deprecated */\n-\t\t\tpush_default = PUSH_DEFAULT_UPSTREAM;\n-\t\telse if (!strcmp(value, \"current\"))\n-\t\t\tpush_default = PUSH_DEFAULT_CURRENT;\n-\t\telse {\n-\t\t\terror(\"Malformed value for %s: %s\", var, value);\n-\t\t\treturn error(\"Must be one of nothing, matching, simple, \"\n-\t\t\t\t \"upstream or current.\");\n-\t\t}\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 static int git_default_mailmap_config(const char *var, const char *value)\n {\n \tif (!strcmp(var, \"mailmap.file\"))\n@@ -1006,9 +977,6 @@ int git_default_config(const char *var, const char *value, void *dummy)\n \tif (starts_with(var, \"branch.\"))\n \t\treturn git_default_branch_config(var, value);\n \n-\tif (starts_with(var, \"push.\"))\n-\t\treturn git_default_push_config(var, value);\n-\n \tif (starts_with(var, \"mailmap.\"))\n \t\treturn git_default_mailmap_config(var, value);\n \ndiff --git a/environment.c b/environment.c\nindex 1ade5c9..bea09b6 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -51,7 +51,6 @@ enum safe_crlf safe_crlf = SAFE_CRLF_WARN;\n unsigned whitespace_rule_cfg = WS_DEFAULT_RULE;\n enum branch_track git_branch_track = BRANCH_TRACK_REMOTE;\n enum rebase_setup_type autorebase = AUTOREBASE_NEVER;\n-enum push_default_type push_default = PUSH_DEFAULT_UNSPECIFIED;\n #ifndef OBJECT_CREATION_MODE\n #define OBJECT_CREATION_MODE OBJECT_CREATION_USES_HARDLINKS\n #endif\n-- \n2.3.0.rc1.287.g761fd19\n"},{"id":"256124","messageId":"20150216055422.GB24611@peff.net","threadId":"38571","inReplyTo":"20150216054550.GA24611@peff.net","subject":"[PATCH 3/2] push: allow --follow-tags to be set by config push.followTags","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-02-16T05:54:22Z","receivedAt":"2015-02-16T05:54:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 16, 2015 at 12:45:50AM -0500, Jeff King wrote:\n\n> On Mon, Feb 16, 2015 at 12:20:49AM -0500, Jeff King wrote:\n> \n> > But here you are adding to git_default_push_config, which is in another\n> > file.\n> > \n> > I'm trying to figure out why git_default_push_config exists at all. The\n> > major difference from git_push_config is that the \"default\" variant will\n> > get loaded for _all_ commands, not just \"push\". So if it affected\n> > variables that were used by other commands, it would be needed. But all\n> > it sets is push_default, which seems to be specific to builtin/push.c.\n> > \n> > So I suspect it can be removed entirely, and folded into\n> > git_config_push. But that's outside the scope of your patch.\n> \n> Here's that cleanup, plus another one I noticed while doing it.\n> \n> [1/2]: git_push_config: drop cargo-culted wt_status pointer\n> [2/2]: builtin/push.c: make push_default a static variable\n\nAnd here's what your patch would look like rebased on top. Two nits,\nthough. One, it could probably use a few basic tests.\n\nAnd two, the way that the config and --follow-tags interact is a little\nnon-obvious (as evidenced by the fact that you needed a comment to\nexplain what was going on).\n\nOne way to do it would be similar to how \"atomic\" is implemented: use\nOPT_BOOL to set an int, and then pick up the final value of that int\nafter config and command-line parsing is done. Then a reader does not\nhave to wonder why the \"follow_tags\" variable is not set by\n\"--follow-tags\".\n\nOr alternatively, we could pull the \"flags\" field from cmd_push out into\na static global \"transport_flags\", and manipulate it directly from the\nconfig (or if we don't like a global, pass it via the config-callback\nvoid pointer; but certainly a global is more common in git for code like\nthis). Then we do not have to worry about propagating values from\nintegers into flag bits at all.\n\n-- >8 --\nFrom: Dave Olszewski \nSubject: push: allow --follow-tags to be set by config push.followTags\n\nSigned-off-by: Dave Olszewski \n---\n Documentation/config.txt | 6 ++++++\n Documentation/git-push.txt | 5 ++++-\n builtin/push.c | 11 +++++++++++\n contrib/completion/git-completion.bash | 1 +\n 4 files changed, 22 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex ae6791d..e01d21c 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2079,6 +2079,12 @@ new default).\n \n --\n \n+push.followTags::\n+\tIf set to true enable '--follow-tags' option by default. You\n+\tmay override this configuration at time of push by specifying\n+\t'--no-follow-tags'.\n+\n+\n rebase.stat::\n \tWhether to show a diffstat of what changed upstream since the last\n \trebase. False by default.\ndiff --git a/Documentation/git-push.txt b/Documentation/git-push.txt\nindex ea97576..caa187b 100644\n--- a/Documentation/git-push.txt\n+++ b/Documentation/git-push.txt\n@@ -128,7 +128,10 @@ already exists on the remote side.\n \tPush all the refs that would be pushed without this option,\n \tand also push annotated tags in `refs/tags` that are missing\n \tfrom the remote but are pointing at commit-ish that are\n-\treachable from the refs being pushed.\n+\treachable from the refs being pushed. This can also be specified\n+\twith configuration variable 'push.followTags'. For more\n+\tinformation, see 'push.followTags' in linkgit:git-config[1].\n+\n \n --signed::\n \tGPG-sign the push request to update refs on the receiving\ndiff --git a/builtin/push.c b/builtin/push.c\nindex ab99f4c..7ddf4dd 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -20,6 +20,7 @@ static int deleterefs;\n static const char *receivepack;\n static int verbosity;\n static int progress = -1;\n+static int follow_tags;\n \n static struct push_cas_option cas;\n \n@@ -511,6 +512,11 @@ static int git_push_config(const char *k, const char *v, void *cb)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(k, \"push.followtags\")) {\n+\t\tfollow_tags = git_config_bool(k, v);\n+\t\treturn 0;\n+\t}\n+\n \treturn git_default_config(k, v, NULL);\n }\n \n@@ -557,6 +563,11 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n \n \tpacket_trace_identity(\"push\");\n \tgit_config(git_push_config, NULL);\n+\n+\t/* set TRANSPORT_PUSH_FOLLOW_TAGS in flags so that --no-follow-tags may unset it */\n+\tif (follow_tags)\n+\t\tflags |= TRANSPORT_PUSH_FOLLOW_TAGS;\n+\n \targc = parse_options(argc, argv, prefix, options, push_usage, 0);\n \n \tif (deleterefs && (tags || (flags & (TRANSPORT_PUSH_ALL | TRANSPORT_PUSH_MIRROR))))\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex c21190d..cffb2b8 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -2188,6 +2188,7 @@ _git_config ()\n \t\tpull.octopus\n \t\tpull.twohead\n \t\tpush.default\n+\t\tpush.followTags\n \t\trebase.autosquash\n \t\trebase.stat\n \t\treceive.autogc\n-- \n2.3.0.rc1.287.g761fd19\n"},{"id":"256125","messageId":"CAPc5daXU6x2ok+XqXDkWWi5O2N_dr+deQtOMx+Eh16UUGi5yJQ@mail.gmail.com","threadId":"38571","inReplyTo":"20150216054754.GB25088@peff.net","subject":"Re: [PATCH 2/2] builtin/push.c: make push_default a static variable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-02-16T05:57:03Z","receivedAt":"2015-02-16T05:57:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"On Sun, Feb 15, 2015 at 9:47 PM, Jeff King wrote:\n> When the \"push_default\" flag was originally added, it was\n> made globally visible to all code. This might have been\n> useful if other commands or library calls ended up depending\n> on it, but as it turns out, only builtin/push.c cares.\n> ...\n> Signed-off-by: Jeff King \n> ---\n> We know this is safe because no other callers needed tweaked when the\n> variable went out of scope. :) It would only be a bad idea if we\n> were planning on having other code in the future depend on push_default\n> (e.g., the code in remote.c to find the push destination). But it does\n> not seem to have needed that in the intervening years, so it's probably\n> fine to do this cleanup now.\n\nYay. Great minds think alike ;-)\n\n\"It definitely smells wrong to touch environment.c and cache.h\" was my\nfirst reaction to the \"follow-tags config\" patch, and I really think this shows\nthe right way forward.\n\nThanks.\n"},{"id":"256126","messageId":"CAPc5daU6VOmuNp3VbYgoFDXJshkC2AnRsZQQdoRMArYpezZr=A@mail.gmail.com","threadId":"38571","inReplyTo":"20150216055422.GB24611@peff.net","subject":"Re: [PATCH 3/2] push: allow --follow-tags to be set by config push.followTags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-02-16T06:02:08Z","receivedAt":"2015-02-16T06:02:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"On Sun, Feb 15, 2015 at 9:54 PM, Jeff King wrote:\n>\n> Or alternatively, we could pull the \"flags\" field from cmd_push out into\n> a static global \"transport_flags\", and manipulate it directly from the\n> config (or if we don't like a global, pass it via the config-callback\n> void pointer; but certainly a global is more common in git for code like\n> this). Then we do not have to worry about propagating values from\n> integers into flag bits at all.\n\nYup, that would be my preference. The largest problem I had with the\noriginal change was how to ensure that future new code would not\nmistakenly set the global follow_tags _without_ letting the command\nline option parser to override it. If the config parser flips the bit in the\nsame flags, it would become much less likely for future code to make\nsuch a mistake.\n\nThanks.\n"},{"id":"256127","messageId":"20150216061051.GA29895@peff.net","threadId":"38571","inReplyTo":"CAPc5daU6VOmuNp3VbYgoFDXJshkC2AnRsZQQdoRMArYpezZr=A@mail.gmail.com","subject":"[PATCH 0/3] cleaner bit-setting in cmd_push","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-02-16T06:10:51Z","receivedAt":"2015-02-16T06:10:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Feb 15, 2015 at 10:02:08PM -0800, Junio C Hamano wrote:\n\n> On Sun, Feb 15, 2015 at 9:54 PM, Jeff King wrote:\n> >\n> > Or alternatively, we could pull the \"flags\" field from cmd_push out into\n> > a static global \"transport_flags\", and manipulate it directly from the\n> > config (or if we don't like a global, pass it via the config-callback\n> > void pointer; but certainly a global is more common in git for code like\n> > this). Then we do not have to worry about propagating values from\n> > integers into flag bits at all.\n> \n> Yup, that would be my preference. The largest problem I had with the\n> original change was how to ensure that future new code would not\n> mistakenly set the global follow_tags _without_ letting the command\n> line option parser to override it. If the config parser flips the bit in the\n> same flags, it would become much less likely for future code to make\n> such a mistake.\n\nSo here's my take on it (on top of the two-patch series I just sent).\nDave's patch is 3rd here, just to show its rebased form, but do not take\nthat as a final endorsement. I still think it could use tests, but I\nwill let him write them. I am just doing the cleanup in the area, none\nof which needs to be his problem. :)\n\n [1/3]: cmd_push: set \"atomic\" bit directly\n [2/3]: cmd_push: pass \"flags\" pointer to config callback\n [3/3]: push: allow --follow-tags to be set by config push.followTags\n\n-Peff\n"},{"id":"256128","messageId":"CAPc5daUX4Jb6xdmv1jLUM28KJCfYAdTCUiqqrk04pTw=89O9YQ@mail.gmail.com","threadId":"38571","inReplyTo":"CAPc5daU6VOmuNp3VbYgoFDXJshkC2AnRsZQQdoRMArYpezZr=A@mail.gmail.com","subject":"Re: [PATCH 3/2] push: allow --follow-tags to be set by config push.followTags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-02-16T06:11:21Z","receivedAt":"2015-02-16T06:11:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"On Sun, Feb 15, 2015 at 10:02 PM, Junio C Hamano wrote:\n> On Sun, Feb 15, 2015 at 9:54 PM, Jeff King wrote:\n>>\n>> Or alternatively, we could pull the \"flags\" field from cmd_push out into\n>> a static global \"transport_flags\", and manipulate it directly from the\n>> config (or if we don't like a global, pass it via the config-callback\n>> void pointer; but certainly a global is more common in git for code like\n>> this). Then we do not have to worry about propagating values from\n>> integers into flag bits at all.\n>\n> Yup, that would be my preference. The largest problem I had with the\n> original change was how to ensure that future new code would not\n> mistakenly set the global follow_tags _without_ letting the command\n> line option parser to override it. If the config parser flips the bit in the\n> same flags, it would become much less likely for future code to make\n> such a mistake.\n\nHaving said that, I think this version is good enough.\n\nUnlike a global in environment.c (that is named not-so-specifically that\nanybody can set by reading the configuration file) that can be overriden\nonly by command line parser used only for \"git push\", the global int and\nthe flags are both localized to \"git push\" in this version, and there is\nmuch less chance to introduce new buggy code that forgets the command\nline override.\n\nThanks, again.\n"},{"id":"256129","messageId":"20150216061204.GA32381@peff.net","threadId":"38571","inReplyTo":"20150216061051.GA29895@peff.net","subject":"[PATCH 1/3] cmd_push: set \"atomic\" bit directly","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-02-16T06:12:04Z","receivedAt":"2015-02-16T06:12:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This makes the code shorter and more obvious by removing an\nunnecessary interim variable.\n\nSigned-off-by: Jeff King \n---\n builtin/push.c | 6 +-----\n 1 file changed, 1 insertion(+), 5 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex ab99f4c..f558c2e 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -519,7 +519,6 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n \tint flags = 0;\n \tint tags = 0;\n \tint rc;\n-\tint atomic = 0;\n \tconst char *repo = NULL;\t/* default repository */\n \tstruct option options[] = {\n \t\tOPT__VERBOSITY(&verbosity),\n@@ -551,7 +550,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n \t\tOPT_BIT(0, \"follow-tags\", &flags, N_(\"push missing but relevant tags\"),\n \t\t\tTRANSPORT_PUSH_FOLLOW_TAGS),\n \t\tOPT_BIT(0, \"signed\", &flags, N_(\"GPG sign the push\"), TRANSPORT_PUSH_CERT),\n-\t\tOPT_BOOL(0, \"atomic\", &atomic, N_(\"request atomic transaction on remote side\")),\n+\t\tOPT_BIT(0, \"atomic\", &flags, N_(\"request atomic transaction on remote side\"), TRANSPORT_PUSH_ATOMIC),\n \t\tOPT_END()\n \t};\n \n@@ -567,9 +566,6 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n \tif (tags)\n \t\tadd_refspec(\"refs/tags/*\");\n \n-\tif (atomic)\n-\t\tflags |= TRANSPORT_PUSH_ATOMIC;\n-\n \tif (argc > 0) {\n \t\trepo = argv[0];\n \t\tset_refspecs(argv + 1, argc - 1, repo);\n-- \n2.3.0.rc1.287.g761fd19\n"},{"id":"256130","messageId":"20150216061325.GB32381@peff.net","threadId":"38571","inReplyTo":"20150216061051.GA29895@peff.net","subject":"[PATCH 2/3] cmd_push: pass \"flags\" pointer to config callback","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-02-16T06:13:25Z","receivedAt":"2015-02-16T06:13:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This will let us manipulate any transport flags which have matching\nconfig options (there are none yet, but we will add one in\nthe next patch).\n\nWe could also just make \"flags\" a static file-scope global,\nbut the result is a little confusing. We end up passing it\nalong through do_push and push_with_options, each of which\nfurther munge it. Having slightly-differing versions of the\nflags variable available to those functions would probably\ncause more confusion than it is worth. Let's just keep the\noriginal local to cmd_push, and it can continue to pass it\nthrough the call-stack.\n\nSigned-off-by: Jeff King \n---\nI was also tempted to just remove the passing of \"flags\" through the\ncall stack entirely, and just have everybody touch a global\ntransport_flags. That is a much bigger change, and less obviously\ncorrect (after a callee munges their local version, do we ever care\nabout seeing the original in the caller?). I don't think so.\n\nTo be honest, the whole do_push is confusing to me. It seems like that\nshould just be part of cmd_push.\n\n builtin/push.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex f558c2e..c25108f 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -555,7 +555,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n \t};\n \n \tpacket_trace_identity(\"push\");\n-\tgit_config(git_push_config, NULL);\n+\tgit_config(git_push_config, &flags);\n \targc = parse_options(argc, argv, prefix, options, push_usage, 0);\n \n \tif (deleterefs && (tags || (flags & (TRANSPORT_PUSH_ALL | TRANSPORT_PUSH_MIRROR))))\n-- \n2.3.0.rc1.287.g761fd19\n"},{"id":"256131","messageId":"20150216061619.GC32381@peff.net","threadId":"38571","inReplyTo":"20150216061051.GA29895@peff.net","subject":"[PATCH 3/3] push: allow --follow-tags to be set by config push.followTags","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-02-16T06:16:19Z","receivedAt":"2015-02-16T06:16:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"From: Dave Olszewski \n\nSigned-off-by: Dave Olszewski \n---\nAgain, this is just a preview. Dave should send the final when he thinks\nit is good.\n\nThe if/else I added to the config callback is kind of ugly. I wonder if\nwe should have git_config_bit, or even just a function to set/clear a\nbit. Then the OPT_BIT code could use it, too. Something like:\n\n munge_bit(flags, TRANSPORT_PUSH_FOLLOW_TAGS, git_config_bool(k, v));\n\nOr maybe that is getting too fancy and obfuscated for a simple bit\nset/clear. I dunno.\n\n Documentation/config.txt | 6 ++++++\n Documentation/git-push.txt | 5 ++++-\n builtin/push.c | 9 +++++++++\n contrib/completion/git-completion.bash | 1 +\n 4 files changed, 20 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex ae6791d..e01d21c 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2079,6 +2079,12 @@ new default).\n \n --\n \n+push.followTags::\n+\tIf set to true enable '--follow-tags' option by default. You\n+\tmay override this configuration at time of push by specifying\n+\t'--no-follow-tags'.\n+\n+\n rebase.stat::\n \tWhether to show a diffstat of what changed upstream since the last\n \trebase. False by default.\ndiff --git a/Documentation/git-push.txt b/Documentation/git-push.txt\nindex ea97576..caa187b 100644\n--- a/Documentation/git-push.txt\n+++ b/Documentation/git-push.txt\n@@ -128,7 +128,10 @@ already exists on the remote side.\n \tPush all the refs that would be pushed without this option,\n \tand also push annotated tags in `refs/tags` that are missing\n \tfrom the remote but are pointing at commit-ish that are\n-\treachable from the refs being pushed.\n+\treachable from the refs being pushed. This can also be specified\n+\twith configuration variable 'push.followTags'. For more\n+\tinformation, see 'push.followTags' in linkgit:git-config[1].\n+\n \n --signed::\n \tGPG-sign the push request to update refs on the receiving\ndiff --git a/builtin/push.c b/builtin/push.c\nindex c25108f..6831c2d 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -482,6 +482,7 @@ static int option_parse_recurse_submodules(const struct option *opt,\n \n static int git_push_config(const char *k, const char *v, void *cb)\n {\n+\tint *flags = cb;\n \tint status;\n \n \tstatus = git_gpg_config(k, v, NULL);\n@@ -511,6 +512,14 @@ static int git_push_config(const char *k, const char *v, void *cb)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(k, \"push.followtags\")) {\n+\t\tif (git_config_bool(k, v))\n+\t\t\t*flags |= TRANSPORT_PUSH_FOLLOW_TAGS;\n+\t\telse\n+\t\t\t*flags &= ~TRANSPORT_PUSH_FOLLOW_TAGS;\n+\t\treturn 0;\n+\t}\n+\n \treturn git_default_config(k, v, NULL);\n }\n \ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex c21190d..cffb2b8 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -2188,6 +2188,7 @@ _git_config ()\n \t\tpull.octopus\n \t\tpull.twohead\n \t\tpush.default\n+\t\tpush.followTags\n \t\trebase.autosquash\n \t\trebase.stat\n \t\treceive.autogc\n-- \n2.3.0.rc1.287.g761fd19\n"},{"id":"256132","messageId":"20150216061736.GD32381@peff.net","threadId":"38571","inReplyTo":"CAPc5daUX4Jb6xdmv1jLUM28KJCfYAdTCUiqqrk04pTw=89O9YQ@mail.gmail.com","subject":"Re: [PATCH 3/2] push: allow --follow-tags to be set by config push.followTags","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-02-16T06:17:36Z","receivedAt":"2015-02-16T06:17:36Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Feb 15, 2015 at 10:11:21PM -0800, Junio C Hamano wrote:\n\n> On Sun, Feb 15, 2015 at 10:02 PM, Junio C Hamano wrote:\n> > On Sun, Feb 15, 2015 at 9:54 PM, Jeff King wrote:\n> >>\n> >> Or alternatively, we could pull the \"flags\" field from cmd_push out into\n> >> a static global \"transport_flags\", and manipulate it directly from the\n> >> config (or if we don't like a global, pass it via the config-callback\n> >> void pointer; but certainly a global is more common in git for code like\n> >> this). Then we do not have to worry about propagating values from\n> >> integers into flag bits at all.\n> >\n> > Yup, that would be my preference. The largest problem I had with the\n> > original change was how to ensure that future new code would not\n> > mistakenly set the global follow_tags _without_ letting the command\n> > line option parser to override it. If the config parser flips the bit in the\n> > same flags, it would become much less likely for future code to make\n> > such a mistake.\n> \n> Having said that, I think this version is good enough.\n\nToo late. :)\n\nI am OK if we leave it here, though, and drop the extra two patches I\njust sent.\n\n-Peff\n"},{"id":"256135","messageId":"xmqqr3tq72ui.fsf@gitster.dls.corp.google.com","threadId":"38571","inReplyTo":"20150216061325.GB32381@peff.net","subject":"Re: [PATCH 2/3] cmd_push: pass \"flags\" pointer to config callback","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-02-16T07:05:57Z","receivedAt":"2015-02-16T07:05:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> This will let us manipulate any transport flags which have matching\n> config options (there are none yet, but we will add one in\n> the next patch).\n\nNice---this will later lets us do push.atomic if we really wanted\nto, right?\n\n> To be honest, the whole do_push is confusing to me. It seems like that\n> should just be part of cmd_push.\n\nYeah, that part of the push callchain always confuses me every time\nI look at it. I think it was a consequence of how transport layer\nwas wedged into the existing codepath that only handled push that\ncalled send-pack to unify the codepaths that push calls into\ndifferent transport backends, and we may have done it differently\nand more cleanly if we were designing the push to transport to\nbackends from scratch.\n\n> builtin/push.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/builtin/push.c b/builtin/push.c\n> index f558c2e..c25108f 100644\n> --- a/builtin/push.c\n> +++ b/builtin/push.c\n> @@ -555,7 +555,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n> \t};\n> \n> \tpacket_trace_identity(\"push\");\n> -\tgit_config(git_push_config, NULL);\n> +\tgit_config(git_push_config, &flags);\n> \targc = parse_options(argc, argv, prefix, options, push_usage, 0);\n> \n> \tif (deleterefs && (tags || (flags & (TRANSPORT_PUSH_ALL | TRANSPORT_PUSH_MIRROR))))\n"},{"id":"256137","messageId":"20150216071638.GA818@peff.net","threadId":"38571","inReplyTo":"xmqqr3tq72ui.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 2/3] cmd_push: pass \"flags\" pointer to config callback","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-02-16T07:16:39Z","receivedAt":"2015-02-16T07:16:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Feb 15, 2015 at 11:05:57PM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > This will let us manipulate any transport flags which have matching\n> > config options (there are none yet, but we will add one in\n> > the next patch).\n> \n> Nice---this will later lets us do push.atomic if we really wanted\n> to, right?\n\nYes, exactly. Or push.signed, or whatever.\n\n> > To be honest, the whole do_push is confusing to me. It seems like that\n> > should just be part of cmd_push.\n> \n> Yeah, that part of the push callchain always confuses me every time\n> I look at it. I think it was a consequence of how transport layer\n> was wedged into the existing codepath that only handled push that\n> called send-pack to unify the codepaths that push calls into\n> different transport backends, and we may have done it differently\n> and more cleanly if we were designing the push to transport to\n> backends from scratch.\n\nI took a very cursory look at folding do_push into cmd_push. It's not\n_too_ bad. You wouldn't want to fold push_with_options in, as that gets\ncalled from a loop (you could make it the loop body, but I think it is\nmore clear as-is).\n\nHowever, it is really do_push which continues to manipulate the flags\nand set up the push, so that is the bit that should be folded in. And\nthen it would be fine to make transport_flags a global, and\npush_with_options could just use it directly, I think.\n\n-Peff\n"},{"id":"256195","messageId":"20150217104628.GA25978@peff.net","threadId":"38571","inReplyTo":"20150216054754.GB25088@peff.net","subject":"Re: [PATCH 2/2] builtin/push.c: make push_default a static variable","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-02-17T10:46:28Z","receivedAt":"2015-02-17T10:46:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 16, 2015 at 12:47:54AM -0500, Jeff King wrote:\n\n> When the \"push_default\" flag was originally added, it was\n> made globally visible to all code. This might have been\n> useful if other commands or library calls ended up depending\n> on it, but as it turns out, only builtin/push.c cares.\n> \n> Let's make it a static variable in builtin/push.c.\n>\n> [...]\n> \n> ---\n> We know this is safe because no other callers needed tweaked when the\n> variable went out of scope. :) It would only be a bad idea if we\n> were planning on having other code in the future depend on push_default\n> (e.g., the code in remote.c to find the push destination). But it does\n> not seem to have needed that in the intervening years, so it's probably\n> fine to do this cleanup now.\n\nI had a nagging feeling that there was some code which wanted to use\nthis elsewhere, and I did finally find it, when I merged this topic with\nmy other personal topics.\n\nIf we wanted to implement \"@{push}\" (or \"@{publish}\") to mean \"the\ntracking ref of the remote ref you would push to if you ran git-push\",\nthen this is a step in the wrong direction.\n\nThe patches I posted last January (and which you carried as\njk/branch-at-publish for a while) do work, and I've used the feature\nonce or twice since then. From the discussion, it looks like they were\nmeant to be a building block for more triangular-flow work, but I don't\nremember what else was needed. I'm tempted to resurrect them, but it's\nnot a high priority for me.\n\nAnyway, food for thought on whether we want to do this cleanup or not,\nthen. We can always leave this here as part of git_default_config, and\nstill move Dave's new option into git_push_config.\n\n-Peff\n"},{"id":"256221","messageId":"xmqqsie4300s.fsf@gitster.dls.corp.google.com","threadId":"38571","inReplyTo":"20150217104628.GA25978@peff.net","subject":"Re: [PATCH 2/2] builtin/push.c: make push_default a static variable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-02-17T17:45:07Z","receivedAt":"2015-02-17T17:45:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> If we wanted to implement \"@{push}\" (or \"@{publish}\") to mean \"the\n> tracking ref of the remote ref you would push to if you ran git-push\",\n> then this is a step in the wrong direction.\n\nIs that because push_default variable needs to be looked at from\nsha1_name.c when resolving \"@{push}\", optionally prefixed with the\nname of the branch? I wonder if that codepath should know the gory\ndetails of which ref at the remote the branch is pushed to and which\nremote-tracking ref we use in the local repository to mirror that\nremote ref in the first place?\n\nWhat do we do for the @{upstream} side of the things---it calls\nbranch_get() and when the branch structure is returned, the details\nhave been computed for us so get_upstream_branch() only needs to use\nthe information already computed. The interesting parts of the\ncomputation all happen inside remote.c, it seems.\n\nSo we probably would do something similar to @{push} side, which\nwould mean that push_default variable and the logic needs to be\nvisible to remote.c if we want to have the helper that is similar to\nset_merge() that is used from branch_get() to support @{upstream}.\n\nHmmm, I have a feeling that \"with default configuration, where does\n'git push' send this branch to?\" logic should be contained within\nthe source file whose name has \"push\" in it and exposed as a helper\nfunction, instead of exposing just one of the lowest level knob\npush_default to outside callers and have them figure things out.\n\nViewed from that angle, it might be the case that remote.c knows too\nmuch about what happens during fetch and pull, but I dunno.\n"},{"id":"256225","messageId":"20150217182324.GA12816@peff.net","threadId":"38571","inReplyTo":"xmqqsie4300s.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 2/2] builtin/push.c: make push_default a static variable","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-02-17T18:23:25Z","receivedAt":"2015-02-17T18:23:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 17, 2015 at 09:45:07AM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > If we wanted to implement \"@{push}\" (or \"@{publish}\") to mean \"the\n> > tracking ref of the remote ref you would push to if you ran git-push\",\n> > then this is a step in the wrong direction.\n> \n> Is that because push_default variable needs to be looked at from\n> sha1_name.c when resolving \"@{push}\", optionally prefixed with the\n> name of the branch?\n\nYes, exactly.\n\n> I wonder if that codepath should know the gory details of which ref at\n> the remote the branch is pushed to and which remote-tracking ref we\n> use in the local repository to mirror that remote ref in the first\n> place?\n\nI think that was one of the ugly bits from the series; that we had to\nreimplement \"where would we push\" and \"what would it be called if we\npushed and then fetched\"? The former cares about push_default, and the\nlatter has to apply push and then fetch refspecs.\n\nIf you want to peek at it again, it's at:\n\n https://github.com/peff/git/commit/8859afb1af63cb3cb0bc4cc8c1719c2011f406c9\n\n(but note that it should not be called @{publish}, as per earlier\ndiscussions).\n\n> What do we do for the @{upstream} side of the things---it calls\n> branch_get() and when the branch structure is returned, the details\n> have been computed for us so get_upstream_branch() only needs to use\n> the information already computed. The interesting parts of the\n> computation all happen inside remote.c, it seems.\n> \n> So we probably would do something similar to @{push} side, which\n> would mean that push_default variable and the logic needs to be\n> visible to remote.c if we want to have the helper that is similar to\n> set_merge() that is used from branch_get() to support @{upstream}.\n\nSure, we could go that way. But I don't think it changes the issue for\n_this_ patch series, which is that the variable needs visibility outside\nof builtin/push.c (and we need to load the config for programs besides\ngit-push).\n\n> Hmmm, I have a feeling that \"with default configuration, where does\n> 'git push' send this branch to?\" logic should be contained within\n> the source file whose name has \"push\" in it and exposed as a helper\n> function, instead of exposing just one of the lowest level knob\n> push_default to outside callers and have them figure things out.\n> \n> Viewed from that angle, it might be the case that remote.c knows too\n> much about what happens during fetch and pull, but I dunno.\n\nYeah, it would be nice if there were a convenient lib-ified set of\nfunctions for getting this information, and \"fetch\" and \"push\" commands\nwere built on top of it. I don't know how painful that would be, though.\nThe existing code has grown somewhat organically.\n\nBut even with that change, the lib-ified code needs to hook into\ngit_default_config (or do its own config lookup) so that we get the\nproper value no matter who the caller is.\n\n-Peff\n"},{"id":"256242","messageId":"xmqqzj8cyyip.fsf@gitster.dls.corp.google.com","threadId":"38571","inReplyTo":"20150217182324.GA12816@peff.net","subject":"Re: [PATCH 2/2] builtin/push.c: make push_default a static variable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-02-17T22:16:30Z","receivedAt":"2015-02-17T22:16:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> So we probably would do something similar to @{push} side, which\n>> would mean that push_default variable and the logic needs to be\n>> visible to remote.c if we want to have the helper that is similar to\n>> set_merge() that is used from branch_get() to support @{upstream}.\n>\n> Sure, we could go that way. But I don't think it changes the issue for\n> _this_ patch series, which is that the variable needs visibility outside\n> of builtin/push.c (and we need to load the config for programs besides\n> git-push).\n\nI do not disagree. push_default and other things that affect the\ncomputation needs to be visible to the code that implements the\nlogic.\n\nDo you want to resurrect that @{publish} stuff? I think it had\nsensible semantics, and I do not think we mind keeping the\npush_default configuration to be read from the default_config\ncodepath.\n\nIf we decide to go that route, then the series would become\nsomething like this:\n\n$gmane/263871 [1/4] git_push_config: drop cargo-culted wt_status pointer\n$gmane/263878 [2/4] cmd_push: set \"atomic\" bit directly\n$gmane/263879 [3/4] cmd_push: pass \"flags\" pointer to config callback\n$gmane/263880 [4/4] push: allow --follow-tags to be set by config push.followTags\n\nomitting the original 2/2 patch we are discussing. I am inclined to\nreplace what I queued with the above four.\n\nThe last one needs a bit of tweaking and should look like the\nattached. Again, as you wrote in $gmane/263880, this is just a\npreview. Dave should send the final when he thinks it is good,\npossibly with some tests.\n\n-- >8 --\nFrom: Dave Olszewski \nDate: Mon, 16 Feb 2015 01:16:19 -0500\nSubject: [PATCH] [NEEDSACK] push: allow --follow-tags to be set by config push.followTags\n\nSigned-off-by: Dave Olszewski \nSigned-off-by: Junio C Hamano \n---\n Documentation/config.txt | 6 ++++++\n Documentation/git-push.txt | 5 ++++-\n builtin/push.c | 10 ++++++++++\n contrib/completion/git-completion.bash | 1 +\n 4 files changed, 21 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex ae6791d..e01d21c 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2079,6 +2079,12 @@ new default).\n \n --\n \n+push.followTags::\n+\tIf set to true enable '--follow-tags' option by default. You\n+\tmay override this configuration at time of push by specifying\n+\t'--no-follow-tags'.\n+\n+\n rebase.stat::\n \tWhether to show a diffstat of what changed upstream since the last\n \trebase. False by default.\ndiff --git a/Documentation/git-push.txt b/Documentation/git-push.txt\nindex ea97576..caa187b 100644\n--- a/Documentation/git-push.txt\n+++ b/Documentation/git-push.txt\n@@ -128,7 +128,10 @@ already exists on the remote side.\n \tPush all the refs that would be pushed without this option,\n \tand also push annotated tags in `refs/tags` that are missing\n \tfrom the remote but are pointing at commit-ish that are\n-\treachable from the refs being pushed.\n+\treachable from the refs being pushed. This can also be specified\n+\twith configuration variable 'push.followTags'. For more\n+\tinformation, see 'push.followTags' in linkgit:git-config[1].\n+\n \n --signed::\n \tGPG-sign the push request to update refs on the receiving\ndiff --git a/builtin/push.c b/builtin/push.c\nindex bba22b8..57c138b 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -473,11 +473,21 @@ static int option_parse_recurse_submodules(const struct option *opt,\n \n static int git_push_config(const char *k, const char *v, void *cb)\n {\n+\tint *flags = cb;\n \tint status;\n \n \tstatus = git_gpg_config(k, v, NULL);\n \tif (status)\n \t\treturn status;\n+\n+\tif (!strcmp(k, \"push.followtags\")) {\n+\t\tif (git_config_bool(k, v))\n+\t\t\t*flags |= TRANSPORT_PUSH_FOLLOW_TAGS;\n+\t\telse\n+\t\t\t*flags &= ~TRANSPORT_PUSH_FOLLOW_TAGS;\n+\t\treturn 0;\n+\t}\n+\n \treturn git_default_config(k, v, NULL);\n }\n \ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex c21190d..cffb2b8 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -2188,6 +2188,7 @@ _git_config ()\n \t\tpull.octopus\n \t\tpull.twohead\n \t\tpush.default\n+\t\tpush.followTags\n \t\trebase.autosquash\n \t\trebase.stat\n \t\treceive.autogc\n-- \n2.3.0-283-g21bf3f5\n"},{"id":"256292","messageId":"20150218185007.GA7257@peff.net","threadId":"38571","inReplyTo":"xmqqzj8cyyip.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 2/2] builtin/push.c: make push_default a static variable","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-02-18T18:50:08Z","receivedAt":"2015-02-18T18:50:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 17, 2015 at 02:16:30PM -0800, Junio C Hamano wrote:\n\n> Do you want to resurrect that @{publish} stuff? I think it had\n> sensible semantics, and I do not think we mind keeping the\n> push_default configuration to be read from the default_config\n> codepath.\n\nI'll take a look at it and see if it's in good enough shape to apply\nas-is, or with minor tweaking. But regardless, let's...\n\n> If we decide to go that route, then the series would become\n> something like this:\n> \n> $gmane/263871 [1/4] git_push_config: drop cargo-culted wt_status pointer\n> $gmane/263878 [2/4] cmd_push: set \"atomic\" bit directly\n> $gmane/263879 [3/4] cmd_push: pass \"flags\" pointer to config callback\n> $gmane/263880 [4/4] push: allow --follow-tags to be set by config push.followTags\n> \n> omitting the original 2/2 patch we are discussing. I am inclined to\n> replace what I queued with the above four.\n\n...do this. Even if we don't apply other patches to make use of\npush_default immediately, it's a plausible area for us to touch later,\nand the cleanup from the dropped patch was not so important.\n\n> +\tif (!strcmp(k, \"push.followtags\")) {\n> +\t\tif (git_config_bool(k, v))\n> +\t\t\t*flags |= TRANSPORT_PUSH_FOLLOW_TAGS;\n> +\t\telse\n> +\t\t\t*flags &= ~TRANSPORT_PUSH_FOLLOW_TAGS;\n> +\t\treturn 0;\n> +\t}\n\nDid you have an opinion on sticking this behind a helper function?\n\nIt feels like a lot of repeating of the same variables and flags, but I\nworried that \"munge_bit\" ends up being even more confusing.\n\n-Peff\n"},{"id":"256300","messageId":"xmqqh9uj2g25.fsf@gitster.dls.corp.google.com","threadId":"38571","inReplyTo":"20150218185007.GA7257@peff.net","subject":"Re: [PATCH 2/2] builtin/push.c: make push_default a static variable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-02-18T19:08:34Z","receivedAt":"2015-02-18T19:08:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> +\tif (!strcmp(k, \"push.followtags\")) {\n>> +\t\tif (git_config_bool(k, v))\n>> +\t\t\t*flags |= TRANSPORT_PUSH_FOLLOW_TAGS;\n>> +\t\telse\n>> +\t\t\t*flags &= ~TRANSPORT_PUSH_FOLLOW_TAGS;\n>> +\t\treturn 0;\n>> +\t}\n>\n> Did you have an opinion on sticking this behind a helper function?\n\nNot very strongly either way. Seeing the above does not bother me\ntoo much, but I do not know how I would feel when I start seeing\n\n\tval = git_config_book(k, v);\n\tflip_bool(val, &flags, TRANSPORT_PUSH_FOLLOW_TAGS);\n\noften. Not having to make sure that the bit constant whose name\ntends to get long is not misspelled is certainly a plus.\n"},{"id":"256304","messageId":"20150218192518.GA7891@peff.net","threadId":"38571","inReplyTo":"xmqqh9uj2g25.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 2/2] builtin/push.c: make push_default a static variable","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-02-18T19:25:18Z","receivedAt":"2015-02-18T19:25:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 18, 2015 at 11:08:34AM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> >> +\tif (!strcmp(k, \"push.followtags\")) {\n> >> +\t\tif (git_config_bool(k, v))\n> >> +\t\t\t*flags |= TRANSPORT_PUSH_FOLLOW_TAGS;\n> >> +\t\telse\n> >> +\t\t\t*flags &= ~TRANSPORT_PUSH_FOLLOW_TAGS;\n> >> +\t\treturn 0;\n> >> +\t}\n> >\n> > Did you have an opinion on sticking this behind a helper function?\n> \n> Not very strongly either way. Seeing the above does not bother me\n> too much, but I do not know how I would feel when I start seeing\n> \n> \tval = git_config_book(k, v);\n> \tflip_bool(val, &flags, TRANSPORT_PUSH_FOLLOW_TAGS);\n> \n> often. Not having to make sure that the bit constant whose name\n> tends to get long is not misspelled is certainly a plus.\n\nI think it would be even nicer as:\n\n git_config_bits(k, v, &flags, TRANSPORT_PUSH_FOLLOW_TAGS);\n\nThere is a similar spot in the tar.*.remote config. And that could of\ncourse build on a \"flip_bool\" or similar, which itself has many other\nuses. But after taking a quick peek, I noticed that one call around\ndiff.c:3600 would look like:\n\n flip_bool(!negate, &opt->filter, bit);\n\nIOW, it is the same pattern of conditional, but it flips the AND and OR,\nbecause its flag is flipped. Reading that line makes me head hurt,\nbecause we've really introduced an extra double-negative into the flow.\n\nThat \"negate\" flag is local to the loop we are in, and we could flip it\nfor clarity. But it makes me second-guess the technique.\n\n-Peff\n"},{"id":"256311","messageId":"xmqq4mqj2e4j.fsf@gitster.dls.corp.google.com","threadId":"38571","inReplyTo":"20150218192518.GA7891@peff.net","subject":"Re: [PATCH 2/2] builtin/push.c: make push_default a static variable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-02-18T19:50:20Z","receivedAt":"2015-02-18T19:50:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Wed, Feb 18, 2015 at 11:08:34AM -0800, Junio C Hamano wrote:\n> ...\n>> Not very strongly either way. Seeing the above does not bother me\n>> too much, but I do not know how I would feel when I start seeing\n>> \n>> \tval = git_config_book(k, v);\n>> \tflip_bool(val, &flags, TRANSPORT_PUSH_FOLLOW_TAGS);\n>> \n>> often. Not having to make sure that the bit constant whose name\n>> tends to get long is not misspelled is certainly a plus.\n>\n> I think it would be even nicer as:\n>\n> git_config_bits(k, v, &flags, TRANSPORT_PUSH_FOLLOW_TAGS);\n\nMaybe. I do not feel very strongly either way.\n"},{"id":"256313","messageId":"20150218200340.GA11861@peff.net","threadId":"38571","inReplyTo":"xmqq4mqj2e4j.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 2/2] builtin/push.c: make push_default a static variable","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-02-18T20:03:41Z","receivedAt":"2015-02-18T20:03:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 18, 2015 at 11:50:20AM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > On Wed, Feb 18, 2015 at 11:08:34AM -0800, Junio C Hamano wrote:\n> > ...\n> >> Not very strongly either way. Seeing the above does not bother me\n> >> too much, but I do not know how I would feel when I start seeing\n> >> \n> >> \tval = git_config_book(k, v);\n> >> \tflip_bool(val, &flags, TRANSPORT_PUSH_FOLLOW_TAGS);\n> >> \n> >> often. Not having to make sure that the bit constant whose name\n> >> tends to get long is not misspelled is certainly a plus.\n> >\n> > I think it would be even nicer as:\n> >\n> > git_config_bits(k, v, &flags, TRANSPORT_PUSH_FOLLOW_TAGS);\n> \n> Maybe. I do not feel very strongly either way.\n\nSo I started to prepare a patch for this, because I wanted to see how it\nwould look. It's below in case you are curious, but note that it doesn't\ncompile, and that is does something a bit dangerous. So I think I am\ngiving up on this line of thought. Read on if you're curious.\n\nThe sticking point is the type of the bit-field. If it is:\n\n void flip_bits(int set_or_clear, int *field, int bits);\n\nthen we cannot pass a pointer to \"unsigned field\". We can pass unsigned\nbits, but note that we might lose the 32nd (or 64th) bit.\n\nWe can do this instead:\n\n void flip_bits(int set_or_clear, void *field, unsigned bits);\n\nBut of course we will end up dereferencing \"void *field\" as \"unsigned *\".\nI suspect if field is originally an \"int\" it works fine on most\nplatforms, but isn't legal according to the standard. Much worse,\nthough: if your original field is a different size, it's even less\nlikely to work. Passing a \"char *\" may set bits in random adjacent\nmemory (depending on your endianness), and an \"unsigned long *\" may set\nbits in the wrong part of the variable. And because of the \"void *\", we\nget no compiler warnings. :)\n\nAdd on top that we may want to use this for C bit-fields, whose address\ncannot legally be taken (and this is why it does not compile).\n\nSo besides adding type-specific bit-flippers (yuck), I think the only\nway to do it universally would be with a macro. Which I think tips it\nover the \"too gross, just write it out\" line.\n\n---\ndiff --git a/archive-tar.c b/archive-tar.c\nindex 0d1e6bd..3c794e2 100644\n--- a/archive-tar.c\n+++ b/archive-tar.c\n@@ -352,10 +352,7 @@ static int tar_filter_config(const char *var, const char *value, void *data)\n \t\treturn 0;\n \t}\n \tif (!strcmp(type, \"remote\")) {\n-\t\tif (git_config_bool(var, value))\n-\t\t\tar->flags |= ARCHIVER_REMOTE;\n-\t\telse\n-\t\t\tar->flags &= ~ARCHIVER_REMOTE;\n+\t\tgit_config_bits(var, value, &ar->flags, ARCHIVER_REMOTE);\n \t\treturn 0;\n \t}\n \ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex d816587..073445e 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -2216,10 +2216,8 @@ static int git_pack_config(const char *k, const char *v, void *cb)\n \t\treturn 0;\n \t}\n \tif (!strcmp(k, \"pack.writebitmaphashcache\")) {\n-\t\tif (git_config_bool(k, v))\n-\t\t\twrite_bitmap_options |= BITMAP_OPT_HASH_CACHE;\n-\t\telse\n-\t\t\twrite_bitmap_options &= ~BITMAP_OPT_HASH_CACHE;\n+\t\tgit_config_bits(k, v, &write_bitmap_options,\n+\t\t\t\tBITMAP_OPT_HASH_CACHE);\n \t}\n \tif (!strcmp(k, \"pack.usebitmaps\")) {\n \t\tuse_bitmap_index = git_config_bool(k, v);\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex 5878986..9d4eb24 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -53,10 +53,7 @@ static int mark_ce_flags(const char *path, int flag, int mark)\n \tint namelen = strlen(path);\n \tint pos = cache_name_pos(path, namelen);\n \tif (0 <= pos) {\n-\t\tif (mark)\n-\t\t\tactive_cache[pos]->ce_flags |= flag;\n-\t\telse\n-\t\t\tactive_cache[pos]->ce_flags &= ~flag;\n+\t\tflip_bits(mark, &active_cache[pos]->ce_flags, flag);\n \t\tactive_cache[pos]->ce_flags |= CE_UPDATE_IN_BASE;\n \t\tcache_tree_invalidate_path(&the_index, path);\n \t\tactive_cache_changed |= CE_ENTRY_CHANGED;\ndiff --git a/cache.h b/cache.h\nindex f704af5..957f150 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1316,6 +1316,18 @@ extern int sha1_object_info_extended(const unsigned char *, struct object_info *\n /* Dumb servers support */\n extern int update_server_info(int);\n \n+/*\n+ * Either set or clear \"bits\" from \"field\", based on\n+ * whether or not \"set_or_clear\" is set.\n+ */\n+static inline void flip_bits(int set_or_clear, void *field, unsigned bits)\n+{\n+\tif (set_or_clear)\n+\t\t*(unsigned *)field |= bits;\n+\telse\n+\t\t*(unsigned *)field &= ~bits;\n+}\n+\n /* git_config_parse_key() returns these negated: */\n #define CONFIG_INVALID_KEY 1\n #define CONFIG_NO_SECTION_OR_NAME 2\n@@ -1385,6 +1397,12 @@ struct config_include_data {\n #define CONFIG_INCLUDE_INIT { 0 }\n extern int git_config_include(const char *name, const char *value, void *data);\n \n+static inline int git_config_bits(const char *name, const char *value, void *field, unsigned bits)\n+{\n+\tflip_bits(git_config_bool(name, value), field, bits);\n+\treturn 0;\n+}\n+\n /*\n * Match and parse a config key of the form:\n *\ndiff --git a/column.c b/column.c\nindex 786abe6..21c9765 100644\n--- a/column.c\n+++ b/column.c\n@@ -281,12 +281,8 @@ static int parse_option(const char *arg, int len, unsigned int *colopts,\n \n \t\tif (opts[i].mask)\n \t\t\t*colopts = (*colopts & ~opts[i].mask) | opts[i].value;\n-\t\telse {\n-\t\t\tif (set)\n-\t\t\t\t*colopts |= opts[i].value;\n-\t\t\telse\n-\t\t\t\t*colopts &= ~opts[i].value;\n-\t\t}\n+\t\telse\n+\t\t\tflip_bits(set, colopts, opts[i].value);\n \t\treturn 0;\n \t}\n \ndiff --git a/combine-diff.c b/combine-diff.c\nindex 91edce5..ad52b77 100644\n--- a/combine-diff.c\n+++ b/combine-diff.c\n@@ -581,12 +581,9 @@ static int make_hunks(struct sline *sline, unsigned long cnt,\n \tunsigned long i;\n \tint has_interesting = 0;\n \n-\tfor (i = 0; i <= cnt; i++) {\n-\t\tif (interesting(&sline[i], all_mask))\n-\t\t\tsline[i].flag |= mark;\n-\t\telse\n-\t\t\tsline[i].flag &= ~mark;\n-\t}\n+\tfor (i = 0; i <= cnt; i++)\n+\t\tflip_bits(interesting(&sline[i], all_mask),\n+\t\t\t &sline[i].flag, mark);\n \tif (!dense)\n \t\treturn give_context(sline, cnt, num_parent);\n \ndiff --git a/diff.c b/diff.c\nindex d1bd534..53b9481 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3585,22 +3585,17 @@ static int parse_diff_filter_opt(const char *optarg, struct diff_options *opt)\n \n \tfor (i = 0; (optch = optarg[i]) != '\\0'; i++) {\n \t\tunsigned int bit;\n-\t\tint negate;\n+\t\tint positive = 1;\n \n \t\tif ('a' <= optch && optch <= 'z') {\n-\t\t\tnegate = 1;\n+\t\t\tpositive = 0;\n \t\t\toptch = toupper(optch);\n-\t\t} else {\n-\t\t\tnegate = 0;\n \t\t}\n \n \t\tbit = (0 <= optch && optch <= 'Z') ? filter_bit[optch] : 0;\n \t\tif (!bit)\n \t\t\treturn optarg[i];\n-\t\tif (negate)\n-\t\t\topt->filter &= ~bit;\n-\t\telse\n-\t\t\topt->filter |= bit;\n+\t\tflip_bits(positive, &opt->filter, bit);\n \t}\n \treturn 0;\n }\ndiff --git a/parse-options.c b/parse-options.c\nindex 80106c0..47c169c 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -100,17 +100,11 @@ static int get_value(struct parse_opt_ctx_t *p,\n \t\treturn (*(parse_opt_ll_cb *)opt->callback)(p, opt, unset);\n \n \tcase OPTION_BIT:\n-\t\tif (unset)\n-\t\t\t*(int *)opt->value &= ~opt->defval;\n-\t\telse\n-\t\t\t*(int *)opt->value |= opt->defval;\n+\t\tflip_bits(!unset, opt->value, opt->defval);\n \t\treturn 0;\n \n \tcase OPTION_NEGBIT:\n-\t\tif (unset)\n-\t\t\t*(int *)opt->value |= opt->defval;\n-\t\telse\n-\t\t\t*(int *)opt->value &= ~opt->defval;\n+\t\tflip_bits(unset, opt->value, opt->defval);\n \t\treturn 0;\n \n \tcase OPTION_COUNTUP:\ndiff --git a/revision.c b/revision.c\nindex 66520c6..85cb245 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -554,10 +554,8 @@ static int compact_treesame(struct rev_info *revs, struct commit *commit, unsign\n \t\tif (nth_parent != 0)\n \t\t\tdie(\"compact_treesame %u\", nth_parent);\n \t\told_same = !!(commit->object.flags & TREESAME);\n-\t\tif (rev_same_tree_as_empty(revs, commit))\n-\t\t\tcommit->object.flags |= TREESAME;\n-\t\telse\n-\t\t\tcommit->object.flags &= ~TREESAME;\n+\t\tflip_bits(rev_same_tree_as_empty(revs, commit),\n+\t\t\t &commit->object.flags, TREESAME);\n \t\treturn old_same;\n \t}\n \n@@ -578,10 +576,8 @@ static int compact_treesame(struct rev_info *revs, struct commit *commit, unsign\n \tif (--st->nparents == 1) {\n \t\tif (commit->parents->next)\n \t\t\tdie(\"compact_treesame parents mismatch\");\n-\t\tif (st->treesame[0] && revs->dense)\n-\t\t\tcommit->object.flags |= TREESAME;\n-\t\telse\n-\t\t\tcommit->object.flags &= ~TREESAME;\n+\t\tflip_bits(st->treesame[0] && revs->dense,\n+\t\t\t &commit->object.flags, TREESAME);\n \t\tfree(add_decoration(&revs->treesame, &commit->object, NULL));\n \t}\n \n@@ -609,10 +605,8 @@ static unsigned update_treesame(struct rev_info *revs, struct commit *commit)\n \t\t\t} else\n \t\t\t\tirrelevant_change |= !st->treesame[n];\n \t\t}\n-\t\tif (relevant_parents ? relevant_change : irrelevant_change)\n-\t\t\tcommit->object.flags &= ~TREESAME;\n-\t\telse\n-\t\t\tcommit->object.flags |= TREESAME;\n+\t\tflip_bits(!(relevant_parents ? relevant_change : irrelevant_change),\n+\t\t\t &commit->object.flags, TREESAME);\n \t}\n \n \treturn commit->object.flags & TREESAME;\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex be84ba2..9089c5b 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -248,10 +248,8 @@ static int apply_sparse_checkout(struct index_state *istate,\n {\n \tint was_skip_worktree = ce_skip_worktree(ce);\n \n-\tif (ce->ce_flags & CE_NEW_SKIP_WORKTREE)\n-\t\tce->ce_flags |= CE_SKIP_WORKTREE;\n-\telse\n-\t\tce->ce_flags &= ~CE_SKIP_WORKTREE;\n+\tflip_bits(ce->ce_flags & CE_NEW_SKIP_WORKTREE,\n+\t\t &ce->ce_flags, CE_SKIP_WORKTREE);\n \tif (was_skip_worktree != ce_skip_worktree(ce)) {\n \t\tce->ce_flags |= CE_UPDATE_IN_BASE;\n \t\tistate->cache_changed |= CE_ENTRY_CHANGED;\n@@ -982,10 +980,7 @@ static void mark_new_skip_worktree(struct exclude_list *el,\n \t\tif (select_flag && !(ce->ce_flags & select_flag))\n \t\t\tcontinue;\n \n-\t\tif (!ce_stage(ce))\n-\t\t\tce->ce_flags |= skip_wt_flag;\n-\t\telse\n-\t\t\tce->ce_flags &= ~skip_wt_flag;\n+\t\tflip_bits(!ce_stage(ce), &ce->ce_flags, skip_wt_flag);\n \t}\n \n \t/*\ndiff --git a/ws.c b/ws.c\nindex ea4b2b1..6c5f4bd 100644\n--- a/ws.c\n+++ b/ws.c\n@@ -30,14 +30,14 @@ unsigned parse_whitespace_rule(const char *string)\n \t\tint i;\n \t\tsize_t len;\n \t\tconst char *ep;\n-\t\tint negated = 0;\n+\t\tint positive = 1;\n \n \t\tstring = string + strspn(string, \", \\t\\n\\r\");\n \t\tep = strchrnul(string, ',');\n \t\tlen = ep - string;\n \n \t\tif (*string == '-') {\n-\t\t\tnegated = 1;\n+\t\t\tpositive = 0;\n \t\t\tstring++;\n \t\t\tlen--;\n \t\t}\n@@ -47,10 +47,8 @@ unsigned parse_whitespace_rule(const char *string)\n \t\t\tif (strncmp(whitespace_rule_names[i].rule_name,\n \t\t\t\t string, len))\n \t\t\t\tcontinue;\n-\t\t\tif (negated)\n-\t\t\t\trule &= ~whitespace_rule_names[i].rule_bits;\n-\t\t\telse\n-\t\t\t\trule |= whitespace_rule_names[i].rule_bits;\n+\t\t\tflip_bits(positive, &rule,\n+\t\t\t\t whitespace_rule_names[i].rule_bits);\n \t\t\tbreak;\n \t\t}\n \t\tif (strncmp(string, \"tabwidth=\", 9) == 0) {\n"},{"id":"257665","messageId":"xmqqh9toxgdd.fsf@gitster.dls.corp.google.com","threadId":"38571","inReplyTo":"20150216061619.GC32381@peff.net","subject":"Re: [PATCH 3/3] push: allow --follow-tags to be set by config push.followTags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-14T06:06:22Z","receivedAt":"2015-03-14T06:06:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> From: Dave Olszewski \n>\n> Signed-off-by: Dave Olszewski \n> ---\n> Again, this is just a preview. Dave should send the final when he thinks\n> it is good.\n\nDave?\n\nI do not see anything wrong with this version that builds on top of\nthe previous 2 clean-up. Personally I find that these clean-up\nchanges more valuable than I care about this particular feature, and\nit is unfortunate that waiting an Ack or reroll of this one kept\nthem stalled.\n\nI am tempted to throw \"Helped-by: Peff\" into the log message and\nmerge the result to 'next', unless I hear otherwise in a few days.\n\n> The if/else I added to the config callback is kind of ugly. I wonder if\n> we should have git_config_bit, or even just a function to set/clear a\n> bit. Then the OPT_BIT code could use it, too. Something like:\n>\n> munge_bit(flags, TRANSPORT_PUSH_FOLLOW_TAGS, git_config_bool(k, v));\n>\n> Or maybe that is getting too fancy and obfuscated for a simple bit\n> set/clear. I dunno.\n\nI think we agreed that the code we have in this series is good.\n\n> Documentation/config.txt | 6 ++++++\n> Documentation/git-push.txt | 5 ++++-\n> builtin/push.c | 9 +++++++++\n> contrib/completion/git-completion.bash | 1 +\n> 4 files changed, 20 insertions(+), 1 deletion(-)\n>\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index ae6791d..e01d21c 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -2079,6 +2079,12 @@ new default).\n> \n> --\n> \n> +push.followTags::\n> +\tIf set to true enable '--follow-tags' option by default. You\n> +\tmay override this configuration at time of push by specifying\n> +\t'--no-follow-tags'.\n> +\n> +\n> rebase.stat::\n> \tWhether to show a diffstat of what changed upstream since the last\n> \trebase. False by default.\n> diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt\n> index ea97576..caa187b 100644\n> --- a/Documentation/git-push.txt\n> +++ b/Documentation/git-push.txt\n> @@ -128,7 +128,10 @@ already exists on the remote side.\n> \tPush all the refs that would be pushed without this option,\n> \tand also push annotated tags in `refs/tags` that are missing\n> \tfrom the remote but are pointing at commit-ish that are\n> -\treachable from the refs being pushed.\n> +\treachable from the refs being pushed. This can also be specified\n> +\twith configuration variable 'push.followTags'. For more\n> +\tinformation, see 'push.followTags' in linkgit:git-config[1].\n> +\n> \n> --signed::\n> \tGPG-sign the push request to update refs on the receiving\n> diff --git a/builtin/push.c b/builtin/push.c\n> index c25108f..6831c2d 100644\n> --- a/builtin/push.c\n> +++ b/builtin/push.c\n> @@ -482,6 +482,7 @@ static int option_parse_recurse_submodules(const struct option *opt,\n> \n> static int git_push_config(const char *k, const char *v, void *cb)\n> {\n> +\tint *flags = cb;\n> \tint status;\n> \n> \tstatus = git_gpg_config(k, v, NULL);\n> @@ -511,6 +512,14 @@ static int git_push_config(const char *k, const char *v, void *cb)\n> \t\treturn 0;\n> \t}\n> \n> +\tif (!strcmp(k, \"push.followtags\")) {\n> +\t\tif (git_config_bool(k, v))\n> +\t\t\t*flags |= TRANSPORT_PUSH_FOLLOW_TAGS;\n> +\t\telse\n> +\t\t\t*flags &= ~TRANSPORT_PUSH_FOLLOW_TAGS;\n> +\t\treturn 0;\n> +\t}\n> +\n> \treturn git_default_config(k, v, NULL);\n> }\n> \n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index c21190d..cffb2b8 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -2188,6 +2188,7 @@ _git_config ()\n> \t\tpull.octopus\n> \t\tpull.twohead\n> \t\tpush.default\n> +\t\tpush.followTags\n> \t\trebase.autosquash\n> \t\trebase.stat\n> \t\treceive.autogc\n"},{"id":"257680","messageId":"20150314173424.GB32599@peff.net","threadId":"38571","inReplyTo":"xmqqh9toxgdd.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 3/3] push: allow --follow-tags to be set by config push.followTags","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-14T17:34:24Z","receivedAt":"2015-03-14T17:34:24Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 13, 2015 at 11:06:22PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > From: Dave Olszewski \n> >\n> > Signed-off-by: Dave Olszewski \n> > ---\n> > Again, this is just a preview. Dave should send the final when he thinks\n> > it is good.\n> \n> Dave?\n> \n> I do not see anything wrong with this version that builds on top of\n> the previous 2 clean-up. Personally I find that these clean-up\n> changes more valuable than I care about this particular feature, and\n> it is unfortunate that waiting an Ack or reroll of this one kept\n> them stalled.\n> \n> I am tempted to throw \"Helped-by: Peff\" into the log message and\n> merge the result to 'next', unless I hear otherwise in a few days.\n\nFWIW, as the author of the leadup patches, that would be fine with me. I\nthink the end patch is in fine shape.\n\n-Peff\n"},{"id":"257682","messageId":"alpine.DEB.2.11.1503141049080.16979@narbuckle.genericorp.net","threadId":"38571","inReplyTo":"xmqqh9toxgdd.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 3/3] push: allow --follow-tags to be set by config push.followTags","fromName":"Dave Olszewski","fromEmail":"cxreg@pobox.com","sentAt":"2015-03-14T17:50:20Z","receivedAt":"2015-03-14T17:50:20Z","isPatch":true,"sender":{"key":"cxreg@pobox.com","avatar":"https://avatars.githubusercontent.com/u/55474?v=4"},"body":"On Fri, 13 Mar 2015, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > From: Dave Olszewski \n> >\n> > Signed-off-by: Dave Olszewski \n> > ---\n> > Again, this is just a preview. Dave should send the final when he thinks\n> > it is good.\n> \n> Dave?\n> \n> I do not see anything wrong with this version that builds on top of\n> the previous 2 clean-up. Personally I find that these clean-up\n> changes more valuable than I care about this particular feature, and\n> it is unfortunate that waiting an Ack or reroll of this one kept\n> them stalled.\n> \n> I am tempted to throw \"Helped-by: Peff\" into the log message and\n> merge the result to 'next', unless I hear otherwise in a few days.\n\nSorry, work has kept me very busy lately, I haven't had time to re-visit\nthis. Jeff's version looks great to me, please go ahead with it.\nThanks everyone.\n\n Dave\n"},{"id":"257689","messageId":"xmqqk2yjw7u1.fsf@gitster.dls.corp.google.com","threadId":"38571","inReplyTo":"alpine.DEB.2.11.1503141049080.16979@narbuckle.genericorp.net","subject":"Re: [PATCH 3/3] push: allow --follow-tags to be set by config push.followTags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-14T22:08:22Z","receivedAt":"2015-03-14T22:08:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dave Olszewski writes:\n\n> On Fri, 13 Mar 2015, Junio C Hamano wrote:\n>\n>> Jeff King writes:\n>> \n>> > From: Dave Olszewski \n>> >\n>> > Signed-off-by: Dave Olszewski \n>> > ---\n>> > Again, this is just a preview. Dave should send the final when he thinks\n>> > it is good.\n>> \n>> Dave?\n>> \n>> I do not see anything wrong with this version that builds on top of\n>> the previous 2 clean-up. Personally I find that these clean-up\n>> changes more valuable than I care about this particular feature, and\n>> it is unfortunate that waiting an Ack or reroll of this one kept\n>> them stalled.\n>> \n>> I am tempted to throw \"Helped-by: Peff\" into the log message and\n>> merge the result to 'next', unless I hear otherwise in a few days.\n>\n> Sorry, work has kept me very busy lately, I haven't had time to re-visit\n> this. Jeff's version looks great to me, please go ahead with it.\n> Thanks everyone.\n>\n> Dave\n\nThanks.\n"}]}