git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Luciano Joublanc <ljoublanc@dinogroup.eu>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: Bad refspec messes up bundle.
Date: Sat, 31 Mar 2018 09:50:17 +0100	[thread overview]
Message-ID: <CAO+-ZX-DvjsOnpvfPuLkx2w2cR5FDb6Ww8xEyuZHMmC57=b2yQ@mail.gmail.com> (raw)
In-Reply-To: <nycvar.QRO.7.76.6.1803301102430.5026@qfpub.tvgsbejvaqbjf.bet>

Hi Johannes,

With such a comprehensive reply, I would feel guilty not making a
contribution now :) Be forewarned though, It's been about ten years
since I wrote anything in `C`!

I've cloned the `maint` branch, built the project, and added the test
as you suggested - it's failing as expected.

I'm somewhat confused though. I think it's m limited understanding of
'ref' and 'commit'.

On 30 March 2018 at 11:20, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
>
>
>   However, this would be incorrect, as the flags are stored with the
>   *commit*, not with the ref. So if two refs point to the same commit,
>   that new code would skip the second one by mistake!


Isn't that the point here? to deduplicate commits?  My limited
understanding is that at a 'ref' is like an alias or pointer to a
commit.

>
>   By the way, this makes me think that there is another very real bug in
>   the bundle code, in the part I showed above. Suppose you have a `master`
>   and a `next` ref, and both point at the same commit, then you would want
>   `git bundle create next.bundle master..next` to list `next`, don't you
>   think?


Doesn't this contradict what you just said, that we don't want to skip
refs with the same commit #?

In fact, if you look in the calling function, there is a
`    object_array_remove_duplicates(&revs.pending);`
Which to the best of my understanding removes duplicate refs (not
commits). However, I suspect this doesn't cover the `--all` case as
it's a switch rather than a revspec? Would that be right?

>
>
> - most likely, the best way to avoid duplicate refs entries is to use the
>   actual ref name and put it into a hash set. Luckily, we do have code
>   for this, and examples how to use it, too. See e.g. fc65b00da7e
>   (merge-recursive: change current file dir string_lists to hashmap,
>   2017-09-07). So you would define something like
>

Separately, if I do end up including the hashmap code, it should be
refactored out into it's own file, right?

Thanks again,

Luciano

-- 
This message is intended only for the personal and confidential use of the 
designated recipient(s) named above.  If you are not the intended recipient 
of this message you are hereby notified that any review, dissemination, 
distribution or copying of this message is strictly prohibited.  This 
communication is for information purposes only and should not be regarded 
as an offer to sell or as a solicitation of an offer to buy any financial 
product, an official confirmation of any transaction, or as an official 
statement of the Dinosaur Group.  Email transmission cannot be guaranteed 
to be secure or error-free.  Therefore, we do not represent that this 
information is complete or accurate and it should not be relied upon as 
such.  All information is subject to change without notice.

  parent reply	other threads:[~2018-03-31  8:50 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-19  8:39 Bad refspec messes up bundle Luciano Joublanc
2018-03-19 17:36 ` Junio C Hamano
2018-03-30 10:20   ` Johannes Schindelin
2018-03-30 17:18     ` Junio C Hamano
2018-03-30 18:58       ` Johannes Schindelin
2018-03-31  8:50     ` Luciano Joublanc [this message]
2018-04-03 14:38       ` Johannes Schindelin

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='CAO+-ZX-DvjsOnpvfPuLkx2w2cR5FDb6Ww8xEyuZHMmC57=b2yQ@mail.gmail.com' \
    --to=ljoublanc@dinogroup.eu \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /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).