Git development
 help / color / mirror / Atom feed
* Re: Improving merge failure message
From: Jeff King @ 2009-09-08  7:51 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Sverre Rabbelier, Nanako Shiraishi, git
In-Reply-To: <7vk509ev20.fsf@alter.siamese.dyndns.org>

On Tue, Sep 08, 2009 at 12:48:23AM -0700, Junio C Hamano wrote:

> Thanks, very true indeed.  "It would be clobbered if we were to continue
> hence we abort." is how we want to explain our behaviour, so "would" is
> definitely better here.

While we're picking apart your wording, is "clobbered" the word we want
to use? Everywhere else that is user-facing we tend to use the term
"overwritten".

-Peff

^ permalink raw reply

* Re: [PATCH] Add more instructions about how to install git.
From: Michael J Gruber @ 2009-09-08  7:56 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: Alex Riesen, Thiago Farina, git
In-Reply-To: <vpqhbvekldk.fsf@bauges.imag.fr>

Matthieu Moy venit, vidit, dixit 08.09.2009 08:20:
> Alex Riesen <raa.lkml@gmail.com> writes:
> 
>> On Tue, Sep 8, 2009 at 07:11, Thiago Farina<tfransosi@gmail.com> wrote:
>>> Signed-off-by: Thiago Farina <tfransosi@gmail.com>
>>> ---
>>>  INSTALL |    9 +++++++++
>>>  1 files changed, 9 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/INSTALL b/INSTALL
>>> index ae7f750..67abfc7 100644
>>> --- a/INSTALL
>>> +++ b/INSTALL
>>> @@ -1,6 +1,15 @@
>>>
>>>                Git installation
>>>
>>> +To install git follow these 4 steps inside your git directory:
>>> +$ autoconf      # Generates the configure file from configure.ac
>>> +$ ./configure   # Configures everything needed to build git
>>> +$ make all      # Compiles git based in the Makefile
>>> +$ make install  # Installs git in your own ~/bin directory
> 
> Your commit message doesn't say what's wrong with
> 
> | Alternatively you can use autoconf generated ./configure script to
> | set up install paths (via config.mak.autogen), so you can write
> | instead
> | 
> |         $ make configure ;# as yourself
> |         $ ./configure --prefix=/usr ;# as yourself
> |         $ make all doc ;# as yourself
> |         # make install install-doc install-html;# as root
> 
> which is already in INSTALL a few lines after what your patch adds.

... especially since that really is a second class alternative these
days, with using the shipped Makefile being the preferred way - just as
the current documentation states.

> 
>>> +If you don't have autoconf installed, you can do this by:
>>> +$ sudo apt-get install autoconf
>>
>> ... if you're on a Debian-derived system. 
> 
> and if you have sudo configured.
> 
> BTW, apt-get is anyway somehow deprecaded, you should
> s/apt-get/aptitude/.
> 

^ permalink raw reply

* Re: Improving merge failure message
From: Mike Ralphson @ 2009-09-08  7:59 UTC (permalink / raw)
  To: Junio C Hamano, Jeff King; +Cc: Sverre Rabbelier, Nanako Shiraishi, git
In-Reply-To: <7vk509ev20.fsf@alter.siamese.dyndns.org>

2009/9/8 Junio C Hamano <gitster@pobox.com>:
> I'll leave the exact wording up to other people.  My primary focus was to
> end all of these messages with "Aborting."
>
> This turns out to be a continuation of an older discussion thread back in
> May 2008, and I do not know if anybody took up the challenge back then.  I
> wouldn't be surprised if "checkout", which was the topic of the old
> thread, has some other scary plumbing message still seeping through to the
> UI layer.  Perhaps there are some other commands that needs similar kind
> of love.

Just a note that Documentation/git-checkout.txt references this
message in an example and should be kept in step with the final
wording change. It would be ideal to be able to regression-test the
examples in the documentation somehow but that might involve abusing
the asciidoc markup somewhat.

2009/9/8 Jeff King <peff@peff.net>:
> While we're picking apart your wording, is "clobbered" the word we want
> to use?

If we're debating 'clobbered', then maybe the non-word 'uptodate' is
fair game too? 8-)

^ permalink raw reply

* Re: Improving merge failure message
From: Johannes Sixt @ 2009-09-08  8:24 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nanako Shiraishi, git
In-Reply-To: <7vbplmhr0i.fsf@alter.siamese.dyndns.org>

Junio C Hamano schrieb:
> Nanako Shiraishi <nanako3@lavabit.com> writes:
> 
>> [2]% git merge feature
>> error: Entry 'cool' not uptodate. Cannot merge.
>> fatal: merging of trees 8ec1d96451ff05451720e4e8968812c46b35e5e4 and aad8d5cef3915ab78b3227abaaac99b62db9eb54 failed
>>
>> ... the messages look unnecessarily scary, with two
>> "error" and "fatal" comments, and long sha1 commit names.
> 
>> It would be nice if the message in the latter case can be toned down.
> 
> Yeah, it would be nice.  This actually was something that bothered me as
> well while trying to explain the recovery procedure for these two cases.
> Give me half an hour or so to cook up something...

I think that this is a symptom of a much more involved issue about error
handling.

Currently, it is customary in the code to report an error at each location
where an exceptional condition is detected:

	if (frotz())
		ret = error("frotz messed up");

If frotz() is a "low-level" routine, like a C library function, then this
is the right thing to do. However, if frotz() is our own function
("high-level") which itself calls low-level functions with the same
pattern, then this is obviously the wrong thing to do, because it results
into two error messages.

We need a guideline how errors are reported. This guideline could be:

(1) Low-level functions do not print error messages, but set an error code
and leave the reporting to the caller.

(2) High-level functions must print an error message (through error() or
die()) when they detect a failure of a low-level function that was called
directly.

(3) High-level functions must not print an error message when they detect
a failure of another high-level function that was called directly.

