git remote update -> rejected

git remote update -> rejected

[Mike Galbraith]

Greetings,

Does anyone know what could lead to this?  git doesn't have any trouble
(well, hasn't had any yet) with any trees other than x86 and
sched-devel. Nobody but little old me seems to be having this trouble...

git is pulled nearly every day, but checking out older versions doesn't
make any difference (unless i go too far back of course).

marge:..git/linux-2.6 # git remote update
Updating origin
Updating linux-next
Updating stable
Updating x86
>From git://git.kernel.org/pub/scm/linux/kernel/git/x86/linux-2.6-x86
 ! [rejected]        latest     -> x86/latest  (non fast forward)
 ! [rejected]        master     -> x86/master  (non fast forward)
 ! [rejected]        testing    -> x86/testing  (non fast forward)
Updating sched-devel
>From git://git.kernel.org/pub/scm/linux/kernel/git/mingo/linux-2.6-sched-devel
 ! [rejected]        for-akpm   -> sched-devel/for-akpm  (non fast forward)
 ! [rejected]        latest     -> sched-devel/latest  (non fast forward)
 ! [rejected]        master     -> sched-devel/master  (non fast forward)

If I rm/add the remote back, all goes fine... until it says rejected.

marge:..git/linux-2.6 # git remote rm x86
marge:..git/linux-2.6 # git remote add x86 git://git.kernel.org/pub/scm/linux/kernel/git/x86/linux-2.6-x86.git
marge:..git/linux-2.6 # git remote update
Updating origin
Updating linux-next
Updating stable
Updating sched-devel
Updating x86
>From git://git.kernel.org/pub/scm/linux/kernel/git/x86/linux-2.6-x86
 * [new branch]      base       -> x86/base
 * [new branch]      for-akpm   -> x86/for-akpm
 * [new branch]      for-linus  -> x86/for-linus
 * [new branch]      latest     -> x86/latest
 * [new branch]      master     -> x86/master
 * [new branch]      origin     -> x86/origin
 * [new branch]      testing    -> x86/testing
marge:..git/linux-2.6 #

Re: git remote update -> rejected

[Jeff King]

On Tue, Apr 22, 2008 at 11:48:53AM +0200, Mike Galbraith wrote:

> marge:..git/linux-2.6 # git remote update
> Updating origin
> Updating linux-next
> Updating stable
> Updating x86
> >From git://git.kernel.org/pub/scm/linux/kernel/git/x86/linux-2.6-x86
>  ! [rejected]        latest     -> x86/latest  (non fast forward)
>  ! [rejected]        master     -> x86/master  (non fast forward)
>  ! [rejected]        testing    -> x86/testing  (non fast forward)

The x86 tree was presumably rewound or rebased, which means that
fetching their new position would overwrite your old. This is probably
OK, since it looks like you have x86/* as tracking branches, and only
they would get overwritten. So it is probably safe to put a '+' in front
of the 'fetch' line in your config file. E.g.,

  [remote "x86"]
    url = ...
    fetch = +refs/heads/*:refs/remotes/x86/*

See 'git help fetch' for details.

-Peff

Re: git remote update -> rejected

[Mike Galbraith]

On Tue, 2008-04-22 at 06:34 -0400, Jeff King wrote:
> On Tue, Apr 22, 2008 at 11:48:53AM +0200, Mike Galbraith wrote:
> 
> > marge:..git/linux-2.6 # git remote update
> > Updating origin
> > Updating linux-next
> > Updating stable
> > Updating x86
> > >From git://git.kernel.org/pub/scm/linux/kernel/git/x86/linux-2.6-x86
> >  ! [rejected]        latest     -> x86/latest  (non fast forward)
> >  ! [rejected]        master     -> x86/master  (non fast forward)
> >  ! [rejected]        testing    -> x86/testing  (non fast forward)
> 
> The x86 tree was presumably rewound or rebased, which means that
> fetching their new position would overwrite your old. This is probably
> OK, since it looks like you have x86/* as tracking branches, and only
> they would get overwritten. So it is probably safe to put a '+' in front
> of the 'fetch' line in your config file. E.g.,
> 
>   [remote "x86"]
>     url = ...
>     fetch = +refs/heads/*:refs/remotes/x86/*
> 
> See 'git help fetch' for details.

Thanks a bunch.  I'll try it.  I notice that origin and linux-next
already had + prefix.  Presumably that came from the home repo.

	-Mike

Re: git remote update -> rejected

[Jeff King]

On Tue, Apr 22, 2008 at 12:55:03PM +0200, Mike Galbraith wrote:

> >   [remote "x86"]
> >     url = ...
> >     fetch = +refs/heads/*:refs/remotes/x86/*
> > 
> > See 'git help fetch' for details.
> 
> Thanks a bunch.  I'll try it.  I notice that origin and linux-next
> already had + prefix.  Presumably that came from the home repo.

Cloning creates an origin with '+' in the fetch line, but "git remote
add" doesn't seem to. Hrm, it looks like this is a regression from the
recent rewrite in C.

Dscho, is this a bug, or was there a conscious decision that I missed?

If a bug, the fix is below.

-- >8 --
remote: create fetch config lines with '+'

Since git-remote always uses remote tracking branches, it
should be safe to always force updates of those branches.
I.e., we should generate

  fetch = +refs/heads/*:refs/remotes/$remote/*

instead of

  fetch = refs/heads/*:refs/remotes/$remote/*

This was the behavior of the perl version, which seems to
have been lost in the C rewrite.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin-remote.c  |    1 +
 t/t5505-remote.sh |   10 ++++++++++
 2 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/builtin-remote.c b/builtin-remote.c
index 9d4432b..8b63619 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c
@@ -107,6 +107,7 @@ static int add(int argc, const char **argv)
 		struct path_list_item *item = track.items + i;
 
 		strbuf_reset(&buf2);
+		strbuf_addch(&buf2, '+');
 		if (mirror)
 			strbuf_addf(&buf2, "refs/%s:refs/%s",
 					item->path, item->path);
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index af2d077..48ff2d4 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -77,6 +77,16 @@ test_expect_success 'add another remote' '
 )
 '
 
+test_expect_success 'remote forces tracking branches' '
+(
+	cd test &&
+	case `git config remote.second.fetch` in
+	+*) true ;;
+	 *) false ;;
+	esac
+)
+'
+
 test_expect_success 'remove remote' '
 (
 	cd test &&
-- 
1.5.5.1.116.g0023.dirty

Re: git remote update -> rejected

[Johannes Schindelin]

Hi Peff,

On Tue, 22 Apr 2008, Jeff King wrote:

> On Tue, Apr 22, 2008 at 12:55:03PM +0200, Mike Galbraith wrote:
> 
> > >   [remote "x86"]
> > >     url = ...
> > >     fetch = +refs/heads/*:refs/remotes/x86/*
> > > 
> > > See 'git help fetch' for details.
> > 
> > Thanks a bunch.  I'll try it.  I notice that origin and linux-next
> > already had + prefix.  Presumably that came from the home repo.
> 
> Cloning creates an origin with '+' in the fetch line, but "git remote
> add" doesn't seem to. Hrm, it looks like this is a regression from the
> recent rewrite in C.

Thanks for working on this: I missed this thread (had too many mails to 
catch up with).

> Dscho, is this a bug, or was there a conscious decision that I missed?

It was a concious decision, but maybe it was wrong.

My rationale was this: if I mirror another person's repository, I want all 
the history.  And I do want to keep it, even if the other person decides 
to clean the original repository out.

(In this case, the reflogs would not help, since I do not have a HEAD 
reflog for all the deleted branches, and deleting the refs deletes their 
reflogs, too.)

But as I said, my rationale may very well be wrong.

Ciao,
Dscho

Re: git remote update -> rejected

[Jeff King]

Mike wrote:

> > > Thanks a bunch.  I'll try it.  I notice that origin and linux-next
> > > already had + prefix.  Presumably that came from the home repo.

Dscho wrote:

> It was a concious decision, but maybe it was wrong.
> 
> My rationale was this: if I mirror another person's repository, I want all 
> the history.  And I do want to keep it, even if the other person decides 
> to clean the original repository out.
>
> (In this case, the reflogs would not help, since I do not have a HEAD 
> reflog for all the deleted branches, and deleting the refs deletes their 
> reflogs, too.)

Hmm, there is an inconsistency here, though, since we set it up one way
on clone and another way on "remote add". Though if clone does finally
become "init + remote add + checkout" that will resolve itself.

Still, I think I prefer the old "+" behavior. We don't actually
_delete_ branches, we just allow non-fast-forward updates. The reflogs
will still be there.

-Peff

Re: git remote update -> rejected

[Johannes Schindelin]

Hi,

On Tue, 22 Apr 2008, Jeff King wrote:

> Dscho wrote:
> 
> > It was a concious decision, but maybe it was wrong.
> > 
> > My rationale was this: if I mirror another person's repository, I want 
> > all the history.  And I do want to keep it, even if the other person 
> > decides to clean the original repository out.
> >
> > (In this case, the reflogs would not help, since I do not have a HEAD 
> > reflog for all the deleted branches, and deleting the refs deletes 
> > their reflogs, too.)
> 
> Hmm, there is an inconsistency here, though, since we set it up one way 
> on clone and another way on "remote add". Though if clone does finally 
> become "init + remote add + checkout" that will resolve itself.
> 
> Still, I think I prefer the old "+" behavior. We don't actually
> _delete_ branches, we just allow non-fast-forward updates. The reflogs
> will still be there.

Oh, that's right: only "git remote prune" will delete stale refs only.

But my other point about possibly interfering with local branches still 
holds true.

Ciao,
Dscho

Re: git remote update -> rejected

[Jeff King]

On Tue, Apr 22, 2008 at 02:03:43PM +0100, Johannes Schindelin wrote:

> Oh, that's right: only "git remote prune" will delete stale refs only.
> 
> But my other point about possibly interfering with local branches still 
> holds true.

In that case, I think that the '+' should come only without --mirror,
and my complaint is then that "--mirror" is a horrible name for that
option. Though when I saw it, I really expected it to do something about
the _push_ line, since that is the only other place we have a --mirror
option. It would make sense to me for it to set up remote.$x.mirror
(which is newly added in next). But then, git-remote doesn't seem to be
geared towards pushing at all.

-Peff

[PATCH] builtin-remote: resurrect forced updates to tracked branches

[Johannes Schindelin]

During the rewrite of git-remote as a builtin, the ability to force
updating the tracked branches (i.e. forcing non-fast-forwards) was
lost.

This patch fixes that.

Noticed by Mike Galbraith, analyzed by Jeff King.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

	On Tue, 22 Apr 2008, Jeff King wrote:

	> On Tue, Apr 22, 2008 at 02:03:43PM +0100, Johannes Schindelin wrote:
	> 
	> > Oh, that's right: only "git remote prune" will delete stale refs only.
	> > 
	> > But my other point about possibly interfering with local 
	> > branches still holds true.
	> 
	> In that case, I think that the '+' should come only without 
	> --mirror, and my complaint is then that "--mirror" is a horrible name 
	> for that option. Though when I saw it, I really expected it to do 
	> something about the _push_ line, since that is the only other place we 
	> have a --mirror option. It would make sense to me for it to set up 
	> remote.$x.mirror (which is newly added in next). But then, git-remote 
	> doesn't seem to be geared towards pushing at all.

	I still think that the --mirror option has merit, but I missed 
	that your patch did fix the behviour _without_ --mirror.

	This is my attempt.

 builtin-remote.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin-remote.c b/builtin-remote.c
index d4f2132..117ff60 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c
@@ -114,7 +114,7 @@ static int add(int argc, const char **argv)
 			strbuf_addf(&buf2, "refs/%s:refs/%s",
 					item->path, item->path);
 		else
-			strbuf_addf(&buf2, "refs/heads/%s:refs/remotes/%s/%s",
+			strbuf_addf(&buf2, "+refs/heads/%s:refs/remotes/%s/%s",
 					item->path, name, item->path);
 		if (git_config_set_multivar(buf.buf, buf2.buf, "^$", 0))
 			return 1;

Re: git remote update -> rejected

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> Dscho, is this a bug, or was there a conscious decision that I missed?
> ...
> But as I said, my rationale may very well be wrong.

Whatever the rationale was, it is very wrong if the series was about
rewriting the scripted version in another language.  That is an
independent behaviour change.

And I do think the fix should be applied to --mirror case as well.  If it
does not "mirror" as it is told, what use would there be?

Re: git remote update -> rejected

[Teemu Likonen]

Jeff King wrote (2008-04-22 07:11 -0400):

> remote: create fetch config lines with '+'
> 
> Since git-remote always uses remote tracking branches, it
> should be safe to always force updates of those branches.
> I.e., we should generate
> 
>   fetch = +refs/heads/*:refs/remotes/$remote/*
> 
> instead of
> 
>   fetch = refs/heads/*:refs/remotes/$remote/*
> 
> This was the behavior of the perl version, which seems to
> have been lost in the C rewrite.

I agree, the "+" should be there. I see remote tracking branches as,
well, branches that track remote repository - no matter what happens
there. Local branches are under user's personal control, so if user
wants to save/keep the information of remote branches he can create
local ones out of them: git branch localcopy $remote/$branch

Re: git remote update -> rejected

[Johannes Schindelin]

Hi,

On Tue, 22 Apr 2008, Teemu Likonen wrote:

> Local branches are under user's personal control, so if user wants to 
> save/keep the information of remote branches he can create local ones 
> out of them: git branch localcopy $remote/$branch

For your benefit, I just assume that you did not yet read my reply to 
Peff's mail.

With the --mirror mode, you can no longer discern clearly between local 
and remote branches.  This is basically what we had in the beginning, 
before the "separate remotes layout".

So your point is not valid, an update will interfer with "local" branches.

And when those branches are deleted via "git fetch", you will not be able 
to reconstruct them easily, because the reflogs are deleted, too.

Ciao,
Dscho

Re: git remote update -> rejected

[Jeff King]

On Tue, Apr 22, 2008 at 01:56:36PM +0100, Johannes Schindelin wrote:

> With the --mirror mode, you can no longer discern clearly between local 
> and remote branches.  This is basically what we had in the beginning, 
> before the "separate remotes layout".
> 
> So your point is not valid, an update will interfer with "local" branches.
> 
> And when those branches are deleted via "git fetch", you will not be able 
> to reconstruct them easily, because the reflogs are deleted, too.

I think I'm a little confused. Is it possible to delete branches on a
fetch? It looks like remote's --mirror option just specifies "don't use
separate remote layout". So again, there is no way to blow away reflogs
through a fetch.

OTOH, if you are using non-separate-remote layout, I think it makes
sense to _not_ have the "+". OTOOH, when you call the option --mirror,
it makes me think of "push --mirror" which implies "+".

-Peff

Re: git remote update -> rejected

[Johannes Schindelin]

Hi,

On Tue, 22 Apr 2008, Jeff King wrote:

> OTOH, if you are using non-separate-remote layout, I think it makes
> sense to _not_ have the "+". OTOOH, when you call the option --mirror,
> it makes me think of "push --mirror" which implies "+".

I am glad somebody else than me finds this to be a dilemma.

Ciao,
Dscho

Re: git remote update -> rejected

[Paolo Bonzini]

Johannes Schindelin wrote:
> Hi,
> 
> On Tue, 22 Apr 2008, Jeff King wrote:
> 
>> OTOH, if you are using non-separate-remote layout, I think it makes
>> sense to _not_ have the "+". OTOOH, when you call the option --mirror,
>> it makes me think of "push --mirror" which implies "+".
> 
> I am glad somebody else than me finds this to be a dilemma.

Pushing to a mirror implies a "+".  But pulling from a mirror had better 
not imply a "+", as Dscho pointed out in this thread and implemented in 
his patch.

Non-fast-forward pulls from a non-mirror are safe, so they can imply a "+".

Maybe, "git remote add --mirror" should add a "push" line instead of a 
"fetch" line, and should not allow -f or -t options.

Paolo

Re: git remote update -> rejected

[Teemu Likonen]

Johannes Schindelin wrote (2008-04-22 13:56 +0100):

> With the --mirror mode, you can no longer discern clearly between
> local and remote branches.  This is basically what we had in the
> beginning, before the "separate remotes layout".

Hmm, to me it looks like --mirror is for mirroring remote repository and
hence the "+" makes sense in here too. It's like "I want to make my
repository a copy of that remote repository" hence the name 'mirror'.
It'll possibly overwrite local branches but in my way of thinking
mirroring implies that possibility. The 'git remote' manual says that
--mirror "only makes sense in bare repositories", so the manual seems to
guide towards not having local working directory nor changes.

Re: git remote update -> rejected

[Johannes Schindelin]

Hi,

On Tue, 22 Apr 2008, Teemu Likonen wrote:

> Johannes Schindelin wrote (2008-04-22 13:56 +0100):
> 
> > With the --mirror mode, you can no longer discern clearly between
> > local and remote branches.  This is basically what we had in the
> > beginning, before the "separate remotes layout".
> 
> Hmm, to me it looks like --mirror is for mirroring remote repository and 
> hence the "+" makes sense in here too. It's like "I want to make my 
> repository a copy of that remote repository" hence the name 'mirror'. 
> It'll possibly overwrite local branches but in my way of thinking 
> mirroring implies that possibility. The 'git remote' manual says that 
> --mirror "only makes sense in bare repositories", so the manual seems to 
> guide towards not having local working directory nor changes.

Okay, how about this: take Peff's patch, but add a warning if --mirror is 
used on a non-bare repository?

Ciao,
Dscho

Re: git remote update -> rejected

[Teemu Likonen]

Johannes Schindelin wrote (2008-04-22 14:49 +0100):

> On Tue, 22 Apr 2008, Teemu Likonen wrote:
> 
> > Hmm, to me it looks like --mirror is for mirroring remote repository
> > and hence the "+" makes sense in here too. It's like "I want to make
> > my repository a copy of that remote repository" hence the name
> > 'mirror'. It'll possibly overwrite local branches but in my way of
> > thinking mirroring implies that possibility. The 'git remote' manual
> > says that --mirror "only makes sense in bare repositories", so the
> > manual seems to guide towards not having local working directory nor
> > changes.
> 
> Okay, how about this: take Peff's patch, but add a warning if --mirror
> is used on a non-bare repository?

Sounds good to me. Indeed, giving a warning is _very_ good idea as
--mirror is potentially, even likely, destructive with local changes and
working directory.

Re: git remote update -> rejected

[Paolo Bonzini]

>> Okay, how about this: take Peff's patch, but add a warning if --mirror
>> is used on a non-bare repository?
> 
> Sounds good to me. Indeed, giving a warning is _very_ good idea as
> --mirror is potentially, even likely, destructive with local changes and
> working directory.

What about changing --mirror to add a push line instead of a fetch line?

Paolo

Re: git remote update -> rejected

[Johannes Schindelin]

Hi,

On Tue, 22 Apr 2008, Paolo Bonzini wrote:

> > > Okay, how about this: take Peff's patch, but add a warning if 
> > > --mirror is used on a non-bare repository?
> > 
> > Sounds good to me. Indeed, giving a warning is _very_ good idea as 
> > --mirror is potentially, even likely, destructive with local changes 
> > and working directory.
> 
> What about changing --mirror to add a push line instead of a fetch line?

I would not expect --mirror to add a push line when "git-remote add" 
without --mirror does not a push line either.

Ciao,
Dscho

Re: git remote update -> rejected

[Paolo Bonzini]

Johannes Schindelin wrote:
> Hi,
> 
> On Tue, 22 Apr 2008, Paolo Bonzini wrote:
> 
>>>> Okay, how about this: take Peff's patch, but add a warning if 
>>>> --mirror is used on a non-bare repository?
>>> Sounds good to me. Indeed, giving a warning is _very_ good idea as 
>>> --mirror is potentially, even likely, destructive with local changes 
>>> and working directory.
>> What about changing --mirror to add a push line instead of a fetch line?
> 
> I would not expect --mirror to add a push line when "git-remote add" 
> without --mirror does not a push line either.

Let me reverse the question.  When does it make sense to use "git-remote 
add --mirror" with the current implementation?

It's not a rhetoric question.  I know when it would make sense to have 
push refspecs on a remote for which you plan to use "git push --mirror" 
(and in "next", that is the case if you create the remote with 
"git-remote add --mirror").  But I'm a total newbie for things that do 
not fit my workflows, so I don't know when it would make sense to pull 
from that kind of remote.

Paolo

Re: git remote update -> rejected

[Paolo Bonzini]

>>> What about changing --mirror to add a push line instead of a fetch line?
>>
>> I would not expect --mirror to add a push line when "git-remote add" 
>> without --mirror does not a push line either.
> 
> Let me reverse the question.  When does it make sense to use "git-remote 
> add --mirror" with the current implementation?
> 
> It's not a rhetoric question.  I know when it would make sense to have 
> push refspecs on a remote for which you plan to use "git push --mirror" 
> (and in "next", that is the case if you create the remote with 
> "git-remote add --mirror").  But I'm a total newbie for things that do 
> not fit my workflows, so I don't know when it would make sense to pull 
> from that kind of remote.

And actually, I just realized that what I expected from --mirror is this:

[remote "mirror"]
         url = blah
         fetch = +refs/heads/*:refs/remotes/mirror/*
         push = +refs/heads/*:refs/heads/*
         mirror

... so that I can check the state of the mirror with "git log -1 
mirror/master", and still the push refspec is there so that my local 
remotes are not entirely mirrored.

Paolo

Re: git remote update -> rejected

[Johannes Schindelin]

Hi,

On Tue, 22 Apr 2008, Paolo Bonzini wrote:

> > > > What about changing --mirror to add a push line instead of a fetch 
> > > > line?
> > >
> > > I would not expect --mirror to add a push line when "git-remote add" 
> > > without --mirror does not a push line either.
> > 
> > Let me reverse the question.  When does it make sense to use 
> > "git-remote add --mirror" with the current implementation?
> > 
> > It's not a rhetoric question.  I know when it would make sense to have push
> > refspecs on a remote for which you plan to use "git push --mirror" (and in
> > "next", that is the case if you create the remote with "git-remote add
> > --mirror").  But I'm a total newbie for things that do not fit my workflows,
> > so I don't know when it would make sense to pull from that kind of remote.
> 
> And actually, I just realized that what I expected from --mirror is this:
> 
> [remote "mirror"]
>         url = blah
>         fetch = +refs/heads/*:refs/remotes/mirror/*
>         push = +refs/heads/*:refs/heads/*
>         mirror
> 
> ... so that I can check the state of the mirror with "git log -1
> mirror/master", and still the push refspec is there so that my local remotes
> are not entirely mirrored.

AFAICT this contradicts everything in the original thread resulting in the 
--mirror patch.

Ciao,
Dscho

Re: git remote update -> rejected

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> For your benefit, I just assume that you did not yet read my reply to 
> Peff's mail.
>
> With the --mirror mode, you can no longer discern clearly between local 
> and remote branches.  This is basically what we had in the beginning, 
> before the "separate remotes layout".
>
> So your point is not valid, an update will interfer with "local" branches.

I personally do not think _your_ point is valid.  Doesn't --mirror mean
you do not have local branches?

At least that was my understanding of the intention of the --mirror
option.  You give control away to the other end on the ref namespace, so
that you can treat it as a, eh, "mirror".  Perhaps you would want to have
a single such repository behind a firewall or this side of slow link and
use it as a feeder repository to serve many other clones on this side.

I personally do not think --mirror option makes sense with --track, nor
it makes sense in a non-bare repository for that matter.

[PATCH] remote add: disallow --master and --mirror with non-bare repositories

[Johannes Schindelin]

In most cases, --master or --mirror in a non-bare repository is an
error.  In those cases where it is not, the user is most likely editing
the config herself anyway.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

	On Tue, 22 Apr 2008, Junio C Hamano wrote:

	> I personally do not think --mirror option makes sense with 
	> --track, nor it makes sense in a non-bare repository for that matter.

	Obviously meant for application together with Peff's patch.

 builtin-remote.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/builtin-remote.c b/builtin-remote.c
index d4f2132..63f025c 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c
@@ -79,6 +79,12 @@ static int add(int argc, const char **argv)
 
 	argc = parse_options(argc, argv, options, builtin_remote_usage, 0);
 
+	if (mirror && is_bare_repository())
+		die("--mirror with non-bare repository.");
+
+	if (master && is_bare_repository())
+		die("--master with non-bare repository.");
+
 	if (argc < 2)
 		usage_with_options(builtin_remote_usage, options);
 

Re: [PATCH] remote add: disallow --master and --mirror with non-bare repositories

[Paolo Bonzini]

Johannes Schindelin wrote:
> In most cases, --master or --mirror in a non-bare repository is an
> error.  In those cases where it is not, the user is most likely editing
> the config herself anyway.

Ah, so I think we're speaking about two different features... sorry for 
the noise.

Paolo

Re: [PATCH] remote add: disallow --master and --mirror with non-bare repositories (review)

[Paolo Bonzini]

> +	if (mirror && is_bare_repository())
> +		die("--mirror with non-bare repository.");
> +

Missing bang.  I think you need to adjust the testsuite, in fact.

Paolo

Re: [PATCH] remote add: disallow --master and --mirror with non-bare repositories (review)

[Johannes Schindelin]

Hi,

On Tue, 22 Apr 2008, Paolo Bonzini wrote:

> > +	if (mirror && is_bare_repository())
> > +		die("--mirror with non-bare repository.");
> > +
> 
> Missing bang.  I think you need to adjust the testsuite, in fact.

Yep, I probably do.  If Junio nobody else beats me to it.

Ciao,
Dscho