Re: [PATCH 2/2] builtin/push.c: make push_default a static variable

2015-02-18 Thread Jeff King
On Tue, Feb 17, 2015 at 02:16:30PM -0800, Junio C Hamano wrote:

 Do you want to resurrect that @{publish} stuff?  I think it had
 sensible semantics, and I do not think we mind keeping the
 push_default configuration to be read from the default_config
 codepath.

I'll take a look at it and see if it's in good enough shape to apply
as-is, or with minor tweaking. But regardless, let's...

 If we decide to go that route, then the series would become
 something like this:
 
 $gmane/263871 [1/4] git_push_config: drop cargo-culted wt_status pointer
 $gmane/263878 [2/4] cmd_push: set atomic bit directly
 $gmane/263879 [3/4] cmd_push: pass flags pointer to config callback
 $gmane/263880 [4/4] push: allow --follow-tags to be set by config 
 push.followTags
 
 omitting the original 2/2 patch we are discussing.  I am inclined to
 replace what I queued with the above four.

...do this. Even if we don't apply other patches to make use of
push_default immediately, it's a plausible area for us to touch later,
and the cleanup from the dropped patch was not so important.

 + if (!strcmp(k, push.followtags)) {
 + if (git_config_bool(k, v))
 + *flags |= TRANSPORT_PUSH_FOLLOW_TAGS;
 + else
 + *flags = ~TRANSPORT_PUSH_FOLLOW_TAGS;
 + return 0;
 + }

Did you have an opinion on sticking this behind a helper function?

It feels like a lot of repeating of the same variables and flags, but I
worried that munge_bit ends up being even more confusing.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] builtin/push.c: make push_default a static variable

2015-02-18 Thread Jeff King
On Wed, Feb 18, 2015 at 11:08:34AM -0800, Junio C Hamano wrote:

 Jeff King p...@peff.net writes:
 
  +  if (!strcmp(k, push.followtags)) {
  +  if (git_config_bool(k, v))
  +  *flags |= TRANSPORT_PUSH_FOLLOW_TAGS;
  +  else
  +  *flags = ~TRANSPORT_PUSH_FOLLOW_TAGS;
  +  return 0;
  +  }
 
  Did you have an opinion on sticking this behind a helper function?
 
 Not very strongly either way.  Seeing the above does not bother me
 too much, but I do not know how I would feel when I start seeing
 
   val = git_config_book(k, v);
   flip_bool(val, flags, TRANSPORT_PUSH_FOLLOW_TAGS);
 
 often.  Not having to make sure that the bit constant whose name
 tends to get long is not misspelled is certainly a plus.

I think it would be even nicer as:

  git_config_bits(k, v, flags, TRANSPORT_PUSH_FOLLOW_TAGS);

There is a similar spot in the tar.*.remote config. And that could of
course build on a flip_bool or similar, which itself has many other
uses. But after taking a quick peek, I noticed that one call around
diff.c:3600 would look like:

  flip_bool(!negate, opt-filter, bit);

IOW, it is the same pattern of conditional, but it flips the AND and OR,
because its flag is flipped. Reading that line makes me head hurt,
because we've really introduced an extra double-negative into the flow.

That negate flag is local to the loop we are in, and we could flip it
for clarity. But it makes me second-guess the technique.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] builtin/push.c: make push_default a static variable

2015-02-18 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 On Wed, Feb 18, 2015 at 11:08:34AM -0800, Junio C Hamano wrote:
 ...
 Not very strongly either way.  Seeing the above does not bother me
 too much, but I do not know how I would feel when I start seeing
 
  val = git_config_book(k, v);
  flip_bool(val, flags, TRANSPORT_PUSH_FOLLOW_TAGS);
 
 often.  Not having to make sure that the bit constant whose name
 tends to get long is not misspelled is certainly a plus.

 I think it would be even nicer as:

   git_config_bits(k, v, flags, TRANSPORT_PUSH_FOLLOW_TAGS);

Maybe.  I do not feel very strongly either way.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] builtin/push.c: make push_default a static variable