There remains to classify our functions into high-level or low-level. But
I think we won't have a lot of low-level functions.

BTW, I applied this guideline when I worked on {start,finish,run}_command
recently, and IMHO it worked out quite nicely because exactly one error is
reported after a failure.

-- Hannes

^ permalink raw reply

* Gitweb linking to bug-tracking system
From: Arnaud Bailly @ 2009-09-08  8:26 UTC (permalink / raw)
  To: git

HEllo,
I have installed gitweb using git version 1.6.3.3 on ubuntu, and
everything is working perfectly. 
However, when a commit have a comment containing a string matching
"DEF[0-9]+", an hyperlink is generated on the web interface which makes
me think it is possible to link to some DTS. Yet I cannot find how to
configure this ? 

Am I missing something ? Is this totally unrelated to DTS ?

Regards,

-- 
Arnaud Bailly -- OQube
<software engineering>
http://www.oqube.com/

^ permalink raw reply

* Re: [PATCH] git-rebase-interactive: avoid breaking when GREP_OPTIONS="-H"
From: Carlo Marcelo Arenas Belon @ 2009-09-08  8:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vpra1gbqc.fsf@alter.siamese.dyndns.org>

On Tue, Sep 08, 2009 at 12:02:51AM -0700, Junio C Hamano wrote:
> 
> Or even this.

definitely better.

Carlo

^ permalink raw reply

* Re: [PATCHv5 00/14] git notes
From: Johan Herland @ 2009-09-08  8:54 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Johannes.Schindelin, trast, tavestbo, git, chriscool,
	spearce
In-Reply-To: <7vd462qdeg.fsf@alter.siamese.dyndns.org>

On Tuesday 08 September 2009, Junio C Hamano wrote:
> Johan Herland <johan@herland.net> writes:
> > Furthermore, the following notes tree structures were tested:
> >
> > - "no-notes": Testing without any notes at all. This is only present as
> > a baseline, and to verify that the notes code does not negatively
> > affect performance when not in use.
> 
> Minor nit.
> 
> For this to be a baseline, you would need to have another algorithm
>  before "before", i.e., without any of these notes implementation.
> 
> Comparison with "before" alone is not meaningful.  That is like starting
> with a state with unknown performance regression compared to the stock
> version, and then boast improvements made by various variations.
> 
> You would need to compare overhead of various "algorithms" with the stock
> git in "no-notes" case as well.  It would give us the true performance
> cost of supporting notes.

True. Here is the same table with the baseline ('next') entry on top:


Algorithm / Notes tree   git log -n10 (x100)   git log --all
------------------------------------------------------------
next / no-notes                4.77s              63.84s

before / no-notes              4.78s              63.90s
before / no-fanout            56.85s              65.69s

16tree / no-notes              4.77s              64.18s
16tree / no-fanout            30.35s              65.39s
16tree / 2_38                  5.57s              65.42s
16tree / 2_2_36                5.19s              65.76s

flexible / no-notes            4.78s              63.91s
flexible / no-fanout          30.34s              65.57s
flexible / 2_38                5.57s              65.46s
flexible / 2_2_36              5.18s              65.72s
flexible / ym                  5.13s              65.66s
flexible / ym_2_38             5.08s              65.63s
flexible / ymd                 5.30s              65.45s
flexible / ymd_2_38            5.29s              65.90s
flexible / y_m                 5.11s              65.72s
flexible / y_m_2_38            5.08s              65.67s
flexible / y_m_d               5.06s              65.50s
flexible / y_m_d_2_38          5.07s              65.79s


...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

^ permalink raw reply

* Re: Improving merge failure message
From: Alex Riesen @ 2009-09-08  9:11 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nanako Shiraishi, git
In-Reply-To: <7veiqhgb4y.fsf@alter.siamese.dyndns.org>

