Git development
 help / color / mirror / Atom feed
* Re: [PATCH/RFC 0/5] win32: support echo for terminal-prompt
From: Erik Faye-Lund @ 2012-11-30 10:16 UTC (permalink / raw)
  To: git, msysgit; +Cc: peff
In-Reply-To: <1352815447-8824-1-git-send-email-kusmabite@gmail.com>

Ping?

On Tue, Nov 13, 2012 at 3:04 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
> We currently only support getpass, which does not echo at all, for
> git_terminal_prompt on Windows. The Windows console is perfectly
> capable of doing this, so let's make it so.
>
> This implementation tries to reuse the /dev/tty-code as much as
> possible.
>
> The big reason that this becomes a bit hairy is that Ctrl+C needs
> to be handled correctly, so we don't leak the console state to a
> non-echoing setting when a user aborts.
>
> Windows makes this bit a little bit tricky, in that we need to
> implement SIGINT for fgetc. However, I suspect that this is a good
> thing to do in the first place.
>
> An earlier iteration was also breifly discussed here:
> http://mid.gmane.org/CABPQNSaUCEDU4+2N63n0k_XwSXOP_iFZG3GEYSPSBPcSVV8wRQ@mail.gmail.com
>
> The series can also be found here, only with an extra patch that
> makes the (interactive) testing a bit easier:
>
> https://github.com/kusma/git/tree/work/terminal-cleanup
>
> Erik Faye-Lund (5):
>   mingw: make fgetc raise SIGINT if apropriate
>   compat/terminal: factor out echo-disabling
>   compat/terminal: separate input and output handles
>   mingw: reuse tty-version of git_terminal_prompt
>   mingw: get rid of getpass implementation
>
>  compat/mingw.c    |  91 +++++++++++++++++++++++++++-----------
>  compat/mingw.h    |   8 +++-
>  compat/terminal.c | 129 ++++++++++++++++++++++++++++++++++++++++--------------
>  3 files changed, 169 insertions(+), 59 deletions(-)
>
> --
> 1.8.0.7.gbeffeda
>

^ permalink raw reply

* Re: [PATCH 1/2] git-fast-import.txt: improve documentation for quoted paths
From: Matthieu Moy @ 2012-11-30  9:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7vvccop6dv.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> That "shell-style" contradicts with what fast-import.c says, though.
> It claims to grok \octal and described as C-style.

As Peff mentionned, my last version is better, although still a bit
incomplete. My new version documents things that _must_ be escaped, but
not what _can_.

I'm reluctant to try being exhaustive in the .txt documentation if there
is an exhaustive comment in the code. One option would be to actually
turn the comment into an official specification by moving it to the .txt
file, but that goes far beyond the scope of my patch.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply

* Re: [PATCH] t4049: avoid test failures on filemode challenged file systems (Windows)
From: Johannes Sixt @ 2012-11-30  7:46 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Antoine Pelisse
In-Reply-To: <7vfw3sp232.fsf@alter.siamese.dyndns.org>

Am 11/29/2012 21:48, schrieb Junio C Hamano:
> I've tested this with the testpen set on vfat mounted on my Linux
> box, ...
> and it seems to work OK,

Works well here on Windows, too.

Thanks,
-- Hannes

^ permalink raw reply

* Re: [PATCH v7 p2 1/2] fast-export: don't handle uninteresting refs
From: Felipe Contreras @ 2012-11-30  5:57 UTC (permalink / raw)
  To: Max Horn; +Cc: git, Junio C Hamano, Jeff King
In-Reply-To: <8FA492C2-71B0-44AB-B816-AFB6C91DC01C@quendi.de>

On Thu, Nov 29, 2012 at 2:16 AM, Max Horn <postbox@quendi.de> wrote:
>
> On 28.11.2012, at 23:23, Felipe Contreras wrote:
>
>> They have been marked as UNINTERESTING for a reason, lets respect that.
>>
>> Currently the first ref is handled properly, but not the rest:
>>
>>  % git fast-export master ^uninteresting ^foo ^bar
>
> All these refs are assumed to point to the same object, right? I think it would be better if the commit message stated that explicitly. To make up for the lost space, you could then get rid of one of the four refs, I think three are sufficient to drive the message home ;-).

Yeah, they point to the same object.

> <snip>
>
>> The reason this happens is that before traversing the commits,
>> fast-export checks if any of the refs point to the same object, and any
>> duplicated ref gets added to a list in order to issue 'reset' commands
>> after the traversing. Unfortunately, it's not even checking if the
>> commit is flagged as UNINTERESTING. The fix of course, is to do
>> precisely that.
>
> Hm... So this might be me being a stupid n00b (I am not yet that familiar with the internal rep of things in git and all...)... but I found the "precisely that" par very confusing, because right afterwards, you say:

Yeah, the next part was added afterwards.

>> However, in order to do it properly we need to get the UNINTERESTING flag
>> from the command line ref, not from the commit object.
>
> So this sounds like you are saying "we do *precisely* that, except we don't, because it is more complicated, so we actually don't do this *precisely*, just manner of speaking..."

Well, we do check fro the UNINTERESTING flag, but on the ref, not on the commit.

> Some details here are beyond my knowledge, I am afraid, so I have to resort to guess: In particular it is not clear to me why the "however" part pops up: Reading it makes it sound as if the commit object also carries an UNINTERESTING flag, but we can't use it because of some reason (perhaps it doesn't have the semantics we need?), so we have to look at revs.pending instead. Right? Wrong? Or is it because the commit objects actually do *not* carry the UNINTERESTING bits, hence we need to look at revs.pending. Or is it due to yet another reason?

It's actually revs.cmdline, I typed the wrong one.

If you have two refs pointing to the same object, and you do 'one
^two', the object (e.g. 8c7a786) will get the UNINTERESTING flag, but
that doesn't tell us anything about the ref being a positive or a
negative one, and revs.pending only has the object flags. On the other
hand revs.cmdline does have the flags for the refs.

Does that explain it?