2015-02-18 Thread Jeff King
On Wed, Feb 18, 2015 at 11:50:20AM -0800, Junio C Hamano wrote:

 Jeff King p...@peff.net writes:
 
  On Wed, Feb 18, 2015 at 11:08:34AM -0800, Junio C Hamano wrote:
  ...
  Not very strongly either way.  Seeing the above does not bother me
  too much, but I do not know how I would feel when I start seeing
  
 val = git_config_book(k, v);
 flip_bool(val, flags, TRANSPORT_PUSH_FOLLOW_TAGS);
  
  often.  Not having to make sure that the bit constant whose name
  tends to get long is not misspelled is certainly a plus.
 
  I think it would be even nicer as:
 
git_config_bits(k, v, flags, TRANSPORT_PUSH_FOLLOW_TAGS);
 
 Maybe.  I do not feel very strongly either way.

So I started to prepare a patch for this, because I wanted to see how it
would look. It's below in case you are curious, but note that it doesn't
compile, and that is does something a bit dangerous. So I think I am
giving up on this line of thought. Read on if you're curious.

The sticking point is the type of the bit-field. If it is:

void flip_bits(int set_or_clear, int *field, int bits);

then we cannot pass a pointer to unsigned field. We can pass unsigned
bits, but note that we might lose the 32nd (or 64th) bit.

We can do this instead:

void flip_bits(int set_or_clear, void *field, unsigned bits);

But of course we will end up dereferencing void *field as unsigned *.
I suspect if field is originally an int it works fine on most
platforms, but isn't legal according to the standard. Much worse,
though: if your original field is a different size, it's even less
likely to work. Passing a char * may set bits in random adjacent
memory (depending on your endianness), and an unsigned long * may set
bits in the wrong part of the variable. And because of the void *, we
get no compiler warnings. :)

Add on top that we may want to use this for C bit-fields, whose address
cannot legally be taken (and this is why it does not compile).

So besides adding type-specific bit-flippers (yuck), I think the only
way to do it universally would be with a macro.  Which I think tips it
over the too gross, just write it out line.