On Tue, Sep 8, 2009 at 09:15, Junio C Hamano<gitster@pobox.com> wrote:
> @@ -170,6 +170,18 @@ static int git_merge_trees(int index_only,
>        int rc;
>        struct tree_desc t[3];
>        struct unpack_trees_options opts;
> +       static struct unpack_trees_error_msgs msgs = {

You can make these const, the struct is copied anyway:

> +       opts.msgs = msgs;

Here --^

^ permalink raw reply

* Re: Gitweb linking to bug-tracking system
From: Alex Riesen @ 2009-09-08  9:22 UTC (permalink / raw)
  To: Arnaud Bailly; +Cc: git
In-Reply-To: <85ab15n8of.fsf@oqube.com>

On Tue, Sep 8, 2009 at 10:26, Arnaud Bailly<abailly@oqube.com> wrote:
> HEllo,
> I have installed gitweb using git version 1.6.3.3 on ubuntu, and
> everything is working perfectly.
> However, when a commit have a comment containing a string matching
> "DEF[0-9]+", an hyperlink is generated on the web interface which makes
> me think it is possible to link to some DTS. Yet I cannot find how to
> configure this ?
>
> Am I missing something ? Is this totally unrelated to DTS ?

It just looks like an SHA1 and is converted to a link to simplify
jumping around revisions.

^ permalink raw reply

* Re: [PATCHv5 00/14] git notes
From: Johannes Schindelin @ 2009-09-08  9:32 UTC (permalink / raw)
  To: Johan Herland
  Cc: Junio C Hamano, git, trast, tavestbo, git, chriscool, spearce
In-Reply-To: <200909081054.02523.johan@herland.net>

Hi,

On Tue, 8 Sep 2009, Johan Herland wrote:

> Algorithm / Notes tree   git log -n10 (x100)   git log --all
> ------------------------------------------------------------
> next / no-notes                4.77s              63.84s
> 
> before / no-notes              4.78s              63.90s
> before / no-fanout            56.85s              65.69s
> 
> 16tree / no-notes              4.77s              64.18s
> 16tree / no-fanout            30.35s              65.39s
> 16tree / 2_38                  5.57s              65.42s
> 16tree / 2_2_36                5.19s              65.76s
> 
> flexible / no-notes            4.78s              63.91s
> flexible / no-fanout          30.34s              65.57s
> flexible / 2_38                5.57s              65.46s
> flexible / 2_2_36              5.18s              65.72s
> flexible / ym                  5.13s              65.66s
> flexible / ym_2_38             5.08s              65.63s
> flexible / ymd                 5.30s              65.45s
> flexible / ymd_2_38            5.29s              65.90s
> flexible / y_m                 5.11s              65.72s
> flexible / y_m_2_38            5.08s              65.67s
> flexible / y_m_d               5.06s              65.50s
> flexible / y_m_d_2_38          5.07s              65.79s

It's good to see that the no-notes behaves roughly like baseline.

I can see that some people may think that date-based fan-out is the cat's 
ass, but I have to warn that we have no idea how notes will be used, and 
the date-based fan-out is rather limiting in certain respects:

- for the typical nightly-build-generated notes, this fan-out is pretty 
  inefficient memory-wise.

- I find the restriction to commits rather limiting.

- most of the performance difference between the date-based and the SHA-1 
  based fan-out looks to me as if the issue was the top-level tree.  
  Basically, this tree has to be read _every_ time _anybody_ wants to read 
  a note.

  Maybe a finer-grained fan-out (finer than 16-bits) could help.  After 
  all, if you have 16 different notes, chances are that they have 16 
  different first letters, but all have the same commit year.  That's 
  where the top-level notes with a fan-out perform incredibly bad.

  But I think that having a dynamic fan-out that can even put blobs into 
  the top-level tree (nothing prevents us from doing that, right?) would 
  _outperform_ the date-based one, at least with less than 1 note/commit 
  (and maybe even then, because the year-based fan-out results in pretty 
  varying entropies per fan-out depth).

  The real question for me, therefore, is: what is the optimal way to 
  strike the balance between size of the tree objects (which we want to 
  be small, so that unpacking them is fast)  and depth of the fan-out 
  (which we want to be shallow to avoid reading worst-case 39 tree objects 
  to get at one single note).

- related to the previous point is my gut feeling that the date-based 
  fan-out has nothing to do with any theoretical optimum.  I am pretty 
  certain that the optimal fan-out strategy depends heavily on the SHA-1s 
  of the annotated objects (if you have 10,000 notes in 2009, but only 1 
  in 2008, the year-based fan-out _must_ be suboptimal)  and maybe is 
  something like a sibling to the Fibonacci heap.

- I'd love to see performance numbers for less than 157118 notes.  Don't 
  get me wrong, it is good to see the worst-case scenario in terms of 
  notes/commits ratio.  But it will hardly be the common case, and I 
  very much would like to optimize for the common case.

  So, I'd appreciate if you could do the tests with something like 500 
  notes, randomly spread over the commits (rationale: my original 
  understanding was that the notes could amend commit messages, and that 
  is much more likely to be done with relatively old commits that you 
  cannot change anymore).

Please understand that I might not have the time to participate in this 
thread as much as I would like to.  The next 4 days will be especially 
hard.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] post-receive-email: do not call sendmail if no mail was generated
From: Lars Noschinski @ 2009-09-08  9:20 UTC (permalink / raw)
  To: git; +Cc: gitster
In-Reply-To: <1251481187-6361-1-git-send-email-lars@public.noschinski.de>

* Lars Noschinski <lars@public.noschinski.de> [09-08-28 19:39]:
> contrib/hooks/post-receive-email used to call the send_mail function
> (and thus, /usr/sbin/sendmail), even if generate_mail returned an error.
> This is problematic, as the sendmail binary provided by exim4 generates
> an error mail if provided with an empty input.
> 
> Therefore, this commit changes post-receive-email to only call sendmail
> if generate_mail returned without error.
> 
> Signed-off-by: Lars Noschinski <lars@public.noschinski.de>

Is anything wrong with this patch? Or is it just queued to be committed
some time?

 - Lars.

^ permalink raw reply

* Re: Improving merge failure message
From: Nanako Shiraishi @ 2009-09-08 10:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7veiqhgb4y.fsf@alter.siamese.dyndns.org>

Quoting Junio C Hamano <gitster@pobox.com>

> Notable points are:
>
>  - End the messages with "Aborting."; they are given when the three-way
>    merge stops without harming the work tree;
>
>  - Do not give the extra message after unpack_trees() already errored out.
>    This "merging of trees failed" message was primarily for debugging
>    merge-recursive itself, and the end user cannot do much with the object
>    names given in the message anyway.
>
>    But do give it under higher verbosity level, or when it happens during
>    the inner merge (the "recursive" one), as unpack_trees() should not
>    fail for the inner merge under normal conditions.
>
> We could later add instructions on how to recover (i.e. "stash changes
> away or commit on a side branch and retry") instead of the silent
> exit(128) I have down there, and then use Peff's advice.* mechanism to
> squelch it (e.g. "advice.mergeindirtytree"), but they are separate topics.

Thank you for a quick response. The patch works fine here, so if you want
please add:

Tested-by: Nanako Shiraishi <nanako3@lavabit.com>

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

^ permalink raw reply

* [ANNOUNCE] GitX v0.7 released
From: Johannes Gilger @ 2009-09-08  9:38 UTC (permalink / raw)
  To: Git Mailing List

Hello git-community,

most of you will already have noticed it (through the builtin 
update-functionality) but I'd thought I make a quick announcement here 
as well: GitX 0.7, a git-GUI for Mac OS X, was released yesterday.

To see what's changed have a look at the Release History [1]. I also 
wrote up a blog-post [2] describing the most obvious changes with 
screenshots. If you're a GitX-user, consider taking our short survey [3] 
which we set up for the release of 0.7.

If you're thinking about contributing to GitX have a look at our fairly 
new mailinglist [4], which is supposed to become the main means of 
coordination for us. We have a few big features that have been requested 
by users time and again, so it would be a perfect chance to make a big 
impression on a relatively young and small open-source project.

Thanks to the git-community for providing us with broad and steady 
shoulders to stand on ;)

Greetings,
Jojo

[1] - http://gitx.frim.nl/release_history.html
[2] - http://heipei.net/2009/09/07/gitx-0-7-released/
[3] - http://www.survs.com/survey?id=DCJKLP2B&channel=Q065KU8XL1
[4] - http://groups.google.com/group/gitx

-- 
Johannes Gilger <heipei@hackvalue.de>
http://heipei.net
GPG-Key: 0x42F6DE81
GPG-Fingerprint: BB49 F967 775E BB52 3A81  882C 58EE B178 42F6 DE81

^ permalink raw reply

* Re: Gitweb linking to bug-tracking system
From: Arnaud Bailly @ 2009-09-08 12:12 UTC (permalink / raw)
  To: git
In-Reply-To: <81b0412b0909080222u58c7e0bey81a5b127b04d6031@mail.gmail.com>

Thanks. 
-- 
Arnaud Bailly -- OQube
<software engineering>
http://www.oqube.com/

^ permalink raw reply

* Re: [PATCHv5 00/14] git notes
From: Johan Herland @ 2009-09-08 12:36 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: git, Junio C Hamano, trast, tavestbo, git, chriscool, spearce
In-Reply-To: <alpine.DEB.1.00.0909081100020.4330@intel-tinevez-2-302>

Hi,

On Tuesday 08 September 2009, Johannes Schindelin wrote:
> On Tue, 8 Sep 2009, Johan Herland wrote:
> > Algorithm / Notes tree   git log -n10 (x100)   git log --all
> > ------------------------------------------------------------
> > next / no-notes                4.77s              63.84s
> >
> > before / no-notes              4.78s              63.90s
> > before / no-fanout            56.85s              65.69s
> >
> > 16tree / no-notes              4.77s              64.18s
> > 16tree / no-fanout            30.35s              65.39s
> > 16tree / 2_38                  5.57s              65.42s
> > 16tree / 2_2_36                5.19s              65.76s
> >
> > flexible / no-notes            4.78s              63.91s
> > flexible / no-fanout          30.34s              65.57s
> > flexible / 2_38                5.57s              65.46s
> > flexible / 2_2_36              5.18s              65.72s
> > flexible / ym                  5.13s              65.66s
> > flexible / ym_2_38             5.08s              65.63s
> > flexible / ymd                 5.30s              65.45s
> > flexible / ymd_2_38            5.29s              65.90s
> > flexible / y_m                 5.11s              65.72s
> > flexible / y_m_2_38            5.08s              65.67s
> > flexible / y_m_d               5.06s              65.50s
> > flexible / y_m_d_2_38          5.07s              65.79s
>
> It's good to see that the no-notes behaves roughly like baseline.

Agreed.

> I can see that some people may think that date-based fan-out is the
> cat's ass, but I have to warn that we have no idea how notes will be
> used,

I don't agree. Although we will certainly see many more use cases for 
notes, I believe that the vast majority of them can be placed in one of 
two categories:

1. Looking up a few (1 - 10) notes on an individual basis. In this case, 
performance will always be "good enough", and I don't believe it's 
worth spending much time optimizing for this case.

2. Looking up many notes in a sequence based on the chronology of the 
objects (commits) they annotate. This is what 'git log' does, and I 
believe this is the case we should optimize for.

Once you work from this assumption, it is clear that date-based fanout 
beats pure SHA1-based fanout, because it changes the notes lookup 
process from random-access to near-streaming. This is clearly reflected 
in the memory usage graphs I posted.

Also note that the "flexible" code to some degree resolves the whole 
date-based fanout vs. SHA1-based fanout discussion: We are now free to 
choose -- at runtime -- which notes tree structure is more optimal for 
a given collection of notes.

> and the date-based fan-out is rather limiting in certain respects:
>
> - for the typical nightly-build-generated notes, this fan-out is
> pretty inefficient memory-wise.

Yes and no. A y/m/d/40 structure with 1 note for every y/m/d is indeed 
wasteful, but using a y/40 structure instead creates a much better 
situation with a "healthy" ~365 notes per year level. And the y/40 
still preserves some of the 'streaming' aspects mentioned above.

> - I find the restriction to commits rather limiting.

I see your point, but I don't agree until I see a compelling case for 
annotating a non-commit.

In any case, the flexible lookup code still allow us to organize notes 
purely by SHA1-based fanout, so annotated trees/blobs could still be 
supported by the same code (modulo a small s/struct commit/struct 
object/ patch) provided that they are stored in a notes tree that 
simply does not use date-based fanout.

> - most of the performance difference between the date-based and the
> SHA-1 based fan-out looks to me as if the issue was the top-level
> tree. Basically, this tree has to be read _every_ time _anybody_
> wants to read a note.

Not sure what you're trying to say here. The top-level notes tree is 
read (as in fill_tree_descriptor()) exactly _once_. After that, it is 
cached by the internal data structure (until free_commit_notes() or 
end-of-process).

In the SHA1-based case, each notes lookup does indeed start at the root 
of the data structure (corresponding to the top-level tree), but in the 
date-based case, we keep a pointer (cur_node) to the innermost 
date-based tree that was most recently used. Thus, if the next note is 
within the same date-based tree (which I assume is the common case), 
then we don't need to look at the root of the data structure.

>   Maybe a finer-grained fan-out (finer than 16-bits) could help. 
> After all, if you have 16 different notes, chances are that they have
> 16 different first letters, but all have the same commit year. 
> That's where the top-level notes with a fan-out perform incredibly
> bad.

Not really, the first lookup would start at the root node, and navigate 
into the year node, but all subsequent lookups would start directly at 
the year node (and only backtrack if the commit year doesn't match the 
year node).

BTW, when you mention "finer than 16-bits", do you mean moving from a 
16-tree to a, say, 32-tree or 64-tree structure? That will complicate 
the tree navigation somewhat, and increase memory waste. (Remember, I 
started out with a 256-tree idea, but scrapped it because of memory 
waste).

>   But I think that having a dynamic fan-out that can even put blobs
> into the top-level tree (nothing prevents us from doing that, right?)

Well, the "flexible" code does add the new requirement that all entries 
in a notes (sub)tree object must follow the same scheme, i.e. you 
cannot have:

  /12/34567890123456789012345678901234567890
  /2345/678901234567890123456789012345678901

but you can have

  /12/34567890123456789012345678901234567890
  /23/45/678901234567890123456789012345678901

> would _outperform_ the date-based one, at least with less than 1
> note/commit (and maybe even then, because the year-based fan-out
> results in pretty varying entropies per fan-out depth).
>
>   The real question for me, therefore, is: what is the optimal way to
>   strike the balance between size of the tree objects (which we want
> to be small, so that unpacking them is fast)  and depth of the
> fan-out (which we want to be shallow to avoid reading worst-case 39
> tree objects to get at one single note).

s/39/19/ (each fanout must use at least 2 chars of the 40-char SHA1)

Yes, the challenge is indeed striking the correct balance. I believe 
that the notes code should be taught to write (and automatically 
re-organize) the notes tree so that it is optimized for the current 
collection of notes.

> - related to the previous point is my gut feeling that the date-based
>   fan-out has nothing to do with any theoretical optimum.  I am
> pretty certain that the optimal fan-out strategy depends heavily on
> the SHA-1s of the annotated objects (if you have 10,000 notes in
> 2009, but only 1 in 2008, the year-based fan-out _must_ be
> suboptimal)  and maybe is something like a sibling to the Fibonacci
> heap.

Yes, it is trivial to create scenarios where any rigid date-based fanout 
is suboptimal. However, it is equally trivial to create scenarios where 
any SHA1-based fanout will perform worse than a carefully chosen 
date-based fanout. I believe the best way forward is to design for 
flexibility in the notes tree structure, and then teach the notes 
_writing_ code to choose a notes tree structure that is good/optimal 
for the given set of notes.

> - I'd love to see performance numbers for less than 157118 notes. 
> Don't get me wrong, it is good to see the worst-case scenario in
> terms of notes/commits ratio.  But it will hardly be the common case,
> and I very much would like to optimize for the common case.
>
>   So, I'd appreciate if you could do the tests with something like
> 500 notes, randomly spread over the commits (rationale: my original
> understanding was that the notes could amend commit messages, and
> that is much more likely to be done with relatively old commits that
> you cannot change anymore).

Ok. I will try to test that.

> Please understand that I might not have the time to participate in
> this thread as much as I would like to.  The next 4 days will be
> especially hard.

Thanks for the feedback! I appreciate any time you're able to spend on 
this. And I don't mind waiting for a few days for more feedback.


Have fun! :)