> Anyway, other than these nitpicky questions, this whole thing looks very logical to me, description and code alike. I also played around with tons of "fast-export" invocations, with and without this patch, and it seems to do what the description says. Finally, I went to the various long threads discussion prior versions of this patch, in particular those starting at
>   http://thread.gmane.org/gmane.comp.version-control.git/208725
> and
>   http://thread.gmane.org/gmane.comp.version-control.git/209355/focus=209370
>
> These contained some concerns. Sadly, several of those discussions ultimately degenerated into not-so-pleasant exchanges :-(, and my impression is that as a result some people are not so inclined to comment on these patches anymore at all. Which is a pity :-(. But overall, it seems this patch makes nothing worse, but fixes some things; and it is simple enough that it shouldn't make future improvements harder.
>
> So *I* at least am quite happy with this, it helps me! My impression is that Felipe's latest patch addresses most concerns people raised by means of an improved description. I couldn't find any in those threads that I feel still applies -- but of course those people should speak for themselves, I am simply afraid they don't want to be part of this anymore :-(.

Indeed. For all the concerns given I made a response to how that
either is not true, or doesn't really matter, and in the case of the
latter, I asked for examples where it would matter, only to receive
nothing. For whatever reason involved people are not responding, not a
single valid concern has been raised and remained.

So I think it's good.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply

* New git.pot is generated for git 1.8.1 l10n round 2
From: Jiang Xin @ 2012-11-30  5:01 UTC (permalink / raw)
  To: Byrial Jensen, Ralf Thielow,
	Ævar Arnfjörð Bjarmason, Marco Paolone,
	Vincent van Ravesteijn, Marco Sousa, Peter Krefting,
	Trần Ngọc Quân, David Hrbáč
  Cc: Git List

Hi,

New "git.pot" is generated from v1.8.0.1-347-gf94c3 in the master branch.

    l10n: Update git.pot (5 new, 1 removed messages)

    L10n for git 1.8.1 round 2: Generate po/git.pot from v1.8.0.1-347-gf94c3.

    Signed-off-by: Jiang Xin <worldhello.net@gmail.com>

This update is for the l10n of upcoming git 1.8.1. You can get it from
the usual place:

    https://github.com/git-l10n/git-po/

--
Jiang Xin

^ permalink raw reply

* Re: [Query] Can we ignore case for commiters name in shortlog?
From: Viresh Kumar @ 2012-11-30  3:35 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.LFD.2.02.1211292231520.4576@xanadu.home>

On 30 November 2012 09:03, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:

> Have a look at the .mailmap file in the top directory of your repo.

Repeating what i said to David in other mail:

I have my name there :)

I thought using names with different case is actually different then misspelling
it. And so, everybody must not be required to update their names in mailmap
with different case. So, with same email id and same name (that may be in
different case), we can show commits together in shortlog.

--
viresh

^ permalink raw reply

* Re: [Query] Can we ignore case for commiters name in shortlog?
From: Nicolas Pitre @ 2012-11-30  3:33 UTC (permalink / raw)
  To: viresh kumar; +Cc: Junio C Hamano, git
In-Reply-To: <CAOh2x==NBeeoE2=PhaDC143ZF_xHKD5m=Po+-DS2X43CEeGiEQ@mail.gmail.com>

On Fri, 30 Nov 2012, viresh kumar wrote:

> Hi Junio and others,
> 
> I have a query. git shortlog lists the patches submitted per commiter, like:
> 
> > Viresh Kumar (7):
> >       cpufreq: Improve debug prints
> >       cpufreq: return early from __cpufreq_driver_getavg()
> >       cpufreq: governors: remove redundant code
> >       cpufreq: Fix sparse warnings by updating cputime64_t to u64
> >       cpufreq: Fix sparse warning by making local function static
> >       cpufreq: Avoid calling cpufreq driver's target() routine if target_freq == policy->cur
> >       cpufreq: Make sure target freq is within limits
> 
> > viresh kumar (3):
> >       cpufreq / core: Fix typo in comment describing show_bios_limit()
> >       cpufreq / core: Fix printing of governor and driver name
> >       cpufreq: Move common part from governors to separate file, v2
> 
> I know, there was something wrong at my end and i have commited stuff
> with different cases.
> 
> I was just thinking if we can ignore case for commiter name while
> listing stuff here?
> So, that we get over any manual mistakes from commiter.

Have a look at the .mailmap file in the top directory of your repo.


Nicolas

^ permalink raw reply

* Re: [Query] Can we ignore case for commiters name in shortlog?
From: Viresh Kumar @ 2012-11-30  3:32 UTC (permalink / raw)
  To: David Aguilar; +Cc: Junio C Hamano, git
In-Reply-To: <CAJDDKr7yr2JSutcEy1mz-SfMq8ZdNzR3+s++ooenn5+wD-LDAw@mail.gmail.com>

On 30 November 2012 08:54, David Aguilar <davvid@gmail.com> wrote:
> There's a feature that does exactly this.
>
> http://www.kernel.org/pub/software/scm/git/docs/git-shortlog.html
>
> See the section called "Mapping Authors".
> It discusses the .mailmap file.

I have my name there :)

I thought using names with different case is actually different then misspelling
it. And so, everybody must not be required to update their names in mailmap
with different case. So, with same email id and same name (that may be in
different case), we can show commits together in shortlog.

--
viresh

^ permalink raw reply

* Re: [Query] Can we ignore case for commiters name in shortlog?
From: David Aguilar @ 2012-11-30  3:24 UTC (permalink / raw)
  To: viresh kumar; +Cc: Junio C Hamano, git
In-Reply-To: <CAOh2x==NBeeoE2=PhaDC143ZF_xHKD5m=Po+-DS2X43CEeGiEQ@mail.gmail.com>

On Thu, Nov 29, 2012 at 6:09 PM, viresh kumar <viresh.kumar@linaro.org> wrote:
> Hi Junio and others,
>
> I have a query. git shortlog lists the patches submitted per commiter, like:
>
>> Viresh Kumar (7):
>>       cpufreq: Improve debug prints
>>       cpufreq: return early from __cpufreq_driver_getavg()
>>       cpufreq: governors: remove redundant code
>>       cpufreq: Fix sparse warnings by updating cputime64_t to u64
>>       cpufreq: Fix sparse warning by making local function static
>>       cpufreq: Avoid calling cpufreq driver's target() routine if target_freq == policy->cur
>>       cpufreq: Make sure target freq is within limits
>
>> viresh kumar (3):
>>       cpufreq / core: Fix typo in comment describing show_bios_limit()
>>       cpufreq / core: Fix printing of governor and driver name
>>       cpufreq: Move common part from governors to separate file, v2
>
> I know, there was something wrong at my end and i have commited stuff
> with different cases.
>
> I was just thinking if we can ignore case for commiter name while
> listing stuff here?
> So, that we get over any manual mistakes from commiter.

There's a feature that does exactly this.

http://www.kernel.org/pub/software/scm/git/docs/git-shortlog.html

See the section called "Mapping Authors".
It discusses the .mailmap file.
-- 
David

^ permalink raw reply

* Re: [PATCH v5 0/2] submodule update: add --remote for submodule's upstream changes
From: W. Trevor King @ 2012-11-30  3:27 UTC (permalink / raw)
  To: Phil Hord
  Cc: Git, Junio C Hamano, Heiko Voigt, Jeff King, Shawn Pearce,
	Jens Lehmann, Nahor
In-Reply-To: <CABURp0piLAG+hEsav-uro+nq9ZRZ9CFFjVG8VKYk3ZtYvRi8=A@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 3121 bytes --]

On Thu, Nov 29, 2012 at 08:11:20PM -0500, Phil Hord wrote:
> On Thu, Nov 29, 2012 at 2:13 PM, W. Trevor King <wking@tremily.us> wrote:
> > On Thu, Nov 29, 2012 at 01:29:12PM -0500, Phil Hord wrote:
> >> For that reason, I don't like the --pull switch since it implies a
> >> fetch, but I will not always want to do a fetch.
> >
> >   $ git submodule update --remote --no-fetch
> >
> > will not fetch the submodule remotes.
> 
> This seems precisely backwards to me. Why not use
> 
>   $ git submodule update --remote --fetch
> 
> to do your "default" behavior instead?   I suppose I am arguing
> against the tide of the dominant workflow, but the fetch-by-default
> idea needlessly conflates two primitive operations:  "float" and
> "fetch".

Because --no-fetch is the existing option, and if it ain't broke… ;).

> >> I don't know which remote I should be tracking, though.  I suppose
> >> it is 'origin' for now, but maybe it is just whatever
> >> $superproject's HEAD's remote-tracking branch indicates.
> >
> > With the --remote series, I always use "origin" because that's what
> > `submodule add` should be setting up.  If people want to change that
> > up by hand, we may need a submodule.<name>.remote configuration
> > option.
> 
> I've always felt that the "origin" defaults are broken and are simply
> being ignored because most users do not trip over them.  But ISTR that
> submodule commands use the remote indicated by the superproject's
> current remote-tracking configuration, with a fallback to 'origin' if
> there is none.  Sort of a "best effort" algorithm, I think.  Am I
> remembering that wrong?

The current code uses a bare "git-fetch".  I'm not sure what that
defaults to if you're on a detached head.  If it bothers you, I'm fine
adding the submodule.<name>.remote option in v6.

> >> I am not sure I want the gitlinks in superproject to update automatically
> >> in the index, but I definitely do not want to automatically create a commit
> >> for them.
> >
> > Commits are dissabled by default (see my recent --commit RFC for how
> > they would be enabled).
> >
> >> But I really don't want to figure out how to handle submodule
> >> collisions during a merge (or rebase!) of my superproject with changes that
> >> someone else auto-committed in his local $superproject as he and I
> >> arbitrarily floated up the upstream independently.  There is nothing but
> >> loathing down that path.
> >
> > This is true.  I'm not sure how gitlink collisions are currently
> > handled…
> 
> They've always been trouble for me.  But it may be that I am ignorant.