---
diff --git a/archive-tar.c b/archive-tar.c
index 0d1e6bd..3c794e2 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -352,10 +352,7 @@ static int tar_filter_config(const char *var, const char 
*value, void *data)
return 0;
}
if (!strcmp(type, remote)) {
-   if (git_config_bool(var, value))
-   ar-flags |= ARCHIVER_REMOTE;
-   else
-   ar-flags = ~ARCHIVER_REMOTE;
+   git_config_bits(var, value, ar-flags, ARCHIVER_REMOTE);
return 0;
}
 
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index d816587..073445e 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2216,10 +2216,8 @@ static int git_pack_config(const char *k, const char *v, 
void *cb)
return 0;
}
if (!strcmp(k, pack.writebitmaphashcache)) {
-   if (git_config_bool(k, v))
-   write_bitmap_options |= BITMAP_OPT_HASH_CACHE;
-   else
-   write_bitmap_options = ~BITMAP_OPT_HASH_CACHE;
+   git_config_bits(k, v, write_bitmap_options,
+   BITMAP_OPT_HASH_CACHE);
}
if (!strcmp(k, pack.usebitmaps)) {
use_bitmap_index = git_config_bool(k, v);
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 5878986..9d4eb24 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -53,10 +53,7 @@ static int mark_ce_flags(const char *path, int flag, int 
mark)
int namelen = strlen(path);
int pos = cache_name_pos(path, namelen);
if (0 = pos) {
-   if (mark)
-   active_cache[pos]-ce_flags |= flag;
-   else
-   active_cache[pos]-ce_flags = ~flag;
+   flip_bits(mark, active_cache[pos]-ce_flags, flag);
active_cache[pos]-ce_flags |= CE_UPDATE_IN_BASE;
cache_tree_invalidate_path(the_index, path);
active_cache_changed |= CE_ENTRY_CHANGED;
diff --git a/cache.h b/cache.h
index f704af5..957f150 100644
--- a/cache.h
+++ b/cache.h
@@ -1316,6 +1316,18 @@ extern int sha1_object_info_extended(const unsigned char 
*, struct object_info *
 /* Dumb servers support */
 extern int update_server_info(int);
 
+/*
+ * Either set or clear bits from field, based on
+ * whether or not set_or_clear is set.
+ */
+static inline void flip_bits(int set_or_clear, void *field, unsigned bits)
+{
+   if (set_or_clear)
+   *(unsigned *)field |= bits;
+   else
+   *(unsigned *)field = ~bits;
+}
+
 /* git_config_parse_key() returns these negated: */
 #define CONFIG_INVALID_KEY 1
 #define CONFIG_NO_SECTION_OR_NAME 2
@@ -1385,6 +1397,12 @@ struct config_include_data {
 #define CONFIG_INCLUDE_INIT { 0 

Re: [PATCH 2/2] builtin/push.c: make push_default a static variable

2015-02-17 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 If we wanted to implement @{push} (or @{publish}) to mean the
 tracking ref of the remote ref you would push to if you ran git-push,
 then this is a step in the wrong direction.

Is that because push_default variable needs to be looked at from
sha1_name.c when resolving @{push}, optionally prefixed with the
name of the branch?  I wonder if that codepath should know the gory
details of which ref at the remote the branch is pushed to and which
remote-tracking ref we use in the local repository to mirror that
remote ref in the first place?

What do we do for the @{upstream} side of the things---it calls
branch_get() and when the branch structure is returned, the details
have been computed for us so get_upstream_branch() only needs to use
the information already computed.  The interesting parts of the
computation all happen inside remote.c, it seems.

So we probably would do something similar to @{push} side, which
would mean that push_default variable and the logic needs to be
visible to remote.c if we want to have the helper that is similar to
set_merge() that is used from branch_get() to support @{upstream}.

Hmmm, I have a feeling that with default configuration, where does
'git push' send this branch to? logic should be contained within
the source file whose name has push in it and exposed as a helper
function, instead of exposing just one of the lowest level knob
push_default to outside callers and have them figure things out.

Viewed from that angle, it might be the case that remote.c knows too
much about what happens during fetch and pull, but I dunno.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] builtin/push.c: make push_default a static variable

2015-02-17 Thread Jeff King
On Tue, Feb 17, 2015 at 09:45:07AM -0800, Junio C Hamano wrote:

 Jeff King p...@peff.net writes:
 
  If we wanted to implement @{push} (or @{publish}) to mean the
  tracking ref of the remote ref you would push to if you ran git-push,
  then this is a step in the wrong direction.
 
 Is that because push_default variable needs to be looked at from
 sha1_name.c when resolving @{push}, optionally prefixed with the
 name of the branch?

Yes, exactly.

 I wonder if that codepath should know the gory details of which ref at
 the remote the branch is pushed to and which remote-tracking ref we
 use in the local repository to mirror that remote ref in the first
 place?

I think that was one of the ugly bits from the series; that we had to
reimplement where would we push and what would it be called if we
pushed and then fetched? The former cares about push_default, and the
latter has to apply push and then fetch refspecs.

If you want to peek at it again, it's at:

  https://github.com/peff/git/commit/8859afb1af63cb3cb0bc4cc8c1719c2011f406c9

(but note that it should not be called @{publish}, as per earlier
discussions).

 What do we do for the @{upstream} side of the things---it calls
 branch_get() and when the branch structure is returned, the details
 have been computed for us so get_upstream_branch() only needs to use
 the information already computed.  The interesting parts of the
 computation all happen inside remote.c, it seems.
 
 So we probably would do something similar to @{push} side, which
 would mean that push_default variable and the logic needs to be
 visible to remote.c if we want to have the helper that is similar to
 set_merge() that is used from branch_get() to support @{upstream}.

Sure, we could go that way. But I don't think it changes the issue for
_this_ patch series, which is that the variable needs visibility outside
of builtin/push.c (and we need to load the config for programs besides
git-push).

 Hmmm, I have a feeling that with default configuration, where does
 'git push' send this branch to? logic should be contained within
 the source file whose name has push in it and exposed as a helper
 function, instead of exposing just one of the lowest level knob
 push_default to outside callers and have them figure things out.
 
 Viewed from that angle, it might be the case that remote.c knows too
 much about what happens during fetch and pull, but I dunno.

Yeah, it would be nice if there were a convenient lib-ified set of
functions for getting this information, and fetch and push commands
were built on top of it. I don't know how painful that would be, though.
The existing code has grown somewhat organically.

But even with that change, the lib-ified code needs to hook into
git_default_config (or do its own config lookup) so that we get the
proper value no matter who the caller is.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] builtin/push.c: make push_default a static variable

2015-02-17 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 So we probably would do something similar to @{push} side, which
 would mean that push_default variable and the logic needs to be
 visible to remote.c if we want to have the helper that is similar to
 set_merge() that is used from branch_get() to support @{upstream}.

 Sure, we could go that way. But I don't think it changes the issue for
 _this_ patch series, which is that the variable needs visibility outside
 of builtin/push.c (and we need to load the config for programs besides
 git-push).

I do not disagree.  push_default and other things that affect the
computation needs to be visible to the code that implements the
logic.

Do you want to resurrect that @{publish} stuff?  I think it had
sensible semantics, and I do not think we mind keeping the
push_default configuration to be read from the default_config
codepath.

If we decide to go that route, then the series would become
something like this:

$gmane/263871 [1/4] git_push_config: drop cargo-culted wt_status pointer
$gmane/263878 [2/4] cmd_push: set atomic bit directly
$gmane/263879 [3/4] cmd_push: pass flags pointer to config callback
$gmane/263880 [4/4] push: allow --follow-tags to be set by config 
push.followTags

omitting the original 2/2 patch we are discussing.  I am inclined to
replace what I queued with the above four.

The last one needs a bit of tweaking and should look like the
attached.  Again, as you wrote in $gmane/263880, this is just a
preview. Dave should send the final when he thinks it is good,
possibly with some tests.

-- 8 --
From: Dave Olszewski cx...@pobox.com
Date: Mon, 16 Feb 2015 01:16:19 -0500
Subject: [PATCH] [NEEDSACK] push: allow --follow-tags to be set by config 
push.followTags

Signed-off-by: Dave Olszewski cx...@pobox.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
 Documentation/config.txt   |  6 ++
 Documentation/git-push.txt |  5 -
 builtin/push.c | 10 ++
 contrib/completion/git-completion.bash |  1 +
 4 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index ae6791d..e01d21c 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2079,6 +2079,12 @@ new default).
 
 --
 
+push.followTags::
+   If set to true enable '--follow-tags' option by default.  You
+   may override this configuration at time of push by specifying
+   '--no-follow-tags'.
+
+
 rebase.stat::
Whether to show a diffstat of what changed upstream since the last
rebase. False by default.
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index ea97576..caa187b 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -128,7 +128,10 @@ already exists on the remote side.
Push all the refs that would be pushed without this option,
and also push annotated tags in `refs/tags` that are missing
from the remote but are pointing at commit-ish that are
-   reachable from the refs being pushed.
+   reachable from the refs being pushed.  This can also be specified
+   with configuration variable 'push.followTags'.  For more
+   information, see 'push.followTags' in linkgit:git-config[1].
+
 
 --signed::
GPG-sign the push request to update refs on the receiving
diff --git a/builtin/push.c b/builtin/push.c
index bba22b8..57c138b 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -473,11 +473,21 @@ static int option_parse_recurse_submodules(const struct 
option *opt,
 
 static int git_push_config(const char *k, const char *v, void *cb)
 {
+   int *flags = cb;
int status;
 
status = git_gpg_config(k, v, NULL);
if (status)
return status;
+
+   if (!strcmp(k, push.followtags)) {
+   if (git_config_bool(k, v))
+   *flags |= TRANSPORT_PUSH_FOLLOW_TAGS;
+   else
+   *flags = ~TRANSPORT_PUSH_FOLLOW_TAGS;
+   return 0;
+   }
+
return git_default_config(k, v, NULL);
 }
 
diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index c21190d..cffb2b8 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2188,6 +2188,7 @@ _git_config ()
pull.octopus
pull.twohead
push.default
+   push.followTags
rebase.autosquash
rebase.stat
receive.autogc
-- 
2.3.0-283-g21bf3f5


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] builtin/push.c: make push_default a static variable

2015-02-17 Thread Jeff King
On Mon, Feb 16, 2015 at 12:47:54AM -0500, Jeff King wrote:

 When the push_default flag was originally added, it was
 made globally visible to all code. This might have been
 useful if other commands or library calls ended up depending
 on it, but as it turns out, only builtin/push.c cares.
 
 Let's make it a static variable in builtin/push.c.

 [...]
 
 ---
 We know this is safe because no other callers needed tweaked when the
 variable went out of scope. :) It would only be a bad idea if we
 were planning on having other code in the future depend on push_default
 (e.g., the code in remote.c to find the push destination). But it does
 not seem to have needed that in the intervening years, so it's probably
 fine to do this cleanup now.