...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

^ permalink raw reply

* Re: Issue 323 in msysgit: Can't clone over http
From: Jakub Narebski @ 2009-09-08 12:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Tay Ray Chuan, git, msysgit, Tom Preston-Werner
In-Reply-To: <7v7hwatw0v.fsf@alter.siamese.dyndns.org>


Junio C Hamano <gitster@pobox.com> writes:
> Tay Ray Chuan <rctay89@gmail.com> writes:
> 
> > Just to clarify: the "check" of CURLE_OK in http-walker.c::fetch_index()
> > in v1.6.3 is fundamentally different from the check we have in 48188c2
> > (http-walker: verify remote packs, 2009-06-06).
> >
> > The first "check" is a full-blooded GET request, and we do get back
> > actual data (in this case, the pack index file). The second check isn't
> > a GET request, just an inconsequential HEAD request; we don't get back
> > any real data.
> 
> Yeah, I realized after I wrote the message you are responding to and ran
> some experiments with github repository myself to see for some of their
> URLs HEAD gives 500 when GET gives the contents just fine.
> 
>   Tom, sorry to have given a confusing list of issues in my earlier message.
>   Please disregard it.  The only funny your HTTP server folks may want to
>   look into is that GET request to fetch the following URL gives the
>   contents just fine, but HEAD request to it results in an Internal Server
>   Error.
> 
>   http://github.com/tekkub/addontemplate.git/objects/pack/pack-382c25c935b744e909c749532578112d72a4aff9.pack
> 
>   Sorry about the confusion.

