Linus Torvalds writes:
> On Fri, Sep 30, 2016 at 10:54 AM, Linus Torvalds
> wrote:
>>
>>> So IMHO, the best combination is the init_default_abbrev() you posted in
>>> [1], but initialized at the top of find_unique_abbrev(). And cached
>>> there, obviously, in a similar way.
>>
>> That's certainl
On Fri, Sep 30, 2016 at 3:01 AM, Linus Torvalds
wrote:
> On Thu, Sep 29, 2016 at 5:56 PM, Mike Hommey wrote:
>>
>> OTOH, how often does one refer to trees or blobs with abbreviated sha1s?
>> Most of the time, you'd use abbreviated sha1s for commits. And the number
>> of commits in git and the ker
Linus Torvalds writes:
> Considering that TRANSPORT_SUMMARY and TRANSPORT_SUMMARY_WIDTH are
> both used in exactly one place each, I'd suggest getting rid of that
> crazy macro, and just expanding it in those places to avoid these
> kinds of crazy "hiding variables inside complex defines thning".
On Fri, Sep 30, 2016 at 11:40 AM, Junio C Hamano wrote:
>
> There is another instance buried deep in an obscure macro. A
> minimum fix may look like this, but I really hope somebody else
> finds a better approach.
Heh. Yeah, that's just ugly. I assume this is why the odd git fetch
pretty-printin
Junio C Hamano writes:
> From: Junio C Hamano
> Date: Thu, 29 Sep 2016 21:19:20 -0700
> Subject: [PATCH] abbrev: adjust to the new world order
>
> The default_abbrev used to be a concrete value usable as the default
> abbreviation length. The code that sets custom abbreviation length,
> in resp
On Fri, Sep 30, 2016 at 10:54 AM, Linus Torvalds
wrote:
>
>> So IMHO, the best combination is the init_default_abbrev() you posted in
>> [1], but initialized at the top of find_unique_abbrev(). And cached
>> there, obviously, in a similar way.
>
> That's certainly possible, but I'm really not happ
On Fri, Sep 30, 2016 at 10:54:16AM -0700, Linus Torvalds wrote:
> On Fri, Sep 30, 2016 at 1:06 AM, Jeff King wrote:
> >
> > I agree that this deals with the performance concerns by caching the
> > default_abbrev_len and starting there. I still think it's unnecessarily
> > invasive to touch get_sh
Jeff King writes:
> I agree that this deals with the performance concerns by caching the
> default_abbrev_len and starting there. I still think it's unnecessarily
> invasive to touch get_short_sha1() at all, which is otherwise only a
> reading function.
>
> So IMHO, the best combination is the in
On Fri, Sep 30, 2016 at 1:06 AM, Jeff King wrote:
>
> I agree that this deals with the performance concerns by caching the
> default_abbrev_len and starting there. I still think it's unnecessarily
> invasive to touch get_short_sha1() at all, which is otherwise only a
> reading function.
So the re
On Thu, Sep 29, 2016 at 06:18:03PM -0700, Linus Torvalds wrote:
> On Thu, Sep 29, 2016 at 5:57 PM, Linus Torvalds
> wrote:
> >
> > Actually, all the other cases seem to be "parse a SHA1 with a known
> > length", so they really don't have a negative length. So this seems
> > ok, and is easier to
On Thu, Sep 29, 2016 at 04:13:38PM -0700, Junio C Hamano wrote:
> There are very early ones in the program startup sequence in the
> following functions, but I do not think of a reason why our new and
> early call to prepare_packed_git() might be problematic, given that
> all of them require us to
Junio C Hamano writes:
> Junio C Hamano writes:
>
>> There still are breakages seen in t5510 and t5526 that are about the
>> verbose output of "git fetch". I'll stop digging at this point
>> tonight, and welcome others who look into it ;-)
>
> OK, just before I leave the keyboard for the night.
On Thu, Sep 29, 2016 at 9:18 PM, Linus Torvalds
wrote:
>
> There are probably other things like that.
t5510-fetch.sh fails oddly, looks like the output is off by one character.
not ok 77 - fetch aligned output
It has a magic "cut -c 22-" that expects the output at a specific
place, and now i
Junio C Hamano writes:
> There still are breakages seen in t5510 and t5526 that are about the
> verbose output of "git fetch". I'll stop digging at this point
> tonight, and welcome others who look into it ;-)
OK, just before I leave the keyboard for the night...
-- >8 --
From: Junio C Hamano
On Thu, Sep 29, 2016 at 9:10 PM, Junio C Hamano wrote:
>
> A quick and dirty fix for it may look like this.
Crossed emails.
Indeed, I just solved the builtin/rev-parse.c thing slightly differently.
And you found another failure in the diff code similarly not liking
the negative DEFAULT_ABBREV.
On Thu, Sep 29, 2016 at 8:54 PM, Junio C Hamano wrote:
>
> * The script uses "git rev-parse --short HEAD"; I suspect that it
>says "ah, default_abbrev is -1 and minimum_abbrev is 4, so let's
>try abbreviating to 4 hexdigits".
Ahh, right you are. The logic there is
Junio C Hamano writes:
> Linus Torvalds writes:
>
>> So this patch may actually be "production ready" apart from the fact
>> that some tests still fail (at least t2027-worktree-list.sh) because
>> of different short SHA1 cases.
>
> t2027 has at least two problems.
>
> * "git worktree" does not
Linus Torvalds writes:
> So this patch may actually be "production ready" apart from the fact
> that some tests still fail (at least t2027-worktree-list.sh) because
> of different short SHA1 cases.
t2027 has at least two problems.
* "git worktree" does not read the core.abbrev configuration,
On Thu, Sep 29, 2016 at 12:06:23PM -0700, Linus Torvalds wrote:
> On Thu, Sep 29, 2016 at 11:55 AM, Linus Torvalds
> wrote:
> >
> > For the kernel, just the *math* right now actually gives 12
> > characters. For current git it actually seems to say that 8 is the
> > correct number. For small proje
On Thu, Sep 29, 2016 at 5:57 PM, Linus Torvalds
wrote:
>
> Actually, all the other cases seem to be "parse a SHA1 with a known
> length", so they really don't have a negative length. So this seems
> ok, and is easier to verify than the "what all contexts might use
> DEFAULT_ABBREV" thing. There's
On Thu, Sep 29, 2016 at 5:56 PM, Mike Hommey wrote:
>
> OTOH, how often does one refer to trees or blobs with abbreviated sha1s?
> Most of the time, you'd use abbreviated sha1s for commits. And the number
> of commits in git and the kernel repositories are much lower than the
> number of overall o
On Thu, Sep 29, 2016 at 5:28 PM, Linus Torvalds
wrote:
>
> To be fair, my original patch had a different worry that I didn't
> bother with: what if one of the _other_ callers of "get_short_sha1()"
> passed in -1 to it. I only handled the -1 case in th eone path care
> about in that first RFC for
On Thu, Sep 29, 2016 at 5:20 PM, Linus Torvalds
wrote:
>
> As you say, my original patch had neither of those issues.
To be fair, my original patch had a different worry that I didn't
bother with: what if one of the _other_ callers of "get_short_sha1()"
passed in -1 to it. I only handled the -1
On Thu, Sep 29, 2016 at 4:13 PM, Junio C Hamano wrote:
>
> One thing that worries me is if we are ready to start accessing the
> object store in all codepaths when we ask for DEFAULT_ABBREV.
Yes. That was my main worry too. I also looked at just doing an explicit
if (abbrev_commit && defaul
Junio C Hamano writes:
> Linus Torvalds writes:
>
>> The advantage of the
>> previous patch was that it got the object counting right almost
>> automatically, this actually has its own new object counting code and
>> maybe I screwed it up.
I guess another advantage of your original approach was
Linus Torvalds writes:
> Somebody should really double-check my heuristics, to see that I did
> the pack counting etc right. It doesn't do alternate loose file
> counting at all, and maybe it could matter. The advantage of the
> previous patch was that it got the object counting right almost
>
On Thu, Sep 29, 2016 at 12:45 PM, Junio C Hamano wrote:
>
> I think that is a reasonable way to go.
>
> #define DEFAULT_ABBREV get_default_abbrev()
>
> would help.
So something like this that replaces the previous patch?
Somebody should really double-check my heuristics, to see that I did
the pa
Linus Torvalds writes:
> But you could easily also just instead have it do something like
>
> if (default_abbrev < 0)
> default_abbrev = initialize_abbrev();
>
> at startup time if "abbrev_commit" is set, and just do it once and for
> all rather rthan the odd loping behavior.
I
Linus Torvalds writes:
> On Thu, Sep 29, 2016 at 11:55 AM, Linus Torvalds
> wrote:
>>
>> For the kernel, just the *math* right now actually gives 12
>> characters. For current git it actually seems to say that 8 is the
>> correct number. For small projects, you'll still see 7.
>
> Sorry, the git
On Thu, Sep 29, 2016 at 12:16 PM, Jeff King wrote:
>
> Hmm. So at length 7, we expect collisions at 2^14, which is 16384. That
> seems really low. I mean, by the birthday paradox that's where expect
> a 50% chance of a collision. But that's a single collision. We
> definitely don't expect them to
On Thu, Sep 29, 2016 at 11:55:46AM -0700, Linus Torvalds wrote:
> I think the patch can speak for itself, but the basic core is this
> section in get_short_sha1():
>
> + if (len < 16 && !status && (flags & GET_SHA1_AUTOMATIC)) {
> + unsigned int expect_collision = 1 << (le
On Thu, Sep 29, 2016 at 11:55 AM, Linus Torvalds
wrote:
>
> For the kernel, just the *math* right now actually gives 12
> characters. For current git it actually seems to say that 8 is the
> correct number. For small projects, you'll still see 7.
Sorry, the git number is 9, not 8. The reason is t
On Thu, Sep 29, 2016 at 11:37 AM, Linus Torvalds
wrote:
>
> I'm playing with an early patch to make the default more dynamic.
> Let's see how well it works in practice, but it looks fairly
> promising. Let me test a bit more and send out an RFC patch..
Ok, this is *very* rough, and it doesn't act
On Thu, Sep 29, 2016 at 11:05 AM, Junio C Hamano wrote:
>
> Yes, "git log --oneline" looks somewhat different and strange for
> me, too ;-)
I'm playing with an early patch to make the default more dynamic.
Let's see how well it works in practice, but it looks fairly
promising. Let me test a bit m
Johannes Sixt writes:
> Am 29.09.2016 um 01:30 schrieb Junio C Hamano:
>> As Peff said, responding in a thread started by Linus's suggestion
>> to raise the default abbreviation to 12 hexdigits:
>
> This is waayy too large for a new default. The vast majority of
> repositories is smallish. For th
Quoting Jeff King :
On Thu, Sep 29, 2016 at 04:44:00AM +0200, SZEDER Gábor wrote:
> So 12 seems reasonable, and the only downside for it (or for "13", for
> that matter) is a few extra bytes. I dunno, maybe people will really
> hate that, but I have a feeling these are mostly
cu
Jeff King writes:
> On Thu, Sep 29, 2016 at 04:44:00AM +0200, SZEDER Gábor wrote:
>
>> > So 12 seems reasonable, and the only downside for it (or for "13", for
>> > that matter) is a few extra bytes. I dunno, maybe people will really
>> > hate that, but I have a feeling these are most
On Wed, Sep 28, 2016 at 04:30:47PM -0700, Junio C Hamano wrote:
> As Peff said, responding in a thread started by Linus's suggestion
> to raise the default abbreviation to 12 hexdigits:
>
> I actually think "12" might be sane for a long time. That's 48 bits of
> sha1, so we'd expect a 50%
On Thu, Sep 29, 2016 at 07:27:01AM +0200, Lukas Fleischer wrote:
> > Sure, users working on smaller repos are free to reset core.abbrev to
> > its original value. I don't have any numbers, of course, but I
> > suspect that there are many more smaller repos out there that this
> > change will affe
On Thu, Sep 29, 2016 at 04:44:00AM +0200, SZEDER Gábor wrote:
> > So 12 seems reasonable, and the only downside for it (or for "13", for
> > that matter) is a few extra bytes. I dunno, maybe people will really
> > hate that, but I have a feeling these are mostly cut-and-pasted anyway.
Am 29.09.2016 um 01:30 schrieb Junio C Hamano:
As Peff said, responding in a thread started by Linus's suggestion
to raise the default abbreviation to 12 hexdigits:
This is waayy too large for a new default. The vast majority of
repositories is smallish. For those, the long sequences of hex di
On Thu, 29 Sep 2016 at 04:44:00, SZEDER Gábor wrote:
> I for one raise my hand in protest...
>
> "few extra bytes" is not the only downside, and it's not at all about
> how many characters are copy-and-pasted. In my opinion it's much more
> important that this change wastes 5 columns worth of val
> As Peff said, responding in a thread started by Linus's suggestion
> to raise the default abbreviation to 12 hexdigits:
>
> I actually think "12" might be sane for a long time. That's 48 bits of
> sha1, so we'd expect a 50% change of a _single_ collision at 2^24, or 16
s/change/chance/
As Peff said, responding in a thread started by Linus's suggestion
to raise the default abbreviation to 12 hexdigits:
I actually think "12" might be sane for a long time. That's 48 bits of
sha1, so we'd expect a 50% change of a _single_ collision at 2^24, or 16
million. The biggest re
44 matches
Mail list logo