I had a nagging feeling that there was some code which wanted to use
this elsewhere, and I did finally find it, when I merged this topic with
my other personal topics.

If we wanted to implement @{push} (or @{publish}) to mean the
tracking ref of the remote ref you would push to if you ran git-push,
then this is a step in the wrong direction.

The patches I posted last January (and which you carried as
jk/branch-at-publish for a while) do work, and I've used the feature
once or twice since then. From the discussion, it looks like they were
meant to be a building block for more triangular-flow work, but I don't
remember what else was needed. I'm tempted to resurrect them, but it's
not a high priority for me.

Anyway, food for thought on whether we want to do this cleanup or not,
then. We can always leave this here as part of git_default_config, and
still move Dave's new option into git_push_config.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] builtin/push.c: make push_default a static variable

2015-02-15 Thread Junio C Hamano
On Sun, Feb 15, 2015 at 9:47 PM, Jeff King p...@peff.net wrote:
 When the push_default flag was originally added, it was
 made globally visible to all code. This might have been
 useful if other commands or library calls ended up depending
 on it, but as it turns out, only builtin/push.c cares.
 ...
 Signed-off-by: Jeff King p...@peff.net
 ---
 We know this is safe because no other callers needed tweaked when the
 variable went out of scope. :) It would only be a bad idea if we
 were planning on having other code in the future depend on push_default
 (e.g., the code in remote.c to find the push destination). But it does
 not seem to have needed that in the intervening years, so it's probably
 fine to do this cleanup now.