I haven't dealt with any gitlink merges, but I think that supporting
easy gitlink merges is orthogonal to this --remote option.  For simple
cases like "autocommitted submodule floats", one of the conflicting
gitlinks will be an ancestor of the other, so it should be easy to
automate that merge.

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* [Query] Can we ignore case for commiters name in shortlog?
From: viresh kumar @ 2012-11-30  2:09 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Hi Junio and others,

I have a query. git shortlog lists the patches submitted per commiter, like:

> Viresh Kumar (7):
>       cpufreq: Improve debug prints
>       cpufreq: return early from __cpufreq_driver_getavg()
>       cpufreq: governors: remove redundant code
>       cpufreq: Fix sparse warnings by updating cputime64_t to u64
>       cpufreq: Fix sparse warning by making local function static
>       cpufreq: Avoid calling cpufreq driver's target() routine if target_freq == policy->cur
>       cpufreq: Make sure target freq is within limits

> viresh kumar (3):
>       cpufreq / core: Fix typo in comment describing show_bios_limit()
>       cpufreq / core: Fix printing of governor and driver name
>       cpufreq: Move common part from governors to separate file, v2

I know, there was something wrong at my end and i have commited stuff
with different cases.

I was just thinking if we can ignore case for commiter name while
listing stuff here?
So, that we get over any manual mistakes from commiter.

--
viresh

^ permalink raw reply

* [PATCH v6 8/8] push: cleanup push rules comment
From: Chris Rorvick @ 2012-11-30  1:41 UTC (permalink / raw)
  To: git
  Cc: Chris Rorvick, Angelo Borsotti, Drew Northup, Michael Haggerty,
	Philip Oakley, Johannes Sixt, Kacper Kornet, Jeff King,
	Felipe Contreras, Junio C Hamano
In-Reply-To: <1354239700-3325-1-git-send-email-chris@rorvick.com>

Rewrite to remove inter-dependencies amongst the rules.

Signed-off-by: Chris Rorvick <chris@rorvick.com>
---
 remote.c | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/remote.c b/remote.c
index ee0c1e5..6309a87 100644
--- a/remote.c
+++ b/remote.c
@@ -1319,27 +1319,29 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
 			continue;
 		}
 
-		/* This part determines what can overwrite what.
-		 * The rules are:
+		/*
+		 * The below logic determines whether an individual
+		 * refspec A:B can be pushed.  The push will succeed
+		 * if any of the following are true:
 		 *
-		 * (0) you can always use --force or +A:B notation to
-		 *     selectively force individual ref pairs.
+		 * (1) the remote reference B does not exist
 		 *
-		 * (1) if the old thing does not exist, it is OK.
+		 * (2) the remote reference B is being removed (i.e.,
+		 *     pushing :B where no source is specified)
 		 *
-		 * (2) if the destination is under refs/tags/ you are
-		 *     not allowed to overwrite it; tags are expected
-		 *     to be static once created
+		 * (3) the update meets all fast-forwarding criteria:
 		 *
-		 * (3) if you do not have the old thing, you are not allowed
-		 *     to overwrite it; you would not know what you are losing
-		 *     otherwise.
+		 *     (a) the destination is not under refs/tags/
+		 *     (b) the old is a commit
+		 *     (c) the new is a descendant of the old
 		 *
-		 * (4) if old is a commit and new is a descendant of old
-		 *     (implying new is commit-ish), it is OK.
+		 *     NOTE: We must actually have the old object in
+		 *     order to overwrite it in the remote reference,
+		 *     and that the new object must be commit-ish.
+		 *     These are implied by (b) and (c) respectively.
 		 *
-		 * (5) regardless of all of the above, removing :B is
-		 *     always allowed.
+		 * (4) it is forced using the +A:B notation, or by
+		 *     passing the --force argument
 		 */
 
 		ref->not_forwardable = !is_forwardable(ref);
-- 
1.8.0.158.g0c4328c

^ permalink raw reply related

* [PATCH v6 6/8] push: require force for annotated tags
From: Chris Rorvick @ 2012-11-30  1:41 UTC (permalink / raw)
  To: git
  Cc: Chris Rorvick, Angelo Borsotti, Drew Northup, Michael Haggerty,
	Philip Oakley, Johannes Sixt, Kacper Kornet, Jeff King,
	Felipe Contreras, Junio C Hamano
In-Reply-To: <1354239700-3325-1-git-send-email-chris@rorvick.com>

Do not allow fast-forwarding of references that point to a tag object.
Updating from a tag is potentially destructive since it would likely
leave the tag dangling.  Disallowing updates to a tag also makes sense
semantically and is consistent with the behavior of lightweight tags.

Signed-off-by: Chris Rorvick <chris@rorvick.com>
---
 Documentation/git-push.txt | 10 +++++-----
 remote.c                   | 11 +++++++++--
 t/t5516-fetch-push.sh      | 21 +++++++++++++++++++++
 3 files changed, 35 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 09bdec7..7a04ce5 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -52,11 +52,11 @@ updated.
 +
 The object referenced by <src> is used to update the <dst> reference
 on the remote side.  By default this is only allowed if <dst> is not
-under refs/tags/, and then only if it can fast-forward <dst>.  By having
-the optional leading `+`, you can tell git to update the <dst> ref even
-if it is not allowed by default (e.g., it is not a fast-forward.)  This
-does *not* attempt to merge <src> into <dst>.  See EXAMPLES below for
-details.
+a tag (annotated or lightweight), and then only if it can fast-forward
+<dst>.  By having the optional leading `+`, you can tell git to update
+the <dst> ref even if it is not allowed by default (e.g., it is not a
+fast-forward.)  This does *not* attempt to merge <src> into <dst>.  See
+EXAMPLES below for details.
 +
 `tag <tag>` means the same as `refs/tags/<tag>:refs/tags/<tag>`.
 +
diff --git a/remote.c b/remote.c
index 012b52f..f5bc4e7 100644
--- a/remote.c
+++ b/remote.c
@@ -1281,9 +1281,16 @@ int match_push_refs(struct ref *src, struct ref **dst,
 
 static inline int is_forwardable(struct ref* ref)
 {
+	struct object *o;
+
 	if (!prefixcmp(ref->name, "refs/tags/"))
 		return 0;
 
+	/* old object must be a commit */
+	o = parse_object(ref->old_sha1);
+	if (!o || o->type != OBJ_COMMIT)
+		return 0;
+
 	return 1;
 }
 
@@ -1323,8 +1330,8 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
 		 *     to overwrite it; you would not know what you are losing
 		 *     otherwise.
 		 *
-		 * (4) if both new and old are commit-ish, and new is a
-		 *     descendant of old, it is OK.
+		 * (4) if old is a commit and new is a descendant of old
+		 *     (implying new is commit-ish), it is OK.
 		 *
 		 * (5) regardless of all of the above, removing :B is
 		 *     always allowed.
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 8f024a0..6009372 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -950,6 +950,27 @@ test_expect_success 'push requires --force to update lightweight tag' '
 	)
 '
 
+test_expect_success 'push requires --force to update annotated tag' '
+	mk_test heads/master &&
+	mk_child child1 &&
+	mk_child child2 &&
+	(
+		cd child1 &&
+		git tag -a -m "message 1" Tag &&
+		git push ../child2 Tag:refs/tmp/Tag &&
+		git push ../child2 Tag:refs/tmp/Tag &&
+		>file1 &&
+		git add file1 &&
+		git commit -m "file1" &&
+		git tag -f -a -m "message 2" Tag &&
+		test_must_fail git push ../child2 Tag:refs/tmp/Tag &&
+		git push --force ../child2 Tag:refs/tmp/Tag &&
+		git tag -f -a -m "message 3" Tag HEAD~ &&
+		test_must_fail git push ../child2 Tag:refs/tmp/Tag &&
+		git push --force ../child2 Tag:refs/tmp/Tag
+	)
+'
+
 test_expect_success 'push --porcelain' '
 	mk_empty &&
 	echo >.git/foo  "To testrepo" &&
-- 
1.8.0.158.g0c4328c

^ permalink raw reply related

* [PATCH v6 5/8] push: require force for refs under refs/tags/
From: Chris Rorvick @ 2012-11-30  1:41 UTC (permalink / raw)
  To: git
  Cc: Chris Rorvick, Angelo Borsotti, Drew Northup, Michael Haggerty,
	Philip Oakley, Johannes Sixt, Kacper Kornet, Jeff King,
	Felipe Contreras, Junio C Hamano
In-Reply-To: <1354239700-3325-1-git-send-email-chris@rorvick.com>

References are allowed to update from one commit-ish to another if the
former is an ancestor of the latter.  This behavior is oriented to
branches which are expected to move with commits.  Tag references are
expected to be static in a repository, though, thus an update to
something under refs/tags/ should be rejected unless the update is
forced.

Signed-off-by: Chris Rorvick <chris@rorvick.com>
---
 Documentation/git-push.txt | 11 ++++++-----
 builtin/push.c             |  2 +-
 builtin/send-pack.c        |  5 +++++
 cache.h                    |  1 +
 remote.c                   | 18 ++++++++++++++----
 send-pack.c                |  1 +
 t/t5516-fetch-push.sh      | 23 ++++++++++++++++++++++-
 transport-helper.c         |  6 ++++++
 transport.c                |  8 ++++++--
 9 files changed, 62 insertions(+), 13 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index fe46c42..09bdec7 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -51,11 +51,12 @@ be named. If `:`<dst> is omitted, the same ref as <src> will be
 updated.
 +
 The object referenced by <src> is used to update the <dst> reference
-on the remote side, but by default this is only allowed if the
-update can fast-forward <dst>.  By having the optional leading `+`,
-you can tell git to update the <dst> ref even when the update is not a
-fast-forward.  This does *not* attempt to merge <src> into <dst>.  See
-EXAMPLES below for details.
+on the remote side.  By default this is only allowed if <dst> is not
+under refs/tags/, and then only if it can fast-forward <dst>.  By having
+the optional leading `+`, you can tell git to update the <dst> ref even
+if it is not allowed by default (e.g., it is not a fast-forward.)  This
+does *not* attempt to merge <src> into <dst>.  See EXAMPLES below for
+details.
 +
 `tag <tag>` means the same as `refs/tags/<tag>:refs/tags/<tag>`.
 +
