Re: 'simple' push check that branch name matches does not work if push.default is unset (and hence implicitly simple)
Jeff King p...@peff.net writes: There is some other magic with simple, too, around triangular workflows. Describing it in detail would probably be too verbose in this message, but we do refer to the description of push.default, which is probably enough. Technically this new bit you are adding here is covered there, too. But since we can improve the description by adding such a small amount of text in this case, it seems like a reasonable tradeoff. I suppose we could also customize the message based on the triangular and non-triangular cases. I dunno. Yeah, I vaguely recall suggesting to polish advice message further to help users along a similar line, but probably that fell in the cracks. Thanks for a quick fix. -- 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: 'simple' push check that branch name matches does not work if push.default is unset (and hence implicitly simple)
On Wed, 2014-11-26 at 22:43 -0500, Jeff King wrote: On Wed, Nov 26, 2014 at 02:29:28PM -0800, Adam Williamson wrote: Hi, folks. Ran into an unfortunate issue with git which helped me mess up a Fedora package repo today :/ The problem can be reproduced thus: 1. Create an empty repo, clone it 2. Push its master branch with something in it (just to get started) 3. git branch --track moo origin/master 4. git checkout moo 5. echo moo moo git commit -a -m create moo 6. git push ** BUG HAPPENS - CHANGES ARE PUSHED TO origin/master ** 7. git config --local push.default simple 8. echo moo2 moo git commit -a -m update moo 9. git push ** PUSH IS CORRECTLY REJECTED ** In both those cases, the push behaviour is supposed to be 'simple' - at step 6 it's *implicitly* set to 'simple' (according to the documentation), while at step 9 it's *explicitly* set to 'simple'. At step 6, a warning is printed to the console: Ugh. Yeah, this never worked properly, even in the original v2.0.0 release. Worse, our tests did not notice it at all. Patch is below. I've got a pile of Fedora stuff to do today so I don't know if I'll get time to dig any further into the history of this and see if there are any hidden wrinkles, but on the face of it, Jeff's patch looks good to me. I'll try and find a moment to test it at least, but the approach makes sense and is what I would've gone for too. It might also be worth improving the warn_unspecified_push_default_msg[] text to mention the name matching behaviour? At present it doesn't clearly explain this (you could argue it's *sort of* implied, but I doubt many people will read it that way - I didn't), you have to follow the chain into 'git help config' to find the description. Something like: Since Git 2.0, Git defaults to the more conservative 'simple'\n behavior, which only pushes the current branch to the corresponding\n remote branch that 'git pull' uses to update the current branch, \n if the names of those two branches match.\n -- Adam Williamson Fedora QA Community Monkey IRC: adamw | Twitter: AdamW_Fedora | XMPP: adamw AT happyassassin . net http://www.happyassassin.net -- 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: 'simple' push check that branch name matches does not work if push.default is unset (and hence implicitly simple)
On Thu, Nov 27, 2014 at 09:12:27AM -0800, Adam Williamson wrote: It might also be worth improving the warn_unspecified_push_default_msg[] text to mention the name matching behaviour? At present it doesn't clearly explain this (you could argue it's *sort of* implied, but I doubt many people will read it that way - I didn't), you have to follow the chain into 'git help config' to find the description. Something like: Since Git 2.0, Git defaults to the more conservative 'simple'\n behavior, which only pushes the current branch to the corresponding\n remote branch that 'git pull' uses to update the current branch, \n if the names of those two branches match.\n Yeah, I agree that is more clear. There is some other magic with simple, too, around triangular workflows. Describing it in detail would probably be too verbose in this message, but we do refer to the description of push.default, which is probably enough. Technically this new bit you are adding here is covered there, too. But since we can improve the description by adding such a small amount of text in this case, it seems like a reasonable tradeoff. I suppose we could also customize the message based on the triangular and non-triangular cases. I dunno. -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
'simple' push check that branch name matches does not work if push.default is unset (and hence implicitly simple)
Hi, folks. Ran into an unfortunate issue with git which helped me mess up a Fedora package repo today :/ The problem can be reproduced thus: 1. Create an empty repo, clone it 2. Push its master branch with something in it (just to get started) 3. git branch --track moo origin/master 4. git checkout moo 5. echo moo moo git commit -a -m create moo 6. git push ** BUG HAPPENS - CHANGES ARE PUSHED TO origin/master ** 7. git config --local push.default simple 8. echo moo2 moo git commit -a -m update moo 9. git push ** PUSH IS CORRECTLY REJECTED ** In both those cases, the push behaviour is supposed to be 'simple' - at step 6 it's *implicitly* set to 'simple' (according to the documentation), while at step 9 it's *explicitly* set to 'simple'. At step 6, a warning is printed to the console: = warning: push.default is unset; its implicit value has changed in Git 2.0 from 'matching' to 'simple'. To squelch this message and maintain the traditional behavior, use: git config --global push.default matching To squelch this message and adopt the new behavior now, use: git config --global push.default simple When push.default is set to 'matching', git will push local branches to the remote branches that already exist with the same name. Since Git 2.0, Git defaults to the more conservative 'simple' behavior, which only pushes the current branch to the corresponding remote branch that 'git pull' uses to update the current branch. See 'git help config' and search for 'push.default' for further information. (the 'simple' mode was introduced in Git 1.7.11. Use the similar mode 'current' instead of 'simple' if you sometimes use older versions of Git) == If you follow the trail there and look at 'git help config', you find this: · simple - in centralized workflow, work like upstream with an added safety to refuse to push if the upstream branch’s name is different from the local one. === However, at step 6, the changes from branch 'moo' are pushed to 'master' - even though that text clearly says they shouldn't be, as the names don't match. After step 7 - *explicitly* setting push.default to simple, rather than relying on it being set to simple *implicitly* - another git push is correctly rejected, with this message: fatal: The upstream branch of your current branch does not match the name of your current branch. To push to the upstream branch on the remote, use git push origin HEAD:master To push to the branch of the same name on the remote, use git push origin moo = I believe the 'implicit' case was broken by the commit push: change `simple` to accommodate triangular workflows: https://github.com/git/git/commit/ed2b18292bfeedc98c9e2b6bd8a35d8001dab2fc It changes the condition for running the 'does the branch name match' test from if (simple) to if (push_default == PUSH_DEFAULT_SIMPLE). AFAICS, in the 'implicit' case, push_default == PUSH_DEFAULT_UNSPECIFIED, not PUSH_DEFAULT_SIMPLE, so the 'does the branch name match' check is not run, even though the behaviour is supposed (according to the documentation) to be the same as if the default were explicitly set to 'simple'. Thanks to this, when I accidentally did 'git push' on a branch of the Fedora kernel package git repo which only exists in my downstream checkout, all my changes got pushed to the upstream master branch :( So it's a bit dangerous. -- Adam Williamson Fedora QA Community Monkey IRC: adamw | Twitter: AdamW_Fedora | XMPP: adamw AT happyassassin . net http://www.happyassassin.net -- 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: 'simple' push check that branch name matches does not work if push.default is unset (and hence implicitly simple)
On Wed, 2014-11-26 at 14:29 -0800, Adam Williamson wrote: Hi, folks. Ran into an unfortunate issue with git which helped me mess up a Fedora package repo today :/ The problem can be reproduced thus: Whoops, I missed step 0: 0. Ensure push.default is not configured globally 1. Create an empty repo, clone it 2. Push its master branch with something in it (just to get started) 3. git branch --track moo origin/master 4. git checkout moo 5. echo moo moo git commit -a -m create moo 6. git push ** BUG HAPPENS - CHANGES ARE PUSHED TO origin/master ** 7. git config --local push.default simple 8. echo moo2 moo git commit -a -m update moo 9. git push ** PUSH IS CORRECTLY REJECTED ** -- Adam Williamson Fedora QA Community Monkey IRC: adamw | Twitter: AdamW_Fedora | XMPP: adamw AT happyassassin . net http://www.happyassassin.net -- 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: 'simple' push check that branch name matches does not work if push.default is unset (and hence implicitly simple)
On Wed, Nov 26, 2014 at 02:29:28PM -0800, Adam Williamson wrote:
Hi, folks. Ran into an unfortunate issue with git which helped me mess
up a Fedora package repo today :/
The problem can be reproduced thus:
1. Create an empty repo, clone it
2. Push its master branch with something in it (just to get started)
3. git branch --track moo origin/master
4. git checkout moo
5. echo moo moo git commit -a -m create moo
6. git push
** BUG HAPPENS - CHANGES ARE PUSHED TO origin/master **
7. git config --local push.default simple
8. echo moo2 moo git commit -a -m update moo
9. git push
** PUSH IS CORRECTLY REJECTED **
In both those cases, the push behaviour is supposed to be 'simple' - at
step 6 it's *implicitly* set to 'simple' (according to the
documentation), while at step 9 it's *explicitly* set to 'simple'. At
step 6, a warning is printed to the console:
Ugh. Yeah, this never worked properly, even in the original v2.0.0
release. Worse, our tests did not notice it at all. Patch is below.
-- 8 --
Subject: push: truly use simple as default, not upstream
The plan for the push.default transition had all along been
to use the simple method rather than upstream as a
default if the user did not specify their own push.default
value. Commit 11037ee (push: switch default from matching
to simple, 2013-01-04) tried to implement that by moving
PUSH_DEFAULT_UNSPECIFIED in our switch statement to
fall-through to the PUSH_DEFAULT_SIMPLE case.
When the commit that became 11037ee was originally written,
that would have been enough. We would fall through to
calling setup_push_upstream() with the simple parameter
set to 1. However, it was delayed for a while until we were
ready to make the transition in Git 2.0.
And in the meantime, commit ed2b182 (push: change `simple`
to accommodate triangular workflows, 2013-06-19) threw a
monkey wrench into the works. That commit drops the simple
parameter to setup_push_upstream, and instead checks whether
the global push_default is PUSH_DEFAULT_SIMPLE. This is
right when the user has explicitly configured push.default
to simple, but wrong when we are a fall-through for the
unspecified case.
We never noticed because our push.default tests do not cover
the case of the variable being totally unset; they only
check the simple behavior itself.
Signed-off-by: Jeff King p...@peff.net
---
ed2b182 comes from Ram, but the suggestion for this bit of code actually
comes from Junio in:
http://thread.gmane.org/gmane.comp.version-control.git/228383/focus=228436
I am not sure I understand the reason for dropping the simple
parameter in that commit in the first place. If we are in triangular
mode, then we would not get to setup_push_upstream from simple (or the
default) in the first place (we would use current instead). The only
time triangular matters to setup_push_upstream is when push.default
really has been set to upstream, but in that case, simple would
always be 0 (and likewise, the equality check that replaces it would
also be false).
So I have a vague concern that I'm missing something. Maybe one of you
who worked on it can recall more.
builtin/push.c | 8
t/t5528-push-default.sh | 32 ++--
2 files changed, 34 insertions(+), 6 deletions(-)
diff --git a/builtin/push.c b/builtin/push.c
index a076b19..7aedf6f 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -161,7 +161,7 @@ static const char message_detached_head_die[] =
git push %s HEAD:name-of-remote-branch\n);
static void setup_push_upstream(struct remote *remote, struct branch *branch,
- int triangular)
+ int triangular, int simple)
{
struct strbuf refspec = STRBUF_INIT;
@@ -184,7 +184,7 @@ static void setup_push_upstream(struct remote *remote,
struct branch *branch,
to update which remote branch.),
remote-name, branch-name);
- if (push_default == PUSH_DEFAULT_SIMPLE) {
+ if (simple) {
/* Additional safety */
if (strcmp(branch-refname, branch-merge[0]-src))
die_push_simple(branch, remote);
@@ -257,11 +257,11 @@ static void setup_default_push_refspecs(struct remote
*remote)
if (triangular)
setup_push_current(remote, branch);
else
- setup_push_upstream(remote, branch, triangular);
+ setup_push_upstream(remote, branch, triangular, 1);
break;
case PUSH_DEFAULT_UPSTREAM:
- setup_push_upstream(remote, branch, triangular);
+ setup_push_upstream(remote, branch, triangular, 0);
break;
case PUSH_DEFAULT_CURRENT:
diff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh
index 6a5ac3a..cc74519 100755
--- a/t/t5528-push-default.sh
+++ b/t/t5528-push-default.sh
@@ -26,7 +26,7 @@
Re: 'simple' push check that branch name matches does not work if push.default is unset (and hence implicitly simple)
On November 26, 2014 7:43:06 PM PST, Jeff King p...@peff.net wrote:
On Wed, Nov 26, 2014 at 02:29:28PM -0800, Adam Williamson wrote:
Hi, folks. Ran into an unfortunate issue with git which helped me
mess
up a Fedora package repo today :/
The problem can be reproduced thus:
1. Create an empty repo, clone it
2. Push its master branch with something in it (just to get started)
3. git branch --track moo origin/master
4. git checkout moo
5. echo moo moo git commit -a -m create moo
6. git push
** BUG HAPPENS - CHANGES ARE PUSHED TO origin/master **
7. git config --local push.default simple
8. echo moo2 moo git commit -a -m update moo
9. git push
** PUSH IS CORRECTLY REJECTED **
In both those cases, the push behaviour is supposed to be 'simple' -
at
step 6 it's *implicitly* set to 'simple' (according to the
documentation), while at step 9 it's *explicitly* set to 'simple'. At
step 6, a warning is printed to the console:
Ugh. Yeah, this never worked properly, even in the original v2.0.0
release. Worse, our tests did not notice it at all. Patch is below.
-- 8 --
Subject: push: truly use simple as default, not upstream
The plan for the push.default transition had all along been
to use the simple method rather than upstream as a
default if the user did not specify their own push.default
value. Commit 11037ee (push: switch default from matching
to simple, 2013-01-04) tried to implement that by moving
PUSH_DEFAULT_UNSPECIFIED in our switch statement to
fall-through to the PUSH_DEFAULT_SIMPLE case.
When the commit that became 11037ee was originally written,
that would have been enough. We would fall through to
calling setup_push_upstream() with the simple parameter
set to 1. However, it was delayed for a while until we were
ready to make the transition in Git 2.0.
And in the meantime, commit ed2b182 (push: change `simple`
to accommodate triangular workflows, 2013-06-19) threw a
monkey wrench into the works. That commit drops the simple
parameter to setup_push_upstream, and instead checks whether
the global push_default is PUSH_DEFAULT_SIMPLE. This is
right when the user has explicitly configured push.default
to simple, but wrong when we are a fall-through for the
unspecified case.
We never noticed because our push.default tests do not cover
the case of the variable being totally unset; they only
check the simple behavior itself.
Signed-off-by: Jeff King p...@peff.net
---
ed2b182 comes from Ram, but the suggestion for this bit of code
actually
comes from Junio in:
http://thread.gmane.org/gmane.comp.version-control.git/228383/focus=228436
I am not sure I understand the reason for dropping the simple
parameter in that commit in the first place. If we are in triangular
mode, then we would not get to setup_push_upstream from simple (or
the
default) in the first place (we would use current instead). The only
time triangular matters to setup_push_upstream is when push.default
really has been set to upstream, but in that case, simple would
always be 0 (and likewise, the equality check that replaces it would
also be false).
So I have a vague concern that I'm missing something. Maybe one of you
who worked on it can recall more.
builtin/push.c | 8
t/t5528-push-default.sh | 32 ++--
2 files changed, 34 insertions(+), 6 deletions(-)
diff --git a/builtin/push.c b/builtin/push.c
index a076b19..7aedf6f 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -161,7 +161,7 @@ static const char message_detached_head_die[] =
git push %s HEAD:name-of-remote-branch\n);
static void setup_push_upstream(struct remote *remote, struct branch
*branch,
- int triangular)
+ int triangular, int simple)
{
struct strbuf refspec = STRBUF_INIT;
@@ -184,7 +184,7 @@ static void setup_push_upstream(struct remote
*remote, struct branch *branch,
to update which remote branch.),
remote-name, branch-name);
- if (push_default == PUSH_DEFAULT_SIMPLE) {
+ if (simple) {
/* Additional safety */
if (strcmp(branch-refname, branch-merge[0]-src))
die_push_simple(branch, remote);
@@ -257,11 +257,11 @@ static void setup_default_push_refspecs(struct
remote *remote)
if (triangular)
setup_push_current(remote, branch);
else
- setup_push_upstream(remote, branch, triangular);
+ setup_push_upstream(remote, branch, triangular, 1);
break;
case PUSH_DEFAULT_UPSTREAM:
- setup_push_upstream(remote, branch, triangular);
+ setup_push_upstream(remote, branch, triangular, 0);
break;
case PUSH_DEFAULT_CURRENT:
diff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh
index 6a5ac3a..cc74519 100755
--- a/t/t5528-push-default.sh
+++