Yay. Great minds think alike ;-)

It definitely smells wrong to touch environment.c and cache.h was my
first reaction to the follow-tags config patch, and I really think this shows
the right way forward.

Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/2] builtin/push.c: make push_default a static variable

2015-02-15 Thread Jeff King
When the push_default flag was originally added, it was
made globally visible to all code. This might have been
useful if other commands or library calls ended up depending
on it, but as it turns out, only builtin/push.c cares.

Let's make it a static variable in builtin/push.c. Since it
is no longer globally visible, it only needs to be set
inside that function. That means we can drop the
git_push_default_config function (which is called from
git_default_config for all commands) and just set it as part
of git_push_config.

That in turn makes it easier for people adding new config to
git-push to know which callback function to add to (since
there is only one now).

Signed-off-by: Jeff King p...@peff.net
---
We know this is safe because no other callers needed tweaked when the
variable went out of scope. :) It would only be a bad idea if we
were planning on having other code in the future depend on push_default
(e.g., the code in remote.c to find the push destination). But it does
not seem to have needed that in the intervening years, so it's probably
fine to do this cleanup now.

 builtin/push.c | 33 +
 cache.h| 10 --
 config.c   | 32 
 environment.c  |  1 -
 4 files changed, 33 insertions(+), 43 deletions(-)

