* Re: [PATCH 2/3] add new Git::Repo API
From: Lea Wiemann @ 2008-07-14 1:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, John Hawley, Jakub Narebski, Petr Baudis
In-Reply-To: <7vmyklpiqk.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
> This probably is something the person who mentors you as a GSoC student
> should tell you, and shouldn't come from me,
No, thanks for telling me. I'm actually keeping John quite busy with
the server side on kernel.org, and wouldn't currently expect him to look
at the patches I'm sending.
> I would have preferred to see the first paragraph of yours stated more
> diplomaticly; it sounds rather unilateral ultimatum to me.
Apologies; I was trying to hold off developers who'd see duplicate
functionality and run off at me screaming "unacceptable! impure!!one".
I should've phrased it more carefully.
> This patch was not signed-off, but I assume it was simply forgotten
Yup. In the unlikely case it gets applied as-is, feel free to add it.
(I'll reply to Jakub's email shortly, unless I fall asleep in the
meantime. ^^)
^ permalink raw reply
* [PATCH] bash completion: Append space after file names have been completed
From: Shawn O. Pearce @ 2008-07-14 0:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
When completing `git show origin/maint:Makef<tab>` we should add a
space after the filename has been completed, so that the user can
immediately begin the next argument.
I also added a special case for the symlink variant so we treat it
just like a normal blob, as there are no items below it in the Git
tree structure.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
contrib/completion/git-completion.bash | 10 +++++++++-
1 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 9a1c66a..790d1c6 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -294,9 +294,17 @@ __git_complete_file ()
ls="$ref"
;;
esac
+ local IFS=$'\n'
COMPREPLY=($(compgen -P "$pfx" \
-W "$(git --git-dir="$(__gitdir)" ls-tree "$ls" \
- | sed '/^100... blob /s,^.* ,,
+ | sed '/^100... blob /{
+ s,^.* ,,
+ s,$, ,
+ }
+ /^120000 blob /{
+ s,^.* ,,
+ s,$, ,
+ }
/^040000 tree /{
s,^.* ,,
s,$,/,
--
1.5.6.2.393.g45096
^ permalink raw reply related
* Re: [PATCH] Added --export option to git-send-email.
From: Eduard - Gabriel Munteanu @ 2008-07-14 0:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Avery Pennarun, ryan, git
In-Reply-To: <7vlk05o14i.fsf@gitster.siamese.dyndns.org>
On Sun, 13 Jul 2008 15:44:29 -0700
Junio C Hamano <gitster@pobox.com> wrote:
> However, strictly speaking,(2) may break the standard workflow of
> generating patches with format-patch and feeding the result to
> send-email, as format-patch does not do /^From / munging (and it
> shouldn't). The issue is mostly theoretical --- it only matters if
> your commit log has a line that begins with ">From ", and people who
> have worked with e-mail for any nontrivial period have already
> learned to reword their sentences to avoid lines that begin with
> "From " (or ">From ", for that matter) anyway.
Munging happens only when doing --export. If you don't supply that
option, it won't affect anything, so there is no breakage.
BTW, git-format-patch already produces output somewhat compatible with
mbox, as it adds a From_ line.
Eduard
^ permalink raw reply
* Re: [PATCH] bash completion: Fix the . -> .. revision range completion
From: Shawn O. Pearce @ 2008-07-14 0:00 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Junio C Hamano, Petr Baudis, git
In-Reply-To: <alpine.LFD.1.10.0807131649380.3305@woody.linux-foundation.org>
Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Sun, 13 Jul 2008, Junio C Hamano wrote:
> >
> > I think that is what Shawn sent a few minutes ago, so you two are in
> > agreement, and I will be happy with it, too.
>
> Does it fix this one too:
>
> git show origin/pu:Makef<tab>
>
> which totally screws up and becomes
>
> git show Makefile
>
> dropping the version specifier?
Its unrelated to the issue you described above. But I just tested
your Makefile completion case and it worked as expected, though
I just found out it doesn't add a trailing space once there is a
unique completion made. I'll look at another patch to fix that.
--
Shawn.
^ permalink raw reply
* Re: [PATCH] Added --export option to git-send-email.
From: Eduard - Gabriel Munteanu @ 2008-07-13 23:58 UTC (permalink / raw)
To: Avery Pennarun; +Cc: Junio C Hamano, ryan, git
In-Reply-To: <32541b130807131642k1382cf84gdf5e8bb8d2ff4ffe@mail.gmail.com>
On Sun, 13 Jul 2008 19:42:35 -0400
"Avery Pennarun" <apenwarr@gmail.com> wrote:
> On 7/13/08, Junio C Hamano <gitster@pobox.com> wrote:
> > If this additional option claims to produce a mbox, I think:
> >
> > (1) quoting only /^From / (not /^>*From/) to be consistent with
> > the standard practice is the right thing to do; and
> >
> > (2) reading side might need to also pay attention to /^>From /,
> > in case somebody feeds an output from this option back to
> > send-email.
> >
> > However, strictly speaking,(2) may break the standard workflow of
> > generating patches with format-patch and feeding the result to
> > send-email, as format-patch does not do /^From / munging (and it
> > shouldn't).
>
> Note that it's generally very bad practice to do (2) unless you always
> quote /^>*From/. Quoting only /^From/ and *then* trying to dequote it
> correctly actually increases the number of places where you can
> corrupt a message. As a sign that very few programs do (2), I think
> it's pretty clear that a lot more people see "From" rewritten as
> ">From" in their mail app of choice than the reverse.
Junio is right when he says we should match against "From " instead of
"From" (i.e. we should not touch lines beginning with "Fromage" for
example). Should I resubmit or can it be corrected during the merge?
> There is also some debate about what "standard practice" means. See:
> http://homepages.tesco.net/J.deBoynePollard/FGA/mail-mbox-formats.html
> .
When I started working on this, I visited Wikipedia which provided a
link to qmail's site, which looks quite authoritative on this matter:
http://www.qmail.org/man/man5/mbox.html
I just followed that spec.
> If git is going to start actually producing mbox files (as opposed to
> just individual messages as it does now), it should probably
> explicitly take a stance on the issue... or perhaps make it
> configurable.
>
> Have fun,
>
> Avery
^ permalink raw reply
* Re: [PATCH] bash completion: Fix the . -> .. revision range completion
From: Linus Torvalds @ 2008-07-13 23:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Petr Baudis, Shawn O. Pearce, git
In-Reply-To: <7vhcatnz80.fsf@gitster.siamese.dyndns.org>
On Sun, 13 Jul 2008, Junio C Hamano wrote:
>
> I think that is what Shawn sent a few minutes ago, so you two are in
> agreement, and I will be happy with it, too.
Does it fix this one too:
git show origin/pu:Makef<tab>
which totally screws up and becomes
git show Makefile
dropping the version specifier?
I don't speak completion-script, so somebody else would have to fix this
really annoying bug (*)
Linus
(*) Ok, so it's rare. I have been bitten by it something like twice in the
last months. But when it happens it's really annoying.
^ permalink raw reply
* Re: [PATCH] Added --export option to git-send-email.
From: Avery Pennarun @ 2008-07-13 23:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Eduard - Gabriel Munteanu, ryan, git
In-Reply-To: <7vlk05o14i.fsf@gitster.siamese.dyndns.org>
On 7/13/08, Junio C Hamano <gitster@pobox.com> wrote:
> If this additional option claims to produce a mbox, I think:
>
> (1) quoting only /^From / (not /^>*From/) to be consistent with the
> standard practice is the right thing to do; and
>
> (2) reading side might need to also pay attention to /^>From /, in case
> somebody feeds an output from this option back to send-email.
>
> However, strictly speaking,(2) may break the standard workflow of
> generating patches with format-patch and feeding the result to send-email,
> as format-patch does not do /^From / munging (and it shouldn't).
Note that it's generally very bad practice to do (2) unless you always
quote /^>*From/. Quoting only /^From/ and *then* trying to dequote it
correctly actually increases the number of places where you can
corrupt a message. As a sign that very few programs do (2), I think
it's pretty clear that a lot more people see "From" rewritten as
">From" in their mail app of choice than the reverse.
There is also some debate about what "standard practice" means. See:
http://homepages.tesco.net/J.deBoynePollard/FGA/mail-mbox-formats.html
.
If git is going to start actually producing mbox files (as opposed to
just individual messages as it does now), it should probably
explicitly take a stance on the issue... or perhaps make it
configurable.
Have fun,
Avery
^ permalink raw reply
* Re: [PATCH 2/3] add new Git::Repo API
From: Jakub Narebski @ 2008-07-13 23:28 UTC (permalink / raw)
To: Lea Wiemann; +Cc: git, John Hawley, Petr Baudis
In-Reply-To: <1215738665-5153-1-git-send-email-LeWiemann@gmail.com>
On Fri, 11 July 2008 03:11, Lea Wiemann napisał:
> This also adds the Git::Commit and Git::Tag classes, which are used by
> Git::Repo, the Git::Object base class, and the Git::RepoRoot helper
> factory class.
I think it would be perhaps better to explain relationship and purpose
of each class in more detail, including Git::Repo.
No signoff - is it deliberate, or just omission?
> ---
> Here's some elaboration on why I didn't use or extend Git.pm.
>
> Please note before starting a reply to this: This is not an argument;
> I'm just explaining why I implemented it the way I did. So please
> don't try to argue with me about what I should or should have done.
> I'm not going to refactor Git::Repo to use Git.pm or vice versa; it's
> really a much more non-trivial task than you might think at first
> glance.
>
> Anyways, the following bullet points are my reasons for not extending
> Git.pm:
>
> - Git.pm doesn't do what I want: It's designed to provide access to
> working copies. Extending it to have more repository-access
> functions might have resulted in a mess.
I don't quite understand. True, Git->repository(...) has a bit strange
[optional] parameters, but you should remember that IIRC it predates
possibility of workdir / worktree / working copy separation. That
I think is why it supports non-bare repositories (default), and bare
repositories, not separating working copy from repository.
Also, from what I understand and remember, Git.pm was created to have
one place, one single implementation for safe and operating system
independent (which means that it works with ActiveState Perl on
Windows) "pipe" and "backticks". Everything else is just convenience.
> Some long-term thoughts on this issue: I don't think Git.pm's design
> approach is sensible: It tries to do (a) WC access, (b) repo access,
> and (c) frontend error handling (with sensible error messages).
I can see (b) and (c), but I have trouble seeing (a).
> Those things should really be separated; e.g. one could write a
> Git::WC class that *has* a Git::Repo instance (since every working
> copy has a repository associated with it); so you can use $wc =
> Git::WC->new to access the working copy, and $wc->repo to access its
> repository.
Please remember that the opposite relation is also true. Non-bare
repository has working copy / worktree associated with it, either
implicitly (.git/.. is working copy), or explicitly (core.worktree).
Also even if worktree and repository are separated, there are a few
files in non-bare repository which refer to worktree condition, namely
$GIT_DIR/HEAD, $GIT_DIR/logs/HEAD and $GIT_DIR/index.
> Git.pm will obviously have to stay since a few git
> commands use it,
The following commands use Git.pm: contrib/examples/git-remote.perl,
git-add--interactive.perl (helper script), git-cvsexportcommit, git-svn,
git-send-email.
The following commands are written in Perl, but *do not* use Git.pm:
git-archimport (which probably should go to contrib, and be replaced by
fast-import / fast-export based Bazaar import), git-cvsimport,
git-cvsserver, git-relink (rarely used now, I think).
> but I'd probably let it die a slow death, and (cleanly!) copy
> functionality to a Git::WC module (and perhaps a Git::Frontend
> module) in the long run.
I'm not so sure if it is a way to go. Most git commands wants to just
invoke other git commands safely, either creating pipeline, or reading
whole input; sometimes command output is not important.
Non OO things, like ability to write
print color('reset') . "\n";
is also important.
> - It would have needed refactoring and more features.
>
> - For example, the Error module should be removed (and normal die
> and eval should be used instead). At some point when I was trying
> to debug it through gitweb, Git.pm would error out somewhere and I
> would randomly get either a blank page (usually) or an error page
> (rarely). I suspect that this is because of the Error module or
> some strange interaction with another.
I'm not sure if using Error module and syntactic sugar in the form of
try { ... } catch <class> with { ... } was a good idea for (c),
frontend error handling. Or if it was a good idea (the explanation in
http://www.perl.com/pub/a/2002/11/14/exception.html is compelling)
if it was implemented fully, completely, and correctly.
How would you like to catch errors from frontend in Git::Repo and
friends?
> [...] (Besides, Error is not in
> the Perl distribution, so it would be an unnecessary dependency
> for gitweb; the only one as far as I can see.)
Git Makefile installs Error module from local copy if one is not
available. perl-Git module (which would be unfortunately required to
use gitweb; well, that or you could install it locally) has
"perl(Error)" as one of dependencies. perl-Error module is a standard
module and I think it can be found in any modern Linux distribution; if
not, see first sentence in this paragraph.
> - Also, I needed something like max_exit_code and a custom path to
> the git binary; adding max_exit_code would have been non-trivial.
What is max_exit_code (and why for example you couldn't just create your
own derivative of Error)?
> Now I'm all in favor of re-using existing code, but refactoring
> Git.pm would have taken *much* longer than simply writing a new
> module. I'm working on caching for gitweb, not on implementing the
> next great Perl API for Git. (And Git::Repo isn't great, FTR.)
Well, I can understand that.
Besides, having more than one implementation of some new feature was
quite common in early stages of feature development. See for example
wit and gitweb, git-applymbox and git-am, git-annotate and git-blame.
> - It's buggy and untested. Neither of these is a problem by itself,
> but the combination is deadly.
Haven't you added t/t9700-perl-git.sh?
> E.g. I was trying to refactor the
> 'repository' constructor (to be able to do instantaneous
> instantiation) and stumbled upon a missing semicolon that rendered
> the surrounding code syntactically correct but obviously buggy (line
> 214 on master). Adding a semicolon there seemed to cause other
> errors, and given that (a) I don't understand what the code does and
> no test or comment tells me what it should, and (b) it doesn't work
> (or maybe it half-works?), I lost all my confidence that I could do
> anything resembling a (behavior-preserving) refactoring on that
> code.
This is not that the code should not be rewritten... but git-blame (and
git log -S, aka pickaxe search) can be created to find who is the
author of said code, and ask him via email (probably Cc-ing git mailing
list).
> I might have been able to work with this particular problem, but
> such a problem (buggy and completely untested code) are indicative
> that the rest of the code might bear similar surprises.
I think that the pipe and command code might be tested best, as it is
collation and condensation of many different "safe pipe" (etc.) code
fragments.
> - It's overly lengthy, and it's a lot of code for not much
> functionality. Git.pm has 1200 LOC, with a large stack of tiny
> subroutines to handle pipes, and a complicated error handling
> system. I can make out ~400 lines of actual Git functionality, plus
> a 100-lines constructor (all including documentation). The part of
> Git::Repo that overlaps with Git.pm (in terms of duplicate
> functionality) seems to be in the range of 150-200 lines, and it's
> mostly pipe-handling. That's not a whole lot.
Pipe handling is IMHO most important part. Well, other parts such as
color(...) are important too, but not for gitweb.
> - When I decided I didn't want to use Git.pm, it took me 2-3 hours to
> replicate the functionality in Git.pm that I needed; it would have
> taken me *much* longer to extend Git.pm to do what I want. Again,
> I'm not working on the next great Git Perl API.
>
> So where do we go with Git.pm and Git::Repo? I would suggest that
> they both stay.
>
> Thus we'd have two APIs (both of them obviously incomplete). If
> there's enough objection to having two competing official APIs, I'll
> be happy to move Git::Repo to the gitweb directory, and make it a
> gitweb-specific thing (though it's not gitweb-specific at all in terms
> of functionality); I really don't care, as my goal is to get caching
> for gitweb working. Again, if you're bothered by the fact that
> there's duplicate functionality at all, please don't complain, but
> send patches to reconcile the APIs; I will not take care of that
> myself, since it's a very much non-trivial task.
What I worry about is that dependence on Git.pm or Git::Repo would make
gitweb installation too hard for some.
> Unrelatedly, should I add copyright notices at the bottom of each perl
> module so they are displayed in the perldoc/man pages? I'm not
> generally a fan of such notices, since they tend to establish code
> ownership, but if it's desired I can add them.
Well, most manpages have information about who made them... which means
who was initial author, usually, and/or who is current maintainer.
[Comments on patch itself in separate email, later]
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH] bash completion: Fix the . -> .. revision range completion
From: Junio C Hamano @ 2008-07-13 23:25 UTC (permalink / raw)
To: Petr Baudis; +Cc: Shawn O. Pearce, git
In-Reply-To: <20080713230724.GJ10151@machine.or.cz>
Petr Baudis <pasky@suse.cz> writes:
> On Sun, Jul 13, 2008 at 02:38:37PM -0700, Junio C Hamano wrote:
>> By the way, the above command line is another "dot" related frustration I
>> always have. If you try:
>>
>> git log v1.5.6.<TAB>
>>
>> the completion code adds a dot unconditionally when I want to choose from
>> the list of v1.5.6.X tags. Of course, I can work this around by dropping
>> the last dot before asking for completion, so it is not really a very big
>> deal, but I mention it here because this annoyance is exactly in the same
>> league as your "git-submodule.<TAB>" example.
>
> Actually, my original solution to this problem was simply to remove the
> . -> .. completion altogether. Maybe this would still be the best course
> of action? I don't think the . -> .. is actually very useful for anyone,
> since you might as well just hit the dot another time instead of a tab.
I think that is what Shawn sent a few minutes ago, so you two are in
agreement, and I will be happy with it, too.
^ permalink raw reply
* Re: [PATCH] bash completion: Fix the . -> .. revision range completion
From: Petr Baudis @ 2008-07-13 23:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Shawn O. Pearce, git
In-Reply-To: <7vskudpiqq.fsf@gitster.siamese.dyndns.org>
On Sun, Jul 13, 2008 at 02:38:37PM -0700, Junio C Hamano wrote:
> By the way, the above command line is another "dot" related frustration I
> always have. If you try:
>
> git log v1.5.6.<TAB>
>
> the completion code adds a dot unconditionally when I want to choose from
> the list of v1.5.6.X tags. Of course, I can work this around by dropping
> the last dot before asking for completion, so it is not really a very big
> deal, but I mention it here because this annoyance is exactly in the same
> league as your "git-submodule.<TAB>" example.
Actually, my original solution to this problem was simply to remove the
. -> .. completion altogether. Maybe this would still be the best course
of action? I don't think the . -> .. is actually very useful for anyone,
since you might as well just hit the dot another time instead of a tab.
> > contrib/completion/git-completion.bash | 7 ++++++-
> > 1 files changed, 6 insertions(+), 1 deletions(-)
> >
> > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> > index 61581fe..fe24b8c 100755
> > --- a/contrib/completion/git-completion.bash
> > +++ b/contrib/completion/git-completion.bash
> > @@ -325,7 +325,12 @@ __git_complete_revlist ()
> > __gitcomp "$(__git_refs)" "$pfx" "$cur"
> > ;;
> > *.)
> > - __gitcomp "$cur."
> > + if ls "$cur"* >/dev/null 2>&1; then
>
> There is a slight Yuck factor for using "ls" here but I do not think of a
> better alternative offhand.
I have thought about this hard for some time, but couldn't come up with
anything better than this or
(shopt -s nullglob; completion=("$cur"*); [ -n "$completion" ])
which looks quite awful (and can waste a lot of memory in case of some
really insane completion).
--
Petr "Pasky" Baudis
GNU, n. An animal of South Africa, which in its domesticated state
resembles a horse, a buffalo and a stag. In its wild condition it is
something like a thunderbolt, an earthquake and a cyclone. -- A. Pierce
^ permalink raw reply
* Re: [PATCH] Added --export option to git-send-email.
From: Junio C Hamano @ 2008-07-13 22:44 UTC (permalink / raw)
To: Avery Pennarun; +Cc: Eduard - Gabriel Munteanu, ryan, git
In-Reply-To: <32541b130807131521w79cbf23m5934678e68312798@mail.gmail.com>
"Avery Pennarun" <apenwarr@gmail.com> writes:
> And it's the MDA's job to munge it, not the MTA, so the
> lines should never be munged while in transit *to* ...
Yeah, you're right. Sending side should not have any issues like that.
My receiving end (I think fetchmail slurps from my ISP and drops mails in
my local mbox, but it's a long time ago I configured my system...) does
quote only the "From " at the beginning, which is consistent with the
traditional mbox behaviour, so I see one, one, two, three in my local
mbox, and zero, one, two, three on gmane news article.
If this additional option claims to produce a mbox, I think:
(1) quoting only /^From / (not /^>*From/) to be consistent with the
standard practice is the right thing to do; and
(2) reading side might need to also pay attention to /^>From /, in case
somebody feeds an output from this option back to send-email.
However, strictly speaking,(2) may break the standard workflow of
generating patches with format-patch and feeding the result to send-email,
as format-patch does not do /^From / munging (and it shouldn't). The
issue is mostly theoretical --- it only matters if your commit log has a
line that begins with ">From ", and people who have worked with e-mail for
any nontrivial period have already learned to reword their sentences to
avoid lines that begin with "From " (or ">From ", for that matter) anyway.
^ permalink raw reply
* Re: [PATCH] Added --export option to git-send-email.
From: Avery Pennarun @ 2008-07-13 22:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Eduard - Gabriel Munteanu, ryan, git
In-Reply-To: <7vvdz9o2wu.fsf@gitster.siamese.dyndns.org>
On 7/13/08, Junio C Hamano <gitster@pobox.com> wrote:
> Heh, so by that definition Gnus is not modern enough; neither is webmail
> interface at gmail.
>
> From which has originally zero level of >
> >From which has one
> >>From which has two
> >>>From which has three
>
> I am reasonably sure the above will be sent with one, one, two and three
> gt before "From". Is your mailer modern enough?
I read this message at gmail, and got zero, one, two, and three
dashes, respectively. For the sake of interest, I then connected to
gmail using IMAP from mutt, and the message was still intact. Then I
saved it to an mbox file using mutt, and it used Content-Length
instead of From munging, so it wasn't a very good experiment :)
If you're not actually delivering your mail using mbox format (as
opposed to imap, webmail, etc), it would be quite insulting for "From
" to be munged at all, so I'm happy at least that I didn't experience
that in my tests. On the other hand I'm surprised that gmail should
have corrupted it for you; gmail should be able to completely forget
about the concept of "From " munging since they don't use mbox
internally. (And it's the MDA's job to munge it, not the MTA, so the
lines should never be munged while in transit *to* gmail unless the
mailer app is extremely buggy.)
Have fun,
Avery
^ permalink raw reply
* Re: [PATCH] bash completion: Improve responsiveness of git-log completion
From: Ingo Molnar @ 2008-07-13 22:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Shawn O. Pearce, git, Petr Baudis
In-Reply-To: <7vy745piqy.fsf@gitster.siamese.dyndns.org>
* Junio C Hamano <gitster@pobox.com> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
>
> > Junio noticed the bash completion has been taking a long time lately....
>
> The credit actually goes to Ingo.
>
> > Petr Baudis tracked it down to 72e5e989b ("bash: Add space after
> > unique command name is completed."). Tracing the code showed
> > we spent significant time inside of this loop within __gitcomp,
> > due to the string copying overhead....
> > ...
> > Does this make things better? Or worse? I'm not seeing a huge
> > difference on my own system. Maybe its too fast these days...
>
> Ingo, I understand you have stopped using the completion long time ago
> due to this latency issue. Together with d773c63 (bash: offer only
> paths after '--', 2008-07-08) that already is in 'maint' and 'master',
> this hopefully would make the completion usable for you again?
yeah. I've checked out the latest version and applied Shawn's patch and
added the patched contrib/completion/git-completion.bash back to my
.bashrc and i'm not seeing the latencies anymore.
Thanks! Please consider this fixed - will follow up if there's any
problem left.
Ingo
^ permalink raw reply
* Re: git pull is slow
From: Shawn O. Pearce @ 2008-07-13 22:11 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Stephan Hennig, Nicolas Pitre, Andreas Ericsson, git
In-Reply-To: <alpine.DEB.1.00.0807131555560.4816@eeepc-johanness>
Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> On Sun, 13 Jul 2008, Shawn O. Pearce wrote:
> > Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > ...
> > > I expect this to touch the resolve_delta() function of index-pack.c in a
> > > major way, though.
> >
> > Yea, that's going to be ugly. The "cache" you speak of above is held on
> > the call stack as resolv_delta() recurses through the delta chain to
> > reconstruct objects and generate their SHA-1s. There isn't a way to
> > release these objects when memory gets low so your worst case scenario
> > is a 100M+ blob with a delta chain of 50 or more - that will take you 5G
> > of memory to pass through index-pack.
>
> Actually, there is...
>
> You would only need to tap into the "release_pack_memory()" mechanism,
> adding a sort of a "smart pointer" that knows how to reconstruct its
> contents.
I guess you don't really know how index-pack is organized than.
It is not quite that simple.
I think we want to use the core.deltabasecachelimit parameter inside
of index-pack to limit the size of the cache, but evict based on
the callstack depth. Objects deeper back in the callstack should
evict before objects closer to the top of the stack.
Reconstruction is a bit complex as we don't have the machinary in
sha1_file.c available to us, as the index is not available, as we
are still in the process of creating the damn thing.
I'm working up a patch series to resolve this. I'm going to shutup
now and just try to post working code sometime this evening.
--
Shawn.
^ permalink raw reply
* Re: [PATCH] bash completion: Fix the . -> .. revision range completion
From: Shawn O. Pearce @ 2008-07-13 22:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Petr Baudis, git
In-Reply-To: <7vskudpiqq.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> wrote:
> Petr Baudis <pasky@suse.cz> writes:
> > When Git sees a string with trailing dot on a place where revision
> > range could occur, it will unconditionally append another dot to
...
> > git log git-submodule.<tab>
...
> By the way, the above command line is another "dot" related frustration I
> always have. If you try:
>
> git log v1.5.6.<TAB>
>
> the completion code adds a dot unconditionally when I want to choose from
> the list of v1.5.6.X tags.
This annoys me too Junio. Its a bug I just never got around to fixing.
Based on Petr's comments and yours I'm wondering if this is just not the
better patch to apply here:
--8<--
bash completion: Don't offer "a.." as a completion for "a."
If the user is trying to complete "v1.5.3.<tab>" to see all of
the available maintenance releases for 1.5.3 we should not give
them an extra dot as the completion. Instead if the user wants
a ".." or a "..." operator they should key the two dots out on
their own. Its the same number of keystrokes either way.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
contrib/completion/git-completion.bash | 3 ---
1 files changed, 0 insertions(+), 3 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 61581fe..9a1c66a 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -324,9 +324,6 @@ __git_complete_revlist ()
cur="${cur#*..}"
__gitcomp "$(__git_refs)" "$pfx" "$cur"
;;
- *.)
- __gitcomp "$cur."
- ;;
*)
__gitcomp "$(__git_refs)"
;;
--
1.5.6.2.393.g45096
--
Shawn.
^ permalink raw reply related
* Re: [PATCH] Added --export option to git-send-email.
From: Junio C Hamano @ 2008-07-13 22:05 UTC (permalink / raw)
To: Avery Pennarun; +Cc: Eduard - Gabriel Munteanu, ryan, git
In-Reply-To: <32541b130807131444s5e9ea0d6v9610dd5871467fc9@mail.gmail.com>
"Avery Pennarun" <apenwarr@gmail.com> writes:
> So by encoding it this way, someone with a modern mailer could...
Heh, so by that definition Gnus is not modern enough; neither is webmail
interface at gmail.
>From which has originally zero level of >
>From which has one
>>From which has two
>>>From which has three
I am reasonably sure the above will be sent with one, one, two and three
gt before "From". Is your mailer modern enough?
^ permalink raw reply
* Re: [PATCH] gitk - work around stderr redirection on Cygwin
From: Mark Levedahl @ 2008-07-13 21:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: paulus, git, Eric Blake
In-Reply-To: <7vfxqdqxh7.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
> Mark Levedahl <mlevedahl@gmail.com> writes:
>
>
>> Mark Levedahl wrote:
>>
>>> Cygwin is *still* shipping with antiquated Tcl 8.4.1, and will continue
>>> to do so for the indefinite future. This version does not understand
>>> the "2>@1" redirection syntax, so such redirection of stderr must be
>>> done using sh.
>>>
>> Ping. This bug is in 1.5.6.x, and thus also in the current Cygwin git
>> release: as a result, several gitk context menu items cause
>> errors. (Let me know if I should resend the patch).
>>
>
> In the meantime, is it an option to apply this as a port specific patch
> when Cygwin and whichever distribution ship with old Tcl package their
> binary releases?
>
> Just checking how urgent this issue is (8.4.1 was from Oct 2002 if I am
> not mistaken) for maintainers of Cygwin port, and I am suspecting that
> kernel folks are somewhat busy near/around OLS/KS timeframe (but not me).
>
>
(I didn't trim the message as I've copied the Cygwin git maintainer).
I certainly have this patch in my tree so the folks I supply git to who
use Cygwin have this patch. The question of whether to maintain this out
of tree for the official Cygwin release is up to Eric.
Mark
^ permalink raw reply
* Re: [PATCH] Added --export option to git-send-email.
From: Avery Pennarun @ 2008-07-13 21:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Eduard - Gabriel Munteanu, ryan, git
In-Reply-To: <7vbq11qxfb.fsf@gitster.siamese.dyndns.org>
On 7/13/08, Junio C Hamano <gitster@pobox.com> wrote:
> >> Sorry, but I am scratching my head here. Shouldn't this "bare From
> >> munging" be like this instead?
> >>
> >> $message =~ s/^From />From /m;
> >
> > That wouldn't be a reversible operation.
>
> So what? Adding Cc: and stuff send-email does are already irreversible
> anyway.
At one point the people who write mailers realized that just munging
"From" to ">From" when putting things into an mbox was irreversible,
ie, there was no way to later display the contents of the mbox without
showing ">From" where "From" was intended.
However, if you always insert _another_ ">" whenever "^>*From" is
detected, then you can also always do the opposite upon displaying to
the user, and the message is effectively not munged. It becomes
simply a part of the mbox file encoding.
So by encoding it this way, someone with a modern mailer could view
the resulting mbox with all the munged text de-mungled. Someone
without a modern mailer would see munging anyhow, so they're no worse
off.
Have fun,
Avery
^ permalink raw reply
* Re: [PATCH 2/3] add new Git::Repo API
From: Junio C Hamano @ 2008-07-13 21:38 UTC (permalink / raw)
To: Lea Wiemann; +Cc: git, John Hawley, Jakub Narebski, Petr Baudis
In-Reply-To: <1215738665-5153-1-git-send-email-LeWiemann@gmail.com>
Lea Wiemann <lewiemann@gmail.com> writes:
> Here's some elaboration on why I didn't use or extend Git.pm.
>
> Please note before starting a reply to this: This is not an argument;
> I'm just explaining why I implemented it the way I did. So please
> don't try to argue with me about what I should or should have done....
> ...
> So where do we go with Git.pm and Git::Repo? I would suggest that
> they both stay.
This probably is something the person who mentors you as a GSoC student
should tell you, and shouldn't come from me, but I am going to mention
it anyway.
I thought GSoC program was about giving participants a chance to acquire
skills to work better with open source community, and I firmly believe
that a major component of that skillset is about working with people,
probably even more so than about working with code. Of course you need
have a certain basic coding skills and disciplines, but it is clear to
everybody that you are beyond that threashold already. I would have
preferred to see the first paragraph of yours stated more diplomaticly;
it sounds rather unilateral ultimatum to me.
Having said that, I agree with your reasoning. People who care enough can
help refactoring Git.pm to build on top of Git::Repo (this would include
adding missing Git::WC and possibly others), but I agree that would
largely be outside of the scope of gitweb caching.
This patch was not signed-off, but I assume it was simply forgotten and
not deliberate?
^ permalink raw reply
* Re: [PATCH] bash completion: Fix the . -> .. revision range completion
From: Junio C Hamano @ 2008-07-13 21:38 UTC (permalink / raw)
To: Petr Baudis; +Cc: Shawn O. Pearce, git
In-Reply-To: <20080713111847.29801.8969.stgit@localhost>
Petr Baudis <pasky@suse.cz> writes:
> When Git sees a string with trailing dot on a place where revision
> range could occur, it will unconditionally append another dot to
> it to help complete a revision range. However, filespec can usually
> occur at such a place as well. I have been hitting this all the time
> lately with
>
> git log git-submodule.<tab>
>
> and the like.
Modulo s/Git/bash-completion/ ;-) I think this makes sense.
> This patch will make Git perform the . -> .. completion in
> __git_complete_revlist only if there is no filename starting with
> the entered prefix available. At few places, filename could not occur
> when calling __git_complete_revlist; however, taking this into account
> did not seem worth complicating the code further.
Theoretically we could take a hint from presense of '--' like d773c63
(bash: offer only paths after '--', 2008-07-08) did. If the command line
has double-dash and the token we are looking at is before it, it cannot be
pathname and this check does not have to trigger. But I agree that is not
worth it, because this "theoretical" solution would mean that the user
needs to something awkward like:
git log v1.5.6. --<C-b><C-b><C-b><TAB>
to take advantage of it.
By the way, the above command line is another "dot" related frustration I
always have. If you try:
git log v1.5.6.<TAB>
the completion code adds a dot unconditionally when I want to choose from
the list of v1.5.6.X tags. Of course, I can work this around by dropping
the last dot before asking for completion, so it is not really a very big
deal, but I mention it here because this annoyance is exactly in the same
league as your "git-submodule.<TAB>" example.
"git show v1.5.6.<TAB>" does complete as expected, which is understandable
(the command does not take range, and completion knows about it -- which
is quite nice).
> contrib/completion/git-completion.bash | 7 ++++++-
> 1 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 61581fe..fe24b8c 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -325,7 +325,12 @@ __git_complete_revlist ()
> __gitcomp "$(__git_refs)" "$pfx" "$cur"
> ;;
> *.)
> - __gitcomp "$cur."
> + if ls "$cur"* >/dev/null 2>&1; then
There is a slight Yuck factor for using "ls" here but I do not think of a
better alternative offhand.
Will queue on top of Shawn's previous one. Thanks.
^ permalink raw reply
* Re: [PATCH] bash completion: Improve responsiveness of git-log completion
From: Junio C Hamano @ 2008-07-13 21:38 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Petr Baudis, Ingo Molnar
In-Reply-To: <20080713023742.GA31760@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
> Junio noticed the bash completion has been taking a long time lately....
The credit actually goes to Ingo.
> Petr Baudis tracked it down to 72e5e989b ("bash: Add space after
> unique command name is completed."). Tracing the code showed
> we spent significant time inside of this loop within __gitcomp,
> due to the string copying overhead....
> ...
> Does this make things better? Or worse? I'm not seeing a huge
> difference on my own system. Maybe its too fast these days...
Ingo, I understand you have stopped using the completion long time ago due
to this latency issue. Together with d773c63 (bash: offer only paths
after '--', 2008-07-08) that already is in 'maint' and 'master', this
hopefully would make the completion usable for you again?
^ permalink raw reply
* Re: [PATCH] git-mailinfo: use strbuf's instead of fixed buffers
From: Junio C Hamano @ 2008-07-13 21:37 UTC (permalink / raw)
To: Lukas Sandström; +Cc: Git Mailing List
In-Reply-To: <487A49B4.6000407@etek.chalmers.se>
Lukas Sandström <lukass@etek.chalmers.se> writes:
> +static void parse_bogus_from(const struct strbuf *line)
> {
> ...
> +static void handle_from(const struct strbuf *from)
> {
> ...
> if (!at)
> - return bogus_from(line);
> + return parse_bogus_from(from);
That's GCCism isn't it? I'll queue this probably for 'pu' with local
fixups, so no need to resend for this particular issue, though.
^ permalink raw reply
* Re: [PATCH] Added --export option to git-send-email.
From: Junio C Hamano @ 2008-07-13 21:36 UTC (permalink / raw)
To: Avery Pennarun; +Cc: Junio C Hamano, Eduard - Gabriel Munteanu, ryan, git
In-Reply-To: <32541b130807131432j78e5100dyea20893268321466@mail.gmail.com>
"Avery Pennarun" <apenwarr@gmail.com> writes:
> On 7/13/08, Junio C Hamano <gitster@pobox.com> wrote:
>> Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro> writes:
>> > + } elsif (defined $export_file) {
>> > + my $mbox_from = "From $raw_from $date";
>> > + $message =~ s/^(>{0,})From/>$1From/m;
>> > + print $out "$mbox_from\n$header\n$message";
>>
>> Sorry, but I am scratching my head here. Shouldn't this "bare From
>> munging" be like this instead?
>>
>> $message =~ s/^From />From /m;
>
> That wouldn't be a reversible operation.
So what? Adding Cc: and stuff send-email does are already irreversible
anyway.
^ permalink raw reply
* Re: [PATCH] gitk - work around stderr redirection on Cygwin
From: Junio C Hamano @ 2008-07-13 21:35 UTC (permalink / raw)
To: Mark Levedahl; +Cc: paulus, git
In-Reply-To: <487A6780.7030500@gmail.com>
Mark Levedahl <mlevedahl@gmail.com> writes:
> Mark Levedahl wrote:
>> Cygwin is *still* shipping with antiquated Tcl 8.4.1, and will continue
>> to do so for the indefinite future. This version does not understand
>> the "2>@1" redirection syntax, so such redirection of stderr must be
>> done using sh.
>
> Ping. This bug is in 1.5.6.x, and thus also in the current Cygwin git
> release: as a result, several gitk context menu items cause
> errors. (Let me know if I should resend the patch).
In the meantime, is it an option to apply this as a port specific patch
when Cygwin and whichever distribution ship with old Tcl package their
binary releases?
Just checking how urgent this issue is (8.4.1 was from Oct 2002 if I am
not mistaken) for maintainers of Cygwin port, and I am suspecting that
kernel folks are somewhat busy near/around OLS/KS timeframe (but not me).
^ permalink raw reply
* Re: [PATCH] Added --export option to git-send-email.
From: Avery Pennarun @ 2008-07-13 21:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Eduard - Gabriel Munteanu, ryan, git
In-Reply-To: <7vskudr11m.fsf@gitster.siamese.dyndns.org>
On 7/13/08, Junio C Hamano <gitster@pobox.com> wrote:
> Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro> writes:
> > + } elsif (defined $export_file) {
> > + my $mbox_from = "From $raw_from $date";
> > + $message =~ s/^(>{0,})From/>$1From/m;
> > + print $out "$mbox_from\n$header\n$message";
>
> Sorry, but I am scratching my head here. Shouldn't this "bare From
> munging" be like this instead?
>
> $message =~ s/^From />From /m;
That wouldn't be a reversible operation.
Have fun,
Avery
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox