git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Michael Haggerty <mhagger@alum.mit.edu>
Cc: git@vger.kernel.org, "Carlos Martín Nieto" <cmn@elego.de>,
	"Michael Schubert" <mschub@elegosoft.com>,
	"Johan Herland" <johan@herland.net>, "Jeff King" <peff@peff.net>,
	"Marc Branchaud" <marcnarc@xiplink.com>,
	"Nicolas Pitre" <nico@fluxnic.net>,
	"John Szakmeister" <john@szakmeister.net>
Subject: Re: [PATCH 10/15] fetch --tags: fetch tags *in addition to* other stuff
Date: Mon, 28 Oct 2013 12:10:04 -0700	[thread overview]
Message-ID: <xmqqa9htfbzn.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: 526A896D.7050801@alum.mit.edu

Michael Haggerty <mhagger@alum.mit.edu> writes:

>> True but when fetching other references, tags relevant to the
>> history being fetched by default should automatically follow, so the
>> above explains why "fetch --tags" is not a useful thing to do daily.
>
> Maybe not necessary in many scenarios, but is it harmful for the common
> case of cloning from and then periodically fetching from an upstream?

There is no mention of "harmful"; I only said "not useful". And it
comes primarily from knowing why "--tags" was introduced in the
first place; I should have said "less useful than before, ever since
we started to reliably auto-follow".

The only "harmful" part is its interaction with "--prune", which
your series nicely addresses.

> ISTM that the result of the declarative --tags option
>
>     we have all upstream tags
>
> is easier to reason about than the history-dependent tag-following behavior

I'd say "we have all the relevant tags" and "we have all the tags
the other side has" are equally valid things to wish for, depending
who you are and how your project is organized, and one is not
necessarily easy/useful than the other.  In case it was unclear, I
was not seriously advocating to deprecate/remove "--tags".

> Regarding your first point: if it matters which of the duplicates is
> chosen, then it can only matter because they differ in some other way
> than their reference names, for example in their flags.  So a robust way
> of de-duping them (if it is possible) would be to compare the two
> records and decide which one should take precedence based on this other
> information rather than based on which one happened to come earlier in
> the list.  Then the list order would be immaterial and (for example) it
> wouldn't be a problem to reorder the items.

But that changes the behaviour to those who has cared to rely on the
ordering.  With the change to first collect and then sort unstably
before deduping, the series already tells them not to rely on the
order, and two of us tentatively agreed in this discussion that it
is not likely to matter.  If later this agreement turns out to be a
mistake and there are people who *do* rely on the ordering, the only
acceptable fix for them is by making sure we restore the "first one
trumps" semantics, not by defining an alternative, arguably better,
behaviour---that is not a regression fix.

In any case, thanks for working on this topic.

  reply	other threads:[~2013-10-28 19:10 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-13  2:54 Local tag killer Michael Haggerty
2013-09-13  4:03 ` Junio C Hamano
2013-09-20 22:51   ` Junio C Hamano
2013-09-21  6:42     ` Michael Haggerty
2013-09-21 12:28       ` John Szakmeister
2013-09-24  7:51       ` Jeff King
2013-09-24 13:22         ` Marc Branchaud
2013-09-25  8:22           ` Jeff King
2013-09-25 22:54         ` Nicolas Pitre
2013-09-28 12:20           ` Michael Haggerty
2013-09-28 21:42             ` Johan Herland
2013-09-29  4:29               ` Michael Haggerty
2013-09-29  9:30                 ` Johan Herland
2013-09-30 15:24                 ` Marc Branchaud
2013-09-30 15:52                   ` Nicolas Pitre
2013-09-30 19:16                     ` Marc Branchaud
2013-09-30 20:08                       ` Nicolas Pitre
2013-09-30 21:14                         ` Marc Branchaud
2013-09-30 22:44                           ` Nicolas Pitre
2013-09-30 23:18                             ` Jeff King
2013-10-01  3:04                             ` Marc Branchaud
2013-10-01  3:28                               ` Nicolas Pitre
2013-10-01 12:45                                 ` Marc Branchaud
2013-10-23 15:50 ` [PATCH 00/15] Change semantics of "fetch --tags" Michael Haggerty
2013-10-23 15:50   ` [PATCH 01/15] t5510: use the correct tag name in test Michael Haggerty
2013-10-23 15:50   ` [PATCH 02/15] t5510: prepare test refs more straightforwardly Michael Haggerty
2013-10-23 18:36     ` Junio C Hamano
2013-10-24  6:49       ` Michael Haggerty
2013-10-24 19:50         ` Junio C Hamano
2013-10-23 15:50   ` [PATCH 03/15] t5510: check that "git fetch --prune --tags" does not prune branches Michael Haggerty
2013-10-23 15:50   ` [PATCH 04/15] api-remote.txt: correct section "struct refspect" Michael Haggerty
2013-10-23 18:43     ` Junio C Hamano
2013-10-24  7:06       ` Michael Haggerty
2013-10-23 15:50   ` [PATCH 05/15] get_ref_map(): rename local variables Michael Haggerty
2013-10-23 18:45     ` Junio C Hamano
2013-10-24  7:24       ` Michael Haggerty
2013-10-23 15:50   ` [PATCH 06/15] ref_remove_duplicates(): avoid redundant bisection Michael Haggerty
2013-10-23 15:50   ` [PATCH 07/15] ref_remove_duplicates(): simplify function Michael Haggerty
2013-10-23 15:50   ` [PATCH 08/15] ref_remove_duplicates(): improve documentation comment Michael Haggerty
2013-10-23 18:47     ` Junio C Hamano
2013-10-23 15:50   ` [PATCH 09/15] builtin/fetch.c: reorder function definitions Michael Haggerty
2013-10-23 15:50   ` [PATCH 10/15] fetch --tags: fetch tags *in addition to* other stuff Michael Haggerty
2013-10-24 20:51     ` Junio C Hamano
2013-10-25 15:08       ` Michael Haggerty
2013-10-28 19:10         ` Junio C Hamano [this message]
2013-10-30  4:26           ` Michael Haggerty
2013-10-26  5:10       ` Michael Haggerty
2013-10-23 15:50   ` [PATCH 11/15] fetch --prune: prune only based on explicit refspecs Michael Haggerty
2013-10-24 21:11     ` Junio C Hamano
2013-10-26  6:49       ` Michael Haggerty
2013-10-28 15:08         ` Junio C Hamano
2013-10-23 15:50   ` [PATCH 12/15] query_refspecs(): move some constants out of the loop Michael Haggerty
2013-10-23 15:50   ` [PATCH 13/15] builtin/remote.c: reorder function definitions Michael Haggerty
2013-10-23 15:50   ` [PATCH 14/15] builtin/remote.c:update(): use struct argv_array Michael Haggerty
2013-10-23 15:50   ` [PATCH 15/15] fetch, remote: properly convey --no-prune options to subprocesses Michael Haggerty
2013-10-24 21:17     ` Junio C Hamano
2013-10-23 16:59   ` [PATCH 00/15] Change semantics of "fetch --tags" Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqa9htfbzn.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=cmn@elego.de \
    --cc=git@vger.kernel.org \
    --cc=johan@herland.net \
    --cc=john@szakmeister.net \
    --cc=marcnarc@xiplink.com \
    --cc=mhagger@alum.mit.edu \
    --cc=mschub@elegosoft.com \
    --cc=nico@fluxnic.net \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).