diff --git a/builtin/push.c b/builtin/push.c
index aa9334c..ab99f4c 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -23,6 +23,15 @@ static int progress = -1;
 
 static struct push_cas_option cas;
 
+static enum push_default_type {
+   PUSH_DEFAULT_NOTHING = 0,
+   PUSH_DEFAULT_MATCHING,
+   PUSH_DEFAULT_SIMPLE,
+   PUSH_DEFAULT_UPSTREAM,
+   PUSH_DEFAULT_CURRENT,
+   PUSH_DEFAULT_UNSPECIFIED
+} push_default = PUSH_DEFAULT_UNSPECIFIED;
+
 static const char **refspec;
 static int refspec_nr;
 static int refspec_alloc;
@@ -478,6 +487,30 @@ static int git_push_config(const char *k, const char *v, 
void *cb)
status = git_gpg_config(k, v, NULL);
if (status)
return status;
+
+   if (!strcmp(k, push.default)) {
+   if (!v)
+   return config_error_nonbool(k);
+   else if (!strcmp(v, nothing))
+   push_default = PUSH_DEFAULT_NOTHING;
+   else if (!strcmp(v, matching))
+   push_default = PUSH_DEFAULT_MATCHING;
+   else if (!strcmp(v, simple))
+   push_default = PUSH_DEFAULT_SIMPLE;
+   else if (!strcmp(v, upstream))
+   push_default = PUSH_DEFAULT_UPSTREAM;
+   else if (!strcmp(v, tracking)) /* deprecated */
+   push_default = PUSH_DEFAULT_UPSTREAM;
+   else if (!strcmp(v, current))
+   push_default = PUSH_DEFAULT_CURRENT;
+   else {
+   error(Malformed value for %s: %s, k, v);
+   return error(Must be one of nothing, matching, simple, 

+upstream or current.);
+   }
+   return 0;
+   }
+
return git_default_config(k, v, NULL);
 }
 
diff --git a/cache.h b/cache.h
index f704af5..5394546 100644
--- a/cache.h
+++ b/cache.h
@@ -636,18 +636,8 @@ enum rebase_setup_type {
AUTOREBASE_ALWAYS
 };
 
-enum push_default_type {
-   PUSH_DEFAULT_NOTHING = 0,
-   PUSH_DEFAULT_MATCHING,
-   PUSH_DEFAULT_SIMPLE,
-   PUSH_DEFAULT_UPSTREAM,
-   PUSH_DEFAULT_CURRENT,
-   PUSH_DEFAULT_UNSPECIFIED
-};
-
 extern enum branch_track git_branch_track;
 extern enum rebase_setup_type autorebase;
-extern enum push_default_type push_default;
 
 enum object_creation_mode {
OBJECT_CREATION_USES_HARDLINKS = 0,
diff --git a/config.c b/config.c
index e5e64dc..5782442 100644
--- a/config.c
+++ b/config.c
@@ -952,35 +952,6 @@ static int git_default_branch_config(const char *var, 
const char *value)
return 0;
 }
 
-static int git_default_push_config(const char *var, const char *value)
-{
-   if (!strcmp(var, push.default)) {
-   if (!value)
-   return config_error_nonbool(var);
-   else if (!strcmp(value, nothing))
-   push_default = PUSH_DEFAULT_NOTHING;
-   else if (!strcmp(value, matching))
-   push_default = PUSH_DEFAULT_MATCHING;
-   else if (!strcmp(value, simple))
-   push_default = PUSH_DEFAULT_SIMPLE;
-   else if (!strcmp(value, upstream))
-   push_default = PUSH_DEFAULT_UPSTREAM;
-   else if (!strcmp(value, tracking)) /* deprecated */
-   push_default = PUSH_DEFAULT_UPSTREAM;
-   else if (!strcmp(value, current))
-   push_default = PUSH_DEFAULT_CURRENT;
-   else {
-   error(Malformed value for %s: %s, var, value);
-   return