[...]

> On the other hand, if a HEAD request to a URL results in an unauthorized,
> what plausible excuse the server operator could give for allowing a GET
> request to the same URL?  If we are going to keep the check if *.pack that
> corresponds to the *.idx will be available, shouldn't we trust whatever
> check we perform?

500 Internal Server Error doesn't look right (well, it can indicate
bug in server code), but would git respond to correct error code other
than 404 Not Found, like 405 Method Not Allowed, or 501 Not Implemented?

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* Re: Issue 323 in msysgit: Can't clone over http
From: Tay Ray Chuan @ 2009-09-08 13:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, msysgit, Tom Preston-Werner
In-Reply-To: <7v7hwatw0v.fsf@alter.siamese.dyndns.org>

Hi,

On Tue, Sep 8, 2009 at 3:06 AM, Junio C Hamano<gitster@pobox.com> wrote:
> I am torn about this.
>
> On one hand, if we are going to treat such a half failure as nothing
> conclusive, I do not see a point to keep that check to begin with.
>
> On the other hand, if a HEAD request to a URL results in an unauthorized,
> what plausible excuse the server operator could give for allowing a GET
> request to the same URL?  If we are going to keep the check if *.pack that
> corresponds to the *.idx will be available, shouldn't we trust whatever
> check we perform?

I am in favour or removing this check, not just due to its
unreliability, but for the sake of consistency (we very rarely send a
HEAD request to poll data before doing a GET).

Do disregard the patch included earlier, if you haven't already done so.

-- 
Cheers,
Ray Chuan

^ permalink raw reply

* Re: What's cooking in git.git (Sep 2009, #02; Mon, 07)
From: Jakub Narebski @ 2009-09-08 13:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vtyzexnhm.fsf@alter.siamese.dyndns.org>

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

> * jn/gitweb-show-size (2009-09-07) 1 commit
>  - gitweb: Add 'show-sizes' feature to show blob sizes in tree view

This is an RFC, because of a) feature naming, and b) tests.
It does work correctly, though.
 
> * jn/gitweb-blame (2009-09-01) 5 commits
>  - gitweb: Minify gitweb.js if JSMIN is defined
>  - gitweb: Create links leading to 'blame_incremental' using JavaScript
>   (merged to 'next' on 2009-09-07 at 3622199)

I plan on reverting the order of those two patches, as JSMIN patch is
more ready than 'blame_incremental' linking.

With JSMIN there is perhaps question of documenting it, and whether
git-instaweb.sh should use minified JavaScript or not (curently it
does not).  With 'blame_incremental' linking there are quite a few
issues:

 1. When to do JavaScript detection
    a. on load (current solution)
    b. on click (via event handler)
 2. How to invoke actions requiring JavaScript
    a. add 'js=1' parameter (perhaps limiting URLs, as not to
       add it e.g. to http://git-scm.com/ $home_link)
    b. rewrite action parameter, e.g. from 'blame' to
       'blame_incremental' (troubles with path_info URLs)
    c. use cookies

Current solution works, but I think is suboptimal.

>  + gitweb: Colorize 'blame_incremental' view during processing
>  + gitweb: Incremental blame (using JavaScript)
>  + gitweb: Add optional "time to generate page" info in footer
> 
> Ajax-y blame.

I'd very much like for others to check if it works with other browsers
(Internet Explorer 6, 7, 8; Opera 9, 10; Safari; Firefox 2, 3, 3.5)

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* Re: What's cooking in git.git (Sep 2009, #02; Mon, 07)
From: Sverre Rabbelier @ 2009-09-08 13:17 UTC (permalink / raw)
  To: Shawn O. Pearce, Junio C Hamano; +Cc: git
In-Reply-To: <7vtyzexnhm.fsf@alter.siamese.dyndns.org>

Heya,

On Tue, Sep 8, 2009 at 02:56, Junio C Hamano<gitster@pobox.com> wrote:
> * sr/gfi-options (2009-09-06) 6 commits
>  (merged to 'next' on 2009-09-07 at 5f6b0ff)
>  + fast-import: test the new option command
>  + fast-import: add option command
>  + fast-import: test the new feature command
>  + fast-import: add feature command
>  + fast-import: put marks reading in it's own function
>  + fast-import: put option parsing code in separate functions
>
> Perhaps 1.6.5 material but I wasn't sure.

Shawn, what's your opinion of v7? I think it handles pretty much
everything we need for at least a draft and is good for 1.6.5, but
it's up to you of course :).

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: Issue 323 in msysgit: Can't clone over http
From: Tay Ray Chuan @ 2009-09-08 13:18 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Junio C Hamano, git, msysgit, Tom Preston-Werner
In-Reply-To: <m3ocplvbmj.fsf@localhost.localdomain>

Hi,

On Tue, Sep 8, 2009 at 8:57 PM, Jakub Narebski<jnareb@gmail.com> wrote:
> 500 Internal Server Error doesn't look right (well, it can indicate
> bug in server code), but would git respond to correct error code other
> than 404 Not Found, like 405 Method Not Allowed, or 501 Not Implemented?

I believe you're referring to git response to such error codes for GET requests?

For 404, we'll try using alternates (objects/info/http-alternates,
objects/info/alternates).

For the rest, we'll just abort the request. I'm not really sure of the
specific responses by git, like if we do ask for authentication, in
the case of 401s.

-- 
Cheers,
Ray Chuan

^ permalink raw reply

* [PATCH] Makefile: Add NEEDS_CRYPTO_WITH_SSL
From: Brian Gernhardt @ 2009-09-08 13:54 UTC (permalink / raw)
  To: Git List; +Cc: Junio C Hamano

The Makefile comment for NEEDS_SSL_WITH_CRYPTO says to define it "if
you need -lcrypto with -lssl (Darwin)."  However, what it actually
does is add -lssl when you use -lcrypto and not the other way around.
However, libcrypto contains a majority of the ERR_* functions from
OpenSSL (at least on OS X) so we need it both ways.

So, add NEEDS_CRYPTO_WITH_SSL which adds -lcrypto to the OpenSSL link
flags and clarify the difference between it and NEEDS_SSL_WITH_CRYPTO.

Signed-off-by: Brian Gernhardt <brian@gernhardtsoftware.com>
---

 Compilation using BLK_SHA1 on OS X 10.5 and 10.6 (at least) is still
 broken without this patch.

 Alex Riesen <raa.lkml@gmail.com> pointed out that just adding LIB_4_CRYPTO
 to git-imap-send is simpler, but judging from the fact that nobody else
 has complained about this issue, I'm guessing that the need for -lcrypto
 when using -lssl is not widespread.  (Or BLK_SHA1 isn't getting used much
 or those who do don't compile git-imap-send with SSL.)

 Makefile |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index ce882d0..121be04 100644
--- a/Makefile
+++ b/Makefile
@@ -91,7 +91,9 @@ all::
 # Define PPC_SHA1 environment variable when running make to make use of
 # a bundled SHA1 routine optimized for PowerPC.
 #
-# Define NEEDS_SSL_WITH_CRYPTO if you need -lcrypto with -lssl (Darwin).
+# Define NEEDS_CRYPTO_WITH_SSL if you need -lcrypto when using -lssl (Darwin).
+#
+# Define NEEDS_SSL_WITH_CRYPTO if you need -lssl when using -lcrypto (Darwin).
 #
 # Define NEEDS_LIBICONV if linking with libc is not enough (Darwin).
 #
@@ -707,6 +709,7 @@ ifeq ($(uname_S),SCO_SV)
 	TAR = gtar
 endif
 ifeq ($(uname_S),Darwin)
+	NEEDS_CRYPTO_WITH_SSL = YesPlease
 	NEEDS_SSL_WITH_CRYPTO = YesPlease
 	NEEDS_LIBICONV = YesPlease
 	ifeq ($(shell expr "$(uname_R)" : '[15678]\.'),2)
@@ -1009,6 +1012,9 @@ ifndef NO_OPENSSL
 	else
 		OPENSSL_LINK =
 	endif
+	ifdef NEEDS_CRYPTO_WITH_SSL
+		OPENSSL_LINK += -lcrypto
+	endif
 else
 	BASIC_CFLAGS += -DNO_OPENSSL
 	BLK_SHA1 = 1
-- 
1.6.4.2.420.g30ecf

^ permalink raw reply related

* Re: [PATCH] Add more instructions about how to install git.
From: Thiago Farina @ 2009-09-08 14:24 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: Alex Riesen, git
In-Reply-To: <vpqhbvekldk.fsf@bauges.imag.fr>

On Tue, Sep 8, 2009 at 3:20 AM, Matthieu
Moy<Matthieu.Moy@grenoble-inp.fr> wrote:
> Your commit message doesn't say what's wrong with
>
> | Alternatively you can use autoconf generated ./configure script to
> | set up install paths (via config.mak.autogen), so you can write
> | instead
> |
> |         $ make configure ;# as yourself
> |         $ ./configure --prefix=/usr ;# as yourself
> |         $ make all doc ;# as yourself
> |         # make install install-doc install-html;# as root
>
> which is already in INSTALL a few lines after what your patch adds.

I tried to being more explicit about what the new users has to do. The
first instruction that the text says is: "Normaly you can just do
"make"".
But the new user, cloning git won't have a configure file, so he can't
just do "make".

And then, the text says: "so you can write instead". This brings me to
one alternative (not what I really should do), when in fact it should
be (I guess) the first instructions that new users need to follow.

^ permalink raw reply

* Re: [PATCH] Add more instructions about how to install git.
From: Erik Faye-Lund @ 2009-09-08 14:37 UTC (permalink / raw)
  To: Thiago Farina; +Cc: Matthieu Moy, Alex Riesen, git
In-Reply-To: <a4c8a6d00909080724q1c792f6bs641ea9d000d08b5a@mail.gmail.com>

On Tue, Sep 8, 2009 at 4:24 PM, Thiago Farina<tfransosi@gmail.com> wrote:
> I tried to being more explicit about what the new users has to do. The
> first instruction that the text says is: "Normaly you can just do
> "make"".
> But the new user, cloning git won't have a configure file, so he can't
> just do "make".

I think you are confused. There's a default Makefile there, so a
normal user CAN just call "make". What a new user can't do, is to call
"./configure" without generating the configure-script first. There are
already instructions on how to do this.

-- 
Erik "kusma" Faye-Lund
kusmabite@gmail.com
(+47) 986 59 656

^ permalink raw reply

* Re: [PATCH] Add more instructions about how to install git.
From: Johannes Sixt @ 2009-09-08 14:38 UTC (permalink / raw)
  To: Thiago Farina; +Cc: Matthieu Moy, Alex Riesen, git
In-Reply-To: <a4c8a6d00909080724q1c792f6bs641ea9d000d08b5a@mail.gmail.com>

Thiago Farina schrieb:
> I tried to being more explicit about what the new users has to do. The
> first instruction that the text says is: "Normaly you can just do
> "make"".
> But the new user, cloning git won't have a configure file, so he can't
> just do "make".
> 
> And then, the text says: "so you can write instead". This brings me to
> one alternative (not what I really should do), when in fact it should
> be (I guess) the first instructions that new users need to follow.

Then guessed wrong. You really only have to say "make". The use of
autoconf and ./configure is purely optional and not necessary.

-- Hannes

^ permalink raw reply

* Re: [PATCH] Add more instructions about how to install git.
From: Thiago Farina @ 2009-09-08 14:52 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Matthieu Moy, Alex Riesen, git
In-Reply-To: <4AA66C54.4060101@viscovery.net>

On Tue, Sep 8, 2009 at 10:38 AM, Johannes Sixt<j.sixt@viscovery.net> wrote:
> Thiago Farina schrieb:
>> I tried to being more explicit about what the new users has to do. The
>> first instruction that the text says is: "Normaly you can just do
>> "make"".
>> But the new user, cloning git won't have a configure file, so he can't
>> just do "make".
>>
>> And then, the text says: "so you can write instead". This brings me to
>> one alternative (not what I really should do), when in fact it should
>> be (I guess) the first instructions that new users need to follow.
>
> Then guessed wrong. You really only have to say "make". The use of
> autoconf and ./configure is purely optional and not necessary.
I don't think so.
Running only "make".
$ make
GIT_VERSION = 1.6.5.rc0
    * new build flags or prefix
    CC fast-import.o
In file included from builtin.h:4,
                 from fast-import.c:143:
git-compat-util.h:129:25: error: openssl/ssl.h: No such file or directory
git-compat-util.h:130:25: error: openssl/err.h: No such file or directory
In file included from builtin.h:6,
                 from fast-import.c:143:
cache.h:8:21: error: openssl/sha.h: No such file or directory
In file included from fast-import.c:152:
csum-file.h:10: error: expected specifier-qualifier-list before ‘SHA_CTX’
fast-import.c: In function ‘create_index’:
fast-import.c:850: error: ‘SHA_CTX’ undeclared (first use in this function)
fast-import.c:850: error: (Each undeclared identifier is reported only once
fast-import.c:850: error: for each function it appears in.)
fast-import.c:850: error: expected ‘;’ before ‘ctx’
fast-import.c:886: warning: implicit declaration of function ‘SHA1_Init’
fast-import.c:886: error: ‘ctx’ undeclared (first use in this function)
fast-import.c:891: warning: implicit declaration of function ‘SHA1_Update’
fast-import.c:896: warning: implicit declaration of function ‘SHA1_Final’
fast-import.c: In function ‘store_object’:
fast-import.c:1035: error: ‘SHA_CTX’ undeclared (first use in this function)
fast-import.c:1035: error: expected ‘;’ before ‘c’
fast-import.c:1040: error: ‘c’ undeclared (first use in this function)
fast-import.c: In function ‘validate_raw_date’:
fast-import.c:1750: warning: ignoring return value of ‘strtoul’,
declared with attribute warn_unused_result
fast-import.c:1758: warning: ignoring return value of ‘strtoul’,
declared with attribute warn_unused_result
make: *** [fast-import.o] Error 1

Running configure and then make
$ make configure
GIT_VERSION = 1.6.5.rc0.dirty
    GEN configure
$ make
    CC fast-import.o
In file included from builtin.h:4,
                 from fast-import.c:143:
git-compat-util.h:129:25: error: openssl/ssl.h: No such file or directory
git-compat-util.h:130:25: error: openssl/err.h: No such file or directory
In file included from builtin.h:6,
                 from fast-import.c:143:
cache.h:8:21: error: openssl/sha.h: No such file or directory
In file included from fast-import.c:152:
csum-file.h:10: error: expected specifier-qualifier-list before ‘SHA_CTX’
fast-import.c: In function ‘create_index’:
fast-import.c:850: error: ‘SHA_CTX’ undeclared (first use in this function)
fast-import.c:850: error: (Each undeclared identifier is reported only once
fast-import.c:850: error: for each function it appears in.)
fast-import.c:850: error: expected ‘;’ before ‘ctx’
fast-import.c:886: warning: implicit declaration of function ‘SHA1_Init’
fast-import.c:886: error: ‘ctx’ undeclared (first use in this function)
fast-import.c:891: warning: implicit declaration of function ‘SHA1_Update’
fast-import.c:896: warning: implicit declaration of function ‘SHA1_Final’
fast-import.c: In function ‘store_object’:
fast-import.c:1035: error: ‘SHA_CTX’ undeclared (first use in this function)
fast-import.c:1035: error: expected ‘;’ before ‘c’
fast-import.c:1040: error: ‘c’ undeclared (first use in this function)
fast-import.c: In function ‘validate_raw_date’:
fast-import.c:1750: warning: ignoring return value of ‘strtoul’,
declared with attribute warn_unused_result
fast-import.c:1758: warning: ignoring return value of ‘strtoul’,
declared with attribute warn_unused_result
make: *** [fast-import.o] Error 1

Now autconf, ./configure, make.
$ autoconf
$ ./configure
configure: Setting lib to 'lib' (the default)
configure: Will try -pthread then -lpthread to enable POSIX Threads.
configure: CHECKS for site configuration
configure: CHECKS for programs
checking for cc... cc
checking for C compiler default output file name... a.out
checking whether the C compiler works... yes
checking whether we are cross compiling... no
checking for suffix of executables...
checking for suffix of object files... o
checking whether we are using the GNU C compiler... yes
checking whether cc accepts -g... yes
checking for cc option to accept ISO C89... none needed
checking if linker supports -R... no
checking if linker supports -Wl,-rpath,... yes
checking for gar... no
checking for ar... ar
checking for gtar... no
checking for tar... tar
checking for asciidoc... asciidoc
checking for asciidoc version... asciidoc 8.2.7 > 7
configure: CHECKS for libraries
checking for SHA1_Init in -lcrypto... no
checking for SHA1_Init in -lssl... no
checking for curl_global_init in -lcurl... yes
checking for XML_ParserCreate in -lexpat... yes
checking for iconv in -lc... yes
checking for deflateBound in -lz... yes
checking for socket in -lc... yes
checking for hstrerror in -lc... yes
checking for basename in -lc... yes
configure: CHECKS for header files
checking how to run the C preprocessor... cc -E
checking for grep that handles long lines and -e... /bin/grep
checking for egrep... /bin/grep -E
checking for ANSI C header files... yes
checking for sys/types.h... yes
checking for sys/stat.h... yes
checking for stdlib.h... yes
checking for string.h... yes
checking for memory.h... yes
checking for strings.h... yes
checking for inttypes.h... yes
checking for stdint.h... yes
checking for unistd.h... yes
checking sys/select.h usability... yes
checking sys/select.h presence... yes
checking for sys/select.h... yes
checking for old iconv()... no
configure: CHECKS for typedefs, structures, and compiler characteristics
checking for struct dirent.d_ino... yes
checking for struct dirent.d_type... yes
checking for struct sockaddr_storage... yes
checking for struct addrinfo... yes
checking for getaddrinfo... yes
checking for library containing getaddrinfo... none required
checking whether formatted IO functions support C99 size specifiers... yes
checking whether system succeeds to read fopen'ed directory... no
checking whether snprintf() and/or vsnprintf() return bogus value... no
configure: CHECKS for library functions
checking libgen.h usability... yes
checking libgen.h presence... yes
checking for libgen.h... yes
checking for strcasestr... yes
checking for library containing strcasestr... none required
checking for memmem... yes
checking for library containing memmem... none required
checking for strlcpy... no
checking for uintmax_t... yes
checking for strtoumax... yes
checking for library containing strtoumax... none required
checking for setenv... yes
checking for library containing setenv... none required
checking for unsetenv... yes
checking for library containing unsetenv... none required
checking for mkdtemp... yes
checking for library containing mkdtemp... none required
checking for mkstemps... no
checking Checking for POSIX Threads with '-pthread'... yes
configure: creating ./config.status
config.status: error: cannot find input file: config.mak.in
$ make

Now it compiles everything.

^ 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