diff --git a/builtin/push.c b/builtin/push.c
index e08485d..83a3cc8 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -222,7 +222,7 @@ static const char message_advice_checkout_pull_push[] =
 
 static const char message_advice_ref_already_exists[] =
 	N_("Updates were rejected because the destination reference already exists\n"
-	   "in the remote and the update is not a fast-forward.");
+	   "in the remote.");
 
 static void advise_pull_before_push(void)
 {
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 9f98607..f849e0a 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -44,6 +44,11 @@ static void print_helper_status(struct ref *ref)
 			msg = "non-fast forward";
 			break;
 
+		case REF_STATUS_REJECT_ALREADY_EXISTS:
+			res = "error";
+			msg = "already exists";
+			break;
+
 		case REF_STATUS_REJECT_NODELETE:
 		case REF_STATUS_REMOTE_REJECT:
 			res = "error";
diff --git a/cache.h b/cache.h
index b7ab4ac..a32a0ea 100644
--- a/cache.h
+++ b/cache.h
@@ -1011,6 +1011,7 @@ struct ref {
 		REF_STATUS_NONE = 0,
 		REF_STATUS_OK,
 		REF_STATUS_REJECT_NONFASTFORWARD,
+		REF_STATUS_REJECT_ALREADY_EXISTS,
 		REF_STATUS_REJECT_NODELETE,
 		REF_STATUS_UPTODATE,
 		REF_STATUS_REMOTE_REJECT,
diff --git a/remote.c b/remote.c
index 4a6f822..012b52f 100644
--- a/remote.c
+++ b/remote.c
@@ -1315,14 +1315,18 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
 		 *
 		 * (1) if the old thing does not exist, it is OK.
 		 *
-		 * (2) if you do not have the old thing, you are not allowed
+		 * (2) if the destination is under refs/tags/ you are
+		 *     not allowed to overwrite it; tags are expected
+		 *     to be static once created
+		 *
+		 * (3) if you do not have the old thing, you are not allowed
 		 *     to overwrite it; you would not know what you are losing
 		 *     otherwise.
 		 *
-		 * (3) if both new and old are commit-ish, and new is a
+		 * (4) if both new and old are commit-ish, and new is a
 		 *     descendant of old, it is OK.
 		 *
-		 * (4) regardless of all of the above, removing :B is
+		 * (5) regardless of all of the above, removing :B is
 		 *     always allowed.
 		 */
 
@@ -1337,7 +1341,13 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
 				!has_sha1_file(ref->old_sha1)
 				  || !ref_newer(ref->new_sha1, ref->old_sha1);
 
-			if (ref->nonfastforward) {
+			if (ref->not_forwardable) {
+				ref->requires_force = 1;
+				if (!force_ref_update) {
+					ref->status = REF_STATUS_REJECT_ALREADY_EXISTS;
+					continue;
+				}
+			} else if (ref->nonfastforward) {
 				ref->requires_force = 1;
 				if (!force_ref_update) {
 					ref->status = REF_STATUS_REJECT_NONFASTFORWARD;
diff --git a/send-pack.c b/send-pack.c
index f50dfd9..1c375f0 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -229,6 +229,7 @@ int send_pack(struct send_pack_args *args,
 		/* Check for statuses set by set_ref_status_for_push() */
 		switch (ref->status) {
 		case REF_STATUS_REJECT_NONFASTFORWARD:
+		case REF_STATUS_REJECT_ALREADY_EXISTS:
 		case REF_STATUS_UPTODATE:
 			continue;
 		default:
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index b5417cc..8f024a0 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -368,7 +368,7 @@ test_expect_success 'push with colon-less refspec (2)' '
 		git branch -D frotz
 	fi &&
 	git tag -f frotz &&
-	git push testrepo frotz &&
+	git push -f testrepo frotz &&
 	check_push_result $the_commit tags/frotz &&
 	check_push_result $the_first_commit heads/frotz
 
@@ -929,6 +929,27 @@ test_expect_success 'push into aliased refs (inconsistent)' '
 	)
 '
 
+test_expect_success 'push requires --force to update lightweight tag' '
+	mk_test heads/master &&
+	mk_child child1 &&
+	mk_child child2 &&
+	(
+		cd child1 &&
+		git tag Tag &&
+		git push ../child2 Tag &&
+		git push ../child2 Tag &&
+		>file1 &&
+		git add file1 &&
+		git commit -m "file1" &&
+		git tag -f Tag &&
+		test_must_fail git push ../child2 Tag &&
+		git push --force ../child2 Tag &&
+		git tag -f Tag &&
+		test_must_fail git push ../child2 Tag HEAD~ &&
+		git push --force ../child2 Tag
+	)
+'
+
 test_expect_success 'push --porcelain' '
 	mk_empty &&
 	echo >.git/foo  "To testrepo" &&
diff --git a/transport-helper.c b/transport-helper.c
index 4713b69..965b778 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -661,6 +661,11 @@ static void push_update_ref_status(struct strbuf *buf,
 			free(msg);
 			msg = NULL;
 		}
+		else if (!strcmp(msg, "already exists")) {
+			status = REF_STATUS_REJECT_ALREADY_EXISTS;
+			free(msg);
+			msg = NULL;
+		}
 	}
 
 	if (*ref)
@@ -720,6 +725,7 @@ static int push_refs_with_push(struct transport *transport,
 		/* Check for statuses set by set_ref_status_for_push() */
 		switch (ref->status) {
 		case REF_STATUS_REJECT_NONFASTFORWARD:
+		case REF_STATUS_REJECT_ALREADY_EXISTS:
 		case REF_STATUS_UPTODATE:
 			continue;
 		default:
diff --git a/transport.c b/transport.c
index f3160b1..2673d27 100644
--- a/transport.c
+++ b/transport.c
@@ -695,6 +695,10 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i
 		print_ref_status('!', "[rejected]", ref, ref->peer_ref,
 						 "non-fast-forward", porcelain);
 		break;
+	case REF_STATUS_REJECT_ALREADY_EXISTS:
+		print_ref_status('!', "[rejected]", ref, ref->peer_ref,
+						 "already exists", porcelain);
+		break;
 	case REF_STATUS_REMOTE_REJECT:
 		print_ref_status('!', "[remote rejected]", ref,
 						 ref->deletion ? NULL : ref->peer_ref,
@@ -740,12 +744,12 @@ void transport_print_push_status(const char *dest, struct ref *refs,
 		    ref->status != REF_STATUS_OK)
 			n += print_one_push_status(ref, dest, n, porcelain);
 		if (ref->status == REF_STATUS_REJECT_NONFASTFORWARD) {
-			if (ref->not_forwardable)
-				*reject_reasons |= REJECT_ALREADY_EXISTS;
 			if (!strcmp(head, ref->name))
 				*reject_reasons |= REJECT_NON_FF_HEAD;
 			else
 				*reject_reasons |= REJECT_NON_FF_OTHER;
+		} else if (ref->status == REF_STATUS_REJECT_ALREADY_EXISTS) {
+			*reject_reasons |= REJECT_ALREADY_EXISTS;
 		}
 	}
 }
-- 
1.8.0.158.g0c4328c

^ permalink raw reply related

* [PATCH v6 7/8] push: clarify rejection of update to non-commit-ish
From: Chris Rorvick @ 2012-11-30  1:41 UTC (permalink / raw)
  To: git
  Cc: Chris Rorvick, Angelo Borsotti, Drew Northup, Michael Haggerty,
	Philip Oakley, Johannes Sixt, Kacper Kornet, Jeff King,
	Felipe Contreras, Junio C Hamano
In-Reply-To: <1354239700-3325-1-git-send-email-chris@rorvick.com>

Pushes must already (by default) update to a commit-ish due to the fast-
forward check in set_ref_status_for_push().  But rejecting for not being
a fast-forward suggests the situation can be resolved with a merge.
Flag these updates (i.e., to a blob or a tree) as not forwardable so the
user is presented with more appropriate advice.

While updating *from* a tag object is potentially destructive, updating
*to* a tag is not.  Additionally, a push to the refs/tags/ hierarchy is
already excluded from fast-forwarding, and refs/heads/ is protected from
anything but commit objects by a check in write_ref_sha1().  Thus
someone fast-forwarding to a tag is probably not doing so by accident.
Since updating to a tag is benign and unlikely to cause confusion, allow
it in case someone finds the behavior useful.

Signed-off-by: Chris Rorvick <chris@rorvick.com>
---
 remote.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/remote.c b/remote.c
index f5bc4e7..ee0c1e5 100644
--- a/remote.c
+++ b/remote.c
@@ -1291,6 +1291,11 @@ static inline int is_forwardable(struct ref* ref)
 	if (!o || o->type != OBJ_COMMIT)
 		return 0;
 
+	/* new object must be commit-ish */
+	o = deref_tag(parse_object(ref->new_sha1), NULL, 0);
+	if (!o || o->type != OBJ_COMMIT)
+		return 0;
+
 	return 1;
 }
 
-- 
1.8.0.158.g0c4328c

^ permalink raw reply related

* [PATCH v6 3/8] push: flag updates
From: Chris Rorvick @ 2012-11-30  1:41 UTC (permalink / raw)
  To: git
  Cc: Chris Rorvick, Angelo Borsotti, Drew Northup, Michael Haggerty,
	Philip Oakley, Johannes Sixt, Kacper Kornet, Jeff King,
	Felipe Contreras, Junio C Hamano
In-Reply-To: <1354239700-3325-1-git-send-email-chris@rorvick.com>

If the reference exists on the remote and it is not being removed, then
mark as an update.  This is in preparation for handling tags (lightweight
and annotated) exceptionally.

Signed-off-by: Chris Rorvick <chris@rorvick.com>
---
 cache.h  |  1 +
 remote.c | 18 +++++++++++-------
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/cache.h b/cache.h
index d72b64d..722321c 100644
--- a/cache.h
+++ b/cache.h
@@ -1003,6 +1003,7 @@ struct ref {
 		merge:1,
 		nonfastforward:1,
 		not_forwardable:1,
+		update:1,
 		deletion:1;
 	enum {
 		REF_STATUS_NONE = 0,
diff --git a/remote.c b/remote.c
index 5101683..07040b8 100644
--- a/remote.c
+++ b/remote.c
@@ -1326,15 +1326,19 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
 
 		ref->not_forwardable = !is_forwardable(ref);
 
-		ref->nonfastforward =
+		ref->update =
 			!ref->deletion &&
-			!is_null_sha1(ref->old_sha1) &&
-			(!has_sha1_file(ref->old_sha1)
-			  || !ref_newer(ref->new_sha1, ref->old_sha1));
+			!is_null_sha1(ref->old_sha1);
 
-		if (ref->nonfastforward && !ref->force && !force_update) {
-			ref->status = REF_STATUS_REJECT_NONFASTFORWARD;
-			continue;
+		if (ref->update) {
+			ref->nonfastforward =
+				!has_sha1_file(ref->old_sha1)
+				  || !ref_newer(ref->new_sha1, ref->old_sha1);
+
+			if (ref->nonfastforward && !ref->force && !force_update) {
+				ref->status = REF_STATUS_REJECT_NONFASTFORWARD;
+				continue;
+			}
 		}
 	}
 }
-- 
1.8.0.158.g0c4328c

^ permalink raw reply related

* [PATCH v6 4/8] push: flag updates that require force
From: Chris Rorvick @ 2012-11-30  1:41 UTC (permalink / raw)
  To: git
  Cc: Chris Rorvick, Angelo Borsotti, Drew Northup, Michael Haggerty,
	Philip Oakley, Johannes Sixt, Kacper Kornet, Jeff King,
	Felipe Contreras, Junio C Hamano
In-Reply-To: <1354239700-3325-1-git-send-email-chris@rorvick.com>

Add a flag for indicating an update to a reference requires force.
Currently the `nonfastforward` flag is used for this when generating the
status message.  A separate flag insulates dependent logic from the
details of set_ref_status_for_push().

Signed-off-by: Chris Rorvick <chris@rorvick.com>
---
 cache.h     |  4 +++-
 remote.c    | 11 ++++++++---
 transport.c |  2 +-
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/cache.h b/cache.h
index 722321c..b7ab4ac 100644
--- a/cache.h
+++ b/cache.h
@@ -999,7 +999,9 @@ struct ref {
 	unsigned char old_sha1[20];
 	unsigned char new_sha1[20];
 	char *symref;
-	unsigned int force:1,
+	unsigned int
+		force:1,
+		requires_force:1,
 		merge:1,
 		nonfastforward:1,
 		not_forwardable:1,
diff --git a/remote.c b/remote.c
index 07040b8..4a6f822 100644
--- a/remote.c
+++ b/remote.c
@@ -1293,6 +1293,8 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
 	struct ref *ref;
 
 	for (ref = remote_refs; ref; ref = ref->next) {
+		int force_ref_update = ref->force || force_update;
+
 		if (ref->peer_ref)
 			hashcpy(ref->new_sha1, ref->peer_ref->new_sha1);
 		else if (!send_mirror)
@@ -1335,9 +1337,12 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
 				!has_sha1_file(ref->old_sha1)
 				  || !ref_newer(ref->new_sha1, ref->old_sha1);
 
-			if (ref->nonfastforward && !ref->force && !force_update) {
-				ref->status = REF_STATUS_REJECT_NONFASTFORWARD;
-				continue;
+			if (ref->nonfastforward) {
+				ref->requires_force = 1;
+				if (!force_ref_update) {
+					ref->status = REF_STATUS_REJECT_NONFASTFORWARD;
+					continue;
+				}
 			}
 		}
 	}
diff --git a/transport.c b/transport.c
index bc31e8e..f3160b1 100644
--- a/transport.c
+++ b/transport.c
@@ -659,7 +659,7 @@ static void print_ok_ref_status(struct ref *ref, int porcelain)
 		const char *msg;
 
 		strcpy(quickref, status_abbrev(ref->old_sha1));
-		if (ref->nonfastforward) {
+		if (ref->requires_force) {
 			strcat(quickref, "...");
 			type = '+';
 			msg = "forced update";
-- 
1.8.0.158.g0c4328c

^ permalink raw reply related

* [PATCH v6 1/8] push: return reject reasons as a bitset
From: Chris Rorvick @ 2012-11-30  1:41 UTC (permalink / raw)
  To: git
  Cc: Chris Rorvick, Angelo Borsotti, Drew Northup, Michael Haggerty,
	Philip Oakley, Johannes Sixt, Kacper Kornet, Jeff King,
	Felipe Contreras, Junio C Hamano
In-Reply-To: <1354239700-3325-1-git-send-email-chris@rorvick.com>

Pass all rejection reasons back from transport_push().  The logic is
simpler and more flexible with regard to providing useful feedback.

Signed-off-by: Chris Rorvick <chris@rorvick.com>
---
 builtin/push.c      | 13 ++++---------
 builtin/send-pack.c |  4 ++--
 transport.c         | 17 ++++++++---------
 transport.h         |  9 +++++----
 4 files changed, 19 insertions(+), 24 deletions(-)

diff --git a/builtin/push.c b/builtin/push.c
index db9ba30..9d17fc7 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -244,7 +244,7 @@ static void advise_checkout_pull_push(void)
 static int push_with_options(struct transport *transport, int flags)
 {
 	int err;
-	int nonfastforward;
+	unsigned int reject_reasons;
 
 	transport_set_verbosity(transport, verbosity, progress);
 
@@ -257,7 +257,7 @@ static int push_with_options(struct transport *transport, int flags)
 	if (verbosity > 0)
 		fprintf(stderr, _("Pushing to %s\n"), transport->url);
 	err = transport_push(transport, refspec_nr, refspec, flags,
-			     &nonfastforward);
+			     &reject_reasons);
 	if (err != 0)
 		error(_("failed to push some refs to '%s'"), transport->url);
 
@@ -265,18 +265,13 @@ static int push_with_options(struct transport *transport, int flags)
 	if (!err)
 		return 0;
 
-	switch (nonfastforward) {
-	default:
-		break;
-	case NON_FF_HEAD:
+	if (reject_reasons & REJECT_NON_FF_HEAD) {
 		advise_pull_before_push();
-		break;
-	case NON_FF_OTHER:
+	} else if (reject_reasons & REJECT_NON_FF_OTHER) {
 		if (default_matching_used)
 			advise_use_upstream();
 		else
 			advise_checkout_pull_push();
-		break;
 	}
 
 	return 1;
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index d342013..9f98607 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -85,7 +85,7 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
 	int send_all = 0;
 	const char *receivepack = "git-receive-pack";
 	int flags;
-	int nonfastforward = 0;
+	unsigned int reject_reasons;
 	int progress = -1;
 
 	argv++;
@@ -223,7 +223,7 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
 	ret |= finish_connect(conn);
 
 	if (!helper_status)
-		transport_print_push_status(dest, remote_refs, args.verbose, 0, &nonfastforward);
+		transport_print_push_status(dest, remote_refs, args.verbose, 0, &reject_reasons);
 
 	if (!args.dry_run && remote) {
 		struct ref *ref;
diff --git a/transport.c b/transport.c
index 9932f40..d4568e7 100644
--- a/transport.c
+++ b/transport.c
@@ -714,7 +714,7 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i
 }
 
 void transport_print_push_status(const char *dest, struct ref *refs,
-				  int verbose, int porcelain, int *nonfastforward)
+				  int verbose, int porcelain, unsigned int *reject_reasons)
 {
 	struct ref *ref;
 	int n = 0;
@@ -733,18 +733,17 @@ void transport_print_push_status(const char *dest, struct ref *refs,
 		if (ref->status == REF_STATUS_OK)
 			n += print_one_push_status(ref, dest, n, porcelain);
 
-	*nonfastforward = 0;
+	*reject_reasons = 0;
 	for (ref = refs; ref; ref = ref->next) {
 		if (ref->status != REF_STATUS_NONE &&
 		    ref->status != REF_STATUS_UPTODATE &&
 		    ref->status != REF_STATUS_OK)
 			n += print_one_push_status(ref, dest, n, porcelain);
-		if (ref->status == REF_STATUS_REJECT_NONFASTFORWARD &&
-		    *nonfastforward != NON_FF_HEAD) {
+		if (ref->status == REF_STATUS_REJECT_NONFASTFORWARD) {
 			if (!strcmp(head, ref->name))
-				*nonfastforward = NON_FF_HEAD;
+				*reject_reasons |= REJECT_NON_FF_HEAD;
 			else
-				*nonfastforward = NON_FF_OTHER;
+				*reject_reasons |= REJECT_NON_FF_OTHER;
 		}
 	}
 }
@@ -1031,9 +1030,9 @@ static void die_with_unpushed_submodules(struct string_list *needs_pushing)
 
 int transport_push(struct transport *transport,
 		   int refspec_nr, const char **refspec, int flags,
-		   int *nonfastforward)
+		   unsigned int *reject_reasons)
 {
-	*nonfastforward = 0;
+	*reject_reasons = 0;
 	transport_verify_remote_names(refspec_nr, refspec);
 
 	if (transport->push) {
@@ -1099,7 +1098,7 @@ int transport_push(struct transport *transport,
 		if (!quiet || err)
 			transport_print_push_status(transport->url, remote_refs,
 					verbose | porcelain, porcelain,
-					nonfastforward);
+					reject_reasons);
 
 		if (flags & TRANSPORT_PUSH_SET_UPSTREAM)
 			set_upstreams(transport, remote_refs, pretend);
diff --git a/transport.h b/transport.h
index 4a61c0c..404b113 100644
--- a/transport.h
+++ b/transport.h
@@ -140,11 +140,12 @@ int transport_set_option(struct transport *transport, const char *name,
 void transport_set_verbosity(struct transport *transport, int verbosity,
 	int force_progress);
 
-#define NON_FF_HEAD 1
-#define NON_FF_OTHER 2
+#define REJECT_NON_FF_HEAD     0x01
+#define REJECT_NON_FF_OTHER    0x02
+
 int transport_push(struct transport *connection,
 		   int refspec_nr, const char **refspec, int flags,
-		   int * nonfastforward);
+		   unsigned int * reject_reasons);
 
 const struct ref *transport_get_remote_refs(struct transport *transport);
 
@@ -170,7 +171,7 @@ void transport_update_tracking_ref(struct remote *remote, struct ref *ref, int v
 int transport_refs_pushed(struct ref *ref);
 
 void transport_print_push_status(const char *dest, struct ref *refs,
-		  int verbose, int porcelain, int *nonfastforward);
+		  int verbose, int porcelain, unsigned int *reject_reasons);
 
 typedef void alternate_ref_fn(const struct ref *, void *);
 extern void for_each_alternate_ref(alternate_ref_fn, void *);
-- 
1.8.0.158.g0c4328c

^ permalink raw reply related

* [PATCH v6 2/8] push: add advice for rejected tag reference
From: Chris Rorvick @ 2012-11-30  1:41 UTC (permalink / raw)
  To: git
  Cc: Chris Rorvick, Angelo Borsotti, Drew Northup, Michael Haggerty,
	Philip Oakley, Johannes Sixt, Kacper Kornet, Jeff King,
	Felipe Contreras, Junio C Hamano
In-Reply-To: <1354239700-3325-1-git-send-email-chris@rorvick.com>

Advising the user to fetch and merge only makes sense if the rejected
reference is a branch.  If none of the rejections are for branches, just
tell the user the reference already exists.

Signed-off-by: Chris Rorvick <chris@rorvick.com>
---
 builtin/push.c | 11 +++++++++++
 cache.h        |  1 +
 remote.c       | 10 ++++++++++
 transport.c    |  2 ++
 transport.h    |  1 +
 5 files changed, 25 insertions(+)

diff --git a/builtin/push.c b/builtin/push.c
index 9d17fc7..e08485d 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -220,6 +220,10 @@ static const char message_advice_checkout_pull_push[] =
 	   "(e.g. 'git pull') before pushing again.\n"
 	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
 
+static const char message_advice_ref_already_exists[] =
+	N_("Updates were rejected because the destination reference already exists\n"
+	   "in the remote and the update is not a fast-forward.");
+
 static void advise_pull_before_push(void)
 {
 	if (!advice_push_non_ff_current || !advice_push_nonfastforward)
@@ -241,6 +245,11 @@ static void advise_checkout_pull_push(void)
 	advise(_(message_advice_checkout_pull_push));
 }
 
+static void advise_ref_already_exists(void)
+{
+	advise(_(message_advice_ref_already_exists));
+}
+
 static int push_with_options(struct transport *transport, int flags)
 {
 	int err;
@@ -272,6 +281,8 @@ static int push_with_options(struct transport *transport, int flags)
 			advise_use_upstream();
 		else
 			advise_checkout_pull_push();
+	} else if (reject_reasons & REJECT_ALREADY_EXISTS) {
+		advise_ref_already_exists();
 	}
 
 	return 1;
diff --git a/cache.h b/cache.h
index dbd8018..d72b64d 100644
--- a/cache.h
+++ b/cache.h
@@ -1002,6 +1002,7 @@ struct ref {
 	unsigned int force:1,
 		merge:1,
 		nonfastforward:1,
+		not_forwardable:1,
 		deletion:1;
 	enum {
 		REF_STATUS_NONE = 0,
diff --git a/remote.c b/remote.c
index 04fd9ea..5101683 100644
--- a/remote.c
+++ b/remote.c
@@ -1279,6 +1279,14 @@ int match_push_refs(struct ref *src, struct ref **dst,
 	return 0;
 }
 
+static inline int is_forwardable(struct ref* ref)
+{
+	if (!prefixcmp(ref->name, "refs/tags/"))
+		return 0;
+
+	return 1;
+}
+
 void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
 	int force_update)
 {
@@ -1316,6 +1324,8 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
 		 *     always allowed.
 		 */
 
+		ref->not_forwardable = !is_forwardable(ref);
+
 		ref->nonfastforward =
 			!ref->deletion &&
 			!is_null_sha1(ref->old_sha1) &&
diff --git a/transport.c b/transport.c
index d4568e7..bc31e8e 100644
--- a/transport.c
+++ b/transport.c
@@ -740,6 +740,8 @@ void transport_print_push_status(const char *dest, struct ref *refs,
 		    ref->status != REF_STATUS_OK)
 			n += print_one_push_status(ref, dest, n, porcelain);
 		if (ref->status == REF_STATUS_REJECT_NONFASTFORWARD) {
+			if (ref->not_forwardable)
+				*reject_reasons |= REJECT_ALREADY_EXISTS;
 			if (!strcmp(head, ref->name))
 				*reject_reasons |= REJECT_NON_FF_HEAD;
 			else
diff --git a/transport.h b/transport.h
index 404b113..bfd2df5 100644
--- a/transport.h
+++ b/transport.h
@@ -142,6 +142,7 @@ void transport_set_verbosity(struct transport *transport, int verbosity,
 
 #define REJECT_NON_FF_HEAD     0x01
 #define REJECT_NON_FF_OTHER    0x02
+#define REJECT_ALREADY_EXISTS  0x04
 
 int transport_push(struct transport *connection,
 		   int refspec_nr, const char **refspec, int flags,
-- 
1.8.0.158.g0c4328c

^ permalink raw reply related

* [PATCH v6 0/8] push: update remote tags only with force
From: Chris Rorvick @ 2012-11-30  1:41 UTC (permalink / raw)
  To: git
  Cc: Chris Rorvick, Angelo Borsotti, Drew Northup, Michael Haggerty,
	Philip Oakley, Johannes Sixt, Kacper Kornet, Jeff King,
	Felipe Contreras, Junio C Hamano

This patch series originated in response to the following thread:

  http://thread.gmane.org/gmane.comp.version-control.git/208354

I made some adjustments based on Junio's last round of feedback
including a new patch reworking the "push rules" comment in remote.c.
Also refined some of the log messages--nothing major.  Finally, took a
stab at putting something together for the release notes, see below.

Chris

Release notes:

"git push" no longer updates tags (lightweight or annotated) by default.
Specifically, if the destination reference already exists and is under
refs/tags/ or it points to a tag object, it is not allowed to fast-
forward (unless forced using +A:B notation or by passing --force.)  This
is consistent with how a tag is normally thought of: a reference that
does not move once defined.  It also ensures a push will not
inadvertently clobber an already existing tag--something that can go
unnoticed if fast-forwarding is allowed.

Chris Rorvick (8):
  push: return reject reasons as a bitset
  push: add advice for rejected tag reference
  push: flag updates
  push: flag updates that require force
  push: require force for refs under refs/tags/
  push: require force for annotated tags
  push: clarify rejection of update to non-commit-ish
  push: cleanup push rules comment

 Documentation/git-push.txt |  9 ++---
 builtin/push.c             | 24 +++++++++-----
 builtin/send-pack.c        |  9 +++--
 cache.h                    |  7 +++-
 remote.c                   | 83 +++++++++++++++++++++++++++++++++++-----------
 send-pack.c                |  1 +
 t/t5516-fetch-push.sh      | 44 +++++++++++++++++++++++-
 transport-helper.c         |  6 ++++
 transport.c                | 25 ++++++++------
 transport.h                | 10 +++---
 10 files changed, 167 insertions(+), 51 deletions(-)

-- 
1.8.0.158.g0c4328c

^ permalink raw reply

* Re: [PATCH] cache-tree: invalidate i-t-a paths after writing trees
From: Nguyen Thai Ngoc Duy @ 2012-11-30  1:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jeff King, Jonathon Mah
In-Reply-To: <7vhao8neck.fsf@alter.siamese.dyndns.org>

On Fri, Nov 30, 2012 at 7:06 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>
>>> An alternative might be to add a "phoney" bit next to "used" in the
>>> cache_tree structure, mark the cache tree as phoney when we skip an
>>> entry marked as CE_REMOVE or CE_ITA, and make the postprocessing
>>> loop this patch adds aware of that bit, instead of iterating over
>>> the index entries; instead, it would recurse the resulting cache
>>> tree and invalidate parts of the tree that have subtrees with the
>>> "phoney" bit set, or something.
>>
>> Yeah, that sounds better.
>
> Did anything happen to this topic after this?

Not from my side because I forgot to mark this thread as a todo item
and unsurprisingly forgot about it.
-- 
Duy

^ permalink raw reply

* Re: [PATCH] gitweb: add readme to overview page
From: Xypron @ 2012-11-30  1:11 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano
In-Reply-To: <50B7E1FD.8060001@gmx.de>

The following setting provides the same feature

# html text to include at home page
$home_text = "indextext.html";

Sorry for the noise.

Best regards

Heinrich Schuchardt

On 29.11.2012 23:30, Xypron wrote:
> Hello Junio,
> 
> thank you for your comment in message
> <7vip9ak971.fsf@alter.siamese.dyndns.org>
> that message <1352652039-31453-1-git-send-email-xypron.glpk@gmx.de>
> lost the thread context.
> 
> As already described I would be happy if a README.html could be added to
> the overview page of gitweb.
> 
> Please, find below an updated patch. Compared to the first version of my
> patch it avoids a warning concerning doubled slashes in filenames and adds
> a subtitle "projects" between the README and the project list.
> 
> Best regards
> 
> Heinrich Schuchardt
> 
> Subject: [PATCH] gitweb: add readme to overview page
> 
> For repositories it is possible to maintain a README.html which will
> be shown on the summary page. This is not possible for the server
> root.
> 
> German law requires to provide contact data on the web server. This
> data could easily be entered in the overview page using a README.html.
> 
> Furthermore it is possible to put the repositories not directly into
> the root directory but into a subdirectory. Here also a README.html
> would be helpful to indicate what the subdirectory is about.
> 
> The patch introduces README.html functionality for the root directory
> and all subdirectories.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  gitweb/gitweb.perl |   13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index e8812fa..618b0d8 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -6368,6 +6368,19 @@ sub git_project_list {
>  	}
>  
>  	git_project_search_form($searchtext, $search_use_regexp);
> +	# If XSS prevention is on, we don't include README.html.
> +	# TODO: Allow a readme in some safe format.
> +	my $path = "";
> +	if (defined $project_filter) {
> +		$path = "/$project_filter";
> +	}
> +	if (!$prevent_xss && -s "$projectroot$path/README.html") {
> +		print "<div class=\"title\">readme</div>\n" .
> +		"<div class=\"readme\">\n";
> +		insert_file("$projectroot$path/README.html");
> +		print "\n</div>\n"; # class="readme"
> +	}
> +	print "<div class=\"title\">projects</div>\n";
>  	git_project_list_body(\@list, $order);
>  	git_footer_html();
>  }

^ permalink raw reply

* Re: [PATCH v5 0/2] submodule update: add --remote for submodule's upstream changes
From: Phil Hord @ 2012-11-30  1:11 UTC (permalink / raw)
  To: W. Trevor King
  Cc: Git, Junio C Hamano, Heiko Voigt, Jeff King, Shawn Pearce,
	Jens Lehmann, Nahor
In-Reply-To: <20121129191326.GC27409@odin.tremily.us>

On Thu, Nov 29, 2012 at 2:13 PM, W. Trevor King <wking@tremily.us> wrote:
>
> On Thu, Nov 29, 2012 at 01:29:12PM -0500, Phil Hord wrote:
>> On Fri, Nov 23, 2012 at 12:54 PM, W. Trevor King <wking@tremily.us> wrote:
>> > [snip initial thoughts leading to the update --remote v5]
>>
>> I was thinking the same thing, but reading this whole thread a couple of
>> weeks late.  Thanks for noticing.
>>
>> Moreover, I think that 'git submodule update --pull' is also the wrong way
>> to spell this action.   Maybe you are misled from the outset by your
>> current workflow:
>
> Did you see my v5 (add --remote) series?

Eventually, I did.  Sorry for the out-of-order replies.


>> For that reason, I don't like the --pull switch since it implies a
>> fetch, but I will not always want to do a fetch.
>
>   $ git submodule update --remote --no-fetch
>
> will not fetch the submodule remotes.

This seems precisely backwards to me. Why not use

  $ git submodule update --remote --fetch

to do your "default" behavior instead?   I suppose I am arguing
against the tide of the dominant workflow, but the fetch-by-default
idea needlessly conflates two primitive operations:  "float" and
"fetch".

>> I don't know which remote I should be tracking, though.  I suppose
>> it is 'origin' for now, but maybe it is just whatever
>> $superproject's HEAD's remote-tracking branch indicates.
>
> With the --remote series, I always use "origin" because that's what
> `submodule add` should be setting up.  If people want to change that
> up by hand, we may need a submodule.<name>.remote configuration
> option.

I've always felt that the "origin" defaults are broken and are simply
being ignored because most users do not trip over them.  But ISTR that
submodule commands use the remote indicated by the superproject's
current remote-tracking configuration, with a fallback to 'origin' if
there is none.  Sort of a "best effort" algorithm, I think.  Am I
remembering that wrong?


>> I am not sure I want the gitlinks in superproject to update automatically
>> in the index, but I definitely do not want to automatically create a commit
>> for them.
>
> Commits are dissabled by default (see my recent --commit RFC for how
> they would be enabled).
>
>> But I really don't want to figure out how to handle submodule
>> collisions during a merge (or rebase!) of my superproject with changes that
>> someone else auto-committed in his local $superproject as he and I
>> arbitrarily floated up the upstream independently.  There is nothing but
>> loathing down that path.
>
> This is true.  I'm not sure how gitlink collisions are currently
> handled…


They've always been trouble for me.  But it may be that I am ignorant.

Phil

^ permalink raw reply

* Re: [PATCH] cache-tree: invalidate i-t-a paths after writing trees
From: Junio C Hamano @ 2012-11-30  0:06 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: git, Jeff King, Jonathon Mah
In-Reply-To: <CACsJy8DEwpg0gY1o6gSB747W5fAYYxz97e-qnkQthSut3B7Eag@mail.gmail.com>

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

>> An alternative might be to add a "phoney" bit next to "used" in the
>> cache_tree structure, mark the cache tree as phoney when we skip an
>> entry marked as CE_REMOVE or CE_ITA, and make the postprocessing
>> loop this patch adds aware of that bit, instead of iterating over
>> the index entries; instead, it would recurse the resulting cache
>> tree and invalidate parts of the tree that have subtrees with the
>> "phoney" bit set, or something.
>
> Yeah, that sounds better.

Did anything happen to this topic after this?

^ permalink raw reply

* Re: [PATCH 6/8] imap-send: change msg_data from storing (char *, len) to storing strbuf
From: Jeff King @ 2012-11-29 23:43 UTC (permalink / raw)
  To: Michael Haggerty; +Cc: Junio C Hamano, git, Jeremy White, Johannes Schindelin
In-Reply-To: <7vboegp04x.fsf@alter.siamese.dyndns.org>

On Thu, Nov 29, 2012 at 01:30:54PM -0800, Junio C Hamano wrote:

> > For some reason, there is a bunch of infrastructure in this file for
> > dealing with IMAP flags, although there is nothing in the code that
> > actually allows any flags to be set.  If there is no plan to add
> > support for flags in the future, a bunch of code could be ripped out
> > and "struct msg_data" could be completely replaced with strbuf.
> 
> Yeah, after all these years we have kept the unused flags field
> there and nobody needed anything out of it.  I am OK with a removal
> if it is done at the very end of the series.

There's a bunch of unused junk in imap-send. The original implementation
copied a bunch of code from isync, a much more full-featured imap
client, and the result ended up way more complex than it needed to be. I
have ripped a few things out over the years when they cause a problem
(e.g., portability of /dev/urandom, conflict over the name "struct
string_list"), but have mostly let it be out of a vague sense that we
might one day want to pull bugfixes from isync upstream.

That has not happened once in the last six years, though, and I would
doubt that a straightforward merge would work after so many years. So
ripping out and refactoring the code in the name of maintainability is
probably a good thing at this point.

-Peff

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox