* Re: [PATCH] Change 'Deltifying objects' to 'Delta compressing objects'
From: Linus Torvalds @ 2007-10-19 4:21 UTC (permalink / raw)
To: Jeff King; +Cc: Nicolas Pitre, Shawn O. Pearce, git
In-Reply-To: <20071019035647.GA18717@coredump.intra.peff.net>
On Thu, 18 Oct 2007, Jeff King wrote:
>
> As for a shortcut notation, what about allowing '..' notation inside a
> reflog. I.e., <ref>@{a..b} is the same as <ref>@{a}..<ref>@{b}? So you
> could perhaps do origin/master@{1..}?
I'd love it, but the way our current SHA1 parser works, I don't think it
can really do it.
Basically, we currently assume that a SHA1 expression always expands to a
*single* SHA1.
And then, on top of that SHA1 expression parser, we then have a totally
separate logic (which is *not* part of the SHA1 expression parser itself)
that handles the "a..b" and "a...b" cases.
In other words, all the magic "head@{xyz}" logic is all in sha1_name.c,
but that never handles any ranges at all.
And then the range handling is a separate thing in revision.c and
builtin-rev-parse.c.
So I think your syntax is wonderful, but it would require moving the
complex range handling into sha1_name.c, and would also require that file
to get a more complex interface (right now all the sha1_name.c routines
just take the "fill in this one SHA1 hash" approach, but ".." and "..."
means that you have multiple objects *and* you can mark one of them as
being "negated" etc..)
> I'm not sure if there are syntactic issues with parsing out the '..' (or
> '...') operator.
See above: I don't think we have syntax problems per se: it's just that
right now the "complex" parser (the one that knows about ^, ~, and @{x}
etc) simply cannot do anything but a single SHA1 due to internal interface
issues.
Linus
^ permalink raw reply
* [PATCH] Teach prune-packed to use the standard progress meter
From: Shawn O. Pearce @ 2007-10-19 4:11 UTC (permalink / raw)
To: Nicolas Pitre, Jeff King; +Cc: git
Rather than reimplementing the progress meter logic and always
showing 100 lines of output while pruning already packed objects
we now use a delayed progress meter and only show it if there are
enough objects to make us take a little while.
Most users won't see the message anymore as it usually doesn't take
very long to delete the already packed loose objects. This neatens
the output of a git-gc or git-repack execution, which is especially
important for a `git gc --auto` triggered from within another
command.
We perform the display_progress() call from within the very innermost
loop in case we spend more than 1 second within any single object
directory. This ensures that a progress_update event from the
timer will still trigger in a timely fashion and allow the user to
see the progress meter.
While I'm in here I changed the message to be more descriptive of
its actual task. "Deleting unused objects" is a little scary for
new users as they wonder where these unused objects came from and
how they should avoid them. Truth is these objects aren't unused
in the sense of what git-prune would call a dangling object, these
are used but are just duplicates of things we have already stored
in a packfile.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
On top of np/progress topic. I kicked around a few different
changes for the progress title but finally settled on this one.
Improvement suggestions welcome.
builtin-prune-packed.c | 15 +++++++++++----
1 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/builtin-prune-packed.c b/builtin-prune-packed.c
index 9777300..015c8bb 100644
--- a/builtin-prune-packed.c
+++ b/builtin-prune-packed.c
@@ -1,5 +1,6 @@
#include "builtin.h"
#include "cache.h"
+#include "progress.h"
static const char prune_packed_usage[] =
"git-prune-packed [-n] [-q]";
@@ -7,6 +8,8 @@ static const char prune_packed_usage[] =
#define DRY_RUN 01
#define VERBOSE 02
+static struct progress progress;
+
static void prune_dir(int i, DIR *dir, char *pathname, int len, int opts)
{
struct dirent *de;
@@ -23,6 +26,8 @@ static void prune_dir(int i, DIR *dir, char *pathname, int len, int opts)
if (!has_sha1_pack(sha1, NULL))
continue;
memcpy(pathname + len, de->d_name, 38);
+ if (opts == VERBOSE)
+ display_progress(&progress, i + 1);
if (opts & DRY_RUN)
printf("rm -f %s\n", pathname);
else if (unlink(pathname) < 0)
@@ -39,6 +44,11 @@ void prune_packed_objects(int opts)
const char *dir = get_object_directory();
int len = strlen(dir);
+ if (opts == VERBOSE)
+ start_progress_delay(&progress,
+ "Removing duplicate objects",
+ 256, 95, 2);
+
if (len > PATH_MAX - 42)
die("impossible object directory");
memcpy(pathname, dir, len);
@@ -49,16 +59,13 @@ void prune_packed_objects(int opts)
sprintf(pathname + len, "%02x/", i);
d = opendir(pathname);
- if (opts == VERBOSE && (d || i == 255))
- fprintf(stderr, "Removing unused objects %d%%...\015",
- ((i+1) * 100) / 256);
if (!d)
continue;
prune_dir(i, d, pathname, len + 3, opts);
closedir(d);
}
if (opts == VERBOSE)
- fprintf(stderr, "\nDone.\n");
+ stop_progress(&progress);
}
int cmd_prune_packed(int argc, const char **argv, const char *prefix)
--
1.5.3.4.1249.g895be
^ permalink raw reply related
* Re: [PATCH] Change 'Deltifying objects' to 'Delta compressing objects'
From: Nicolas Pitre @ 2007-10-19 4:02 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Jeff King, git
In-Reply-To: <20071019034501.GG14735@spearce.org>
On Thu, 18 Oct 2007, Shawn O. Pearce wrote:
> Nicolas Pitre <nico@cam.org> wrote:
> > And imagine that you see the progress moving slowly because the remote
> > server is a NSLU2, but it says 80%. Then you go for a coffee and the
> > progress says 20% when you return because it now has moved to a
> > different phase. Rather counter intuitive.
>
> Yea, I didn't consider that. That's where you need to show the
> number of steps and which one you are on, so the meter looks
> more like:
>
> Step 1/3: Counting objects: .... \r
> Step 2/4: Compressing objects: ... \r
> Step 3/3: Writing objects: .... \r
>
> only all smashed into one line of course, so only the most recent
> one is being displayed.
Yet you might not know in advance how many steps there'll be. You might
or might not have the deltification phase (I simply can't let that term
go...), pack indexing also have 1 or 2 steps, and if objects are
unpacked instead then you have only one step.
Given the asynchronous nature of the sideband messages, I think that
could only create messed up displays. Some messages are terminated with
\n and others with \r.
Nicolas
^ permalink raw reply
* Re: Proposed git mv behavioral change
From: Jeff King @ 2007-10-19 3:58 UTC (permalink / raw)
To: Michael Witten; +Cc: git
In-Reply-To: <C5184350-51A7-46B4-B0C9-E32F79214546@MIT.EDU>
On Thu, Oct 18, 2007 at 11:53:05PM -0400, Michael Witten wrote:
>> 3. You don't want to commit your changes, so you git-mv in the index
>> only without involving your dirty working tree file.
> That's exactly what the original poster wanted.
>
> It's not unreasonable.
I guess. I really think git-stash is your friend here. But you can still
do step (3) with git-update-index (I'll leave the exact details as an
exercise for the reader).
-Peff
^ permalink raw reply
* Re: [PATCH] Change 'Deltifying objects' to 'Delta compressing objects'
From: Jeff King @ 2007-10-19 3:56 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Shawn O. Pearce, git
In-Reply-To: <alpine.LFD.0.9999.0710182340550.19446@xanadu.home>
On Thu, Oct 18, 2007 at 11:50:39PM -0400, Nicolas Pitre wrote:
> Maybe we should have a shortcut notation for <ref>@{1}..<ref> instead?
> I end up using that all the time since the fetch result has long
> scrolled off the screen when I want to look at what was fetched.
I must confess to never using reflogs in that case...for some reason
they just never come to mind. But now that you and Shawn mention them,
there's really no reason to leave the hash..hash for the fetch.
As for a shortcut notation, what about allowing '..' notation inside a
reflog. I.e., <ref>@{a..b} is the same as <ref>@{a}..<ref>@{b}? So you
could perhaps do origin/master@{1..}?
I'm not sure if there are syntactic issues with parsing out the '..' (or
'...') operator.
> > although that URL is almost a line by itself. :)
> It is, therefore I'd skip "Fetching from " entirely.
Yes, I was tempted to suggest that. I think it might need some context,
depending on what comes right before it.
> That looks fine for a push. I'd say "remote foo -> local bar" for a
> fetch.
Agreed.
-Peff
^ permalink raw reply
* Re: Proposed git mv behavioral change
From: Michael Witten @ 2007-10-19 3:53 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20071019034704.GB11095@coredump.intra.peff.net>
On 18 Oct 2007, at 11:47:05 PM, Jeff King wrote:
> The only workflow I can think of is:
>
> 1. you modify a.c
> 2. your boss comes in and tells you to make some unrelated change,
> which involves moving a.c to b.c
> 3. You don't want to commit your changes, so you git-mv in the index
> only without involving your dirty working tree file.
> 4. You commit the index (which doesn't have your changes from (1)
>
> I think that is sort of a bogus workflow
Ha!
That's exactly what the original poster wanted.
It's not unreasonable.
^ permalink raw reply
* Re: [PATCH] Change 'Deltifying objects' to 'Delta compressing objects'
From: Nicolas Pitre @ 2007-10-19 3:50 UTC (permalink / raw)
To: Jeff King; +Cc: Shawn O. Pearce, git
In-Reply-To: <20071019033228.GA10697@coredump.intra.peff.net>
On Thu, 18 Oct 2007, Jeff King wrote:
> On Thu, Oct 18, 2007 at 11:24:41PM -0400, Nicolas Pitre wrote:
>
> > You usually get long lines that gets wrapped, so that means 3 lines of
> > screen space for one updated branches. Is the "66ffb04..4fa4d23"
> > information really useful? Might someone ever care?
>
> I have used it occasionally when tracking repos to see what new commits
> have happened. Usually I use a separate branch to mark "what I've seen"
> (i.e., fetch, gitk origin..master, pull), but if it's a branch that I'm
> not actively tracking, the display is useful.
Maybe we should have a shortcut notation for <ref>@{1}..<ref> instead?
I end up using that all the time since the fetch result has long
scrolled off the screen when I want to look at what was fetched.
Usully this llooks like:
git pull
git log @{1}..
But using origin/master@{1}..origin/master is a bit cumbersome.
> What is really useless in that line is the fact that _every_ ref is
> going to have the name of the remote, even though we only support
> fetching from one remote at a time. Perhaps something like:
>
> Fetching from git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6
> * refs/heads/origin: fast forward to branch 'master'
>
> although that URL is almost a line by itself. :)
It is, therefore I'd skip "Fetching from " entirely.
> Also, why do we abbreviate "refs/heads/master" from the remote, but we
> don't abbreviate refs/heads/origin for the local? Maybe something like:
>
> * local heads/origin -> remote heads/master (fast forward)
>
> or for separate remote
>
> * local remotes/origin/master -> remote heads/master (fast forward)
That looks fine for a push. I'd say "remote foo -> local bar" for a
fetch.
Nicolas
^ permalink raw reply
* Re: Proposed git mv behavioral change
From: Jeff King @ 2007-10-19 3:47 UTC (permalink / raw)
To: Michael Witten; +Cc: git
In-Reply-To: <93BF5798-F1C3-48EE-8233-A0F111BF8138@MIT.EDU>
On Thu, Oct 18, 2007 at 11:40:47PM -0400, Michael Witten wrote:
> Anyway, succinctly:
>
>> What you want to happen is the following:
>>
>> git show HEAD:A.txt > path/B.txt
>> git add path/B.txt
>> mv A.txt B.txt
>> git rm A.txt
>
> Better:
>
>> mv A.txt path/B.txt
>> Point the index entry for A.txt to path/B.txt
>
> I hope that's right.
Hrm. So you _do_ want to do an index-only move of A to B, in which case
the suggestion of a "git-mv --cached" seems sensible. Though I'm curious
why you want that. The only workflow I can think of is:
1. you modify a.c
2. your boss comes in and tells you to make some unrelated change,
which involves moving a.c to b.c
3. You don't want to commit your changes, so you git-mv in the index
only without involving your dirty working tree file.
4. You commit the index (which doesn't have your changes from (1)
I think that is sort of a bogus workflow, though, since you will never
have actually compiled or tested the changes in (2). You are much better
to git-stash your current work, fulfill the boss's request, then
unstash.
-Peff
^ permalink raw reply
* Re: [PATCH] Change 'Deltifying objects' to 'Delta compressing objects'
From: Shawn O. Pearce @ 2007-10-19 3:45 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Jeff King, git
In-Reply-To: <alpine.LFD.0.9999.0710182328580.19446@xanadu.home>
Nicolas Pitre <nico@cam.org> wrote:
> On Thu, 18 Oct 2007, Shawn O. Pearce wrote:
> > Yup. Your patches were a big improvement. But I'm now sitting here
> > wondering if we shouldn't just allow a progress meter to overwrite
> > the prior one. Then you only see the current task and progress,
> > or the final output if we have nothing further to say about that.
>
> And then you've lost some diagnostic clue (the absolute numbers) about
> the actual number of objects that were listed for "deltification" for
> example.
Leave the "Total" line. Add to it the number of objects we had to
consider for deltification as part of the packing.
> And imagine that you see the progress moving slowly because the remote
> server is a NSLU2, but it says 80%. Then you go for a coffee and the
> progress says 20% when you return because it now has moved to a
> different phase. Rather counter intuitive.
Yea, I didn't consider that. That's where you need to show the
number of steps and which one you are on, so the meter looks
more like:
Step 1/3: Counting objects: .... \r
Step 2/4: Compressing objects: ... \r
Step 3/3: Writing objects: .... \r
only all smashed into one line of course, so only the most recent
one is being displayed.
--
Shawn.
^ permalink raw reply
* Re: [PATCH] Change 'Deltifying objects' to 'Delta compressing objects'
From: Jeff King @ 2007-10-19 3:41 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Shawn O. Pearce, git
In-Reply-To: <alpine.LFD.0.9999.0710182328580.19446@xanadu.home>
On Thu, Oct 18, 2007 at 11:38:59PM -0400, Nicolas Pitre wrote:
> > Yup. Your patches were a big improvement. But I'm now sitting here
> > wondering if we shouldn't just allow a progress meter to overwrite
> > the prior one. Then you only see the current task and progress,
> > or the final output if we have nothing further to say about that.
>
> And then you've lost some diagnostic clue (the absolute numbers) about
> the actual number of objects that were listed for "deltification" for
> example.
>
> And imagine that you see the progress moving slowly because the remote
> server is a NSLU2, but it says 80%. Then you go for a coffee and the
> progress says 20% when you return because it now has moved to a
> different phase. Rather counter intuitive.
Obligatory me too: I totally agree with Nicolas here.
-Peff
^ permalink raw reply
* Re: Proposed git mv behavioral change
From: Michael Witten @ 2007-10-19 3:40 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20071019033500.GB10697@coredump.intra.peff.net>
On 18 Oct 2007, at 11:35:00 PM, Jeff King wrote:
> Right, I mean regular "mv", not "git-mv". The only thing that doesn't
> accomplish is moving the "from" entry in the index to the "to" entry
> (but I'm not sure that's such a good idea). Perhaps I've lost
> sight of
> your original goal. Can you state more succintly what you are
> trying to
> accomplish?
I think you're right.
Anyway, succinctly:
> What you want to happen is the following:
>
> git show HEAD:A.txt > path/B.txt
> git add path/B.txt
> mv A.txt B.txt
> git rm A.txt
Better:
> mv A.txt path/B.txt
> Point the index entry for A.txt to path/B.txt
I hope that's right.
^ permalink raw reply
* Re: [PATCH] Change 'Deltifying objects' to 'Delta compressing objects'
From: Nicolas Pitre @ 2007-10-19 3:38 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Jeff King, git
In-Reply-To: <20071019031737.GD14735@spearce.org>
On Thu, 18 Oct 2007, Shawn O. Pearce wrote:
> Nicolas Pitre <nico@cam.org> wrote:
> > On Thu, 18 Oct 2007, Shawn O. Pearce wrote:
> >
> > > I really don't have an opinion either way. Actually I think the
> > > entire progress system of git-pack-objects should be reduced even
> > > further so that users aren't exposed to the internal phases we
> > > are going through. Most of them just don't care. They just
> > > want to know when its going to be done[*1*].
> >
> > Well, with my latest patches in that area, the typical progress on
> > screen has been cut in half. And the different phases are intertaining.
>
> Yup. Your patches were a big improvement. But I'm now sitting here
> wondering if we shouldn't just allow a progress meter to overwrite
> the prior one. Then you only see the current task and progress,
> or the final output if we have nothing further to say about that.
And then you've lost some diagnostic clue (the absolute numbers) about
the actual number of objects that were listed for "deltification" for
example.
And imagine that you see the progress moving slowly because the remote
server is a NSLU2, but it says 80%. Then you go for a coffee and the
progress says 20% when you return because it now has moved to a
different phase. Rather counter intuitive.
Nicolas
^ permalink raw reply
* Re: Proposed git mv behavioral change
From: Jeff King @ 2007-10-19 3:35 UTC (permalink / raw)
To: Michael Witten; +Cc: git
In-Reply-To: <7E3647F4-E61C-4FBE-9AA7-81CDBE324308@MIT.EDU>
On Thu, Oct 18, 2007 at 11:26:32PM -0400, Michael Witten wrote:
>>> touch the working tree". Here we want to touch the working tree
>>> in the sense of moving the file. So --cached is not the correct
>>> option name.
>>
>> Doesn't "mv" do that?
>
> We only want to move it, not also add its dirty contents.
Right, I mean regular "mv", not "git-mv". The only thing that doesn't
accomplish is moving the "from" entry in the index to the "to" entry
(but I'm not sure that's such a good idea). Perhaps I've lost sight of
your original goal. Can you state more succintly what you are trying to
accomplish?
-Peff
^ permalink raw reply
* Re: [PATCH] Change 'Deltifying objects' to 'Delta compressing objects'
From: Shawn O. Pearce @ 2007-10-19 3:33 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Jeff King, git
In-Reply-To: <alpine.LFD.0.9999.0710182312160.19446@xanadu.home>
Nicolas Pitre <nico@cam.org> wrote:
> Frankly, I think effort should be spent on the refs update display at
> this point. Something that looks like:
>
> * refs/heads/origin: fast forward to branch 'master' of git://gi
> t.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6
> old..new: 66ffb04..4fa4d23
>
> [ note that I arbitrarily cut the long line before the 80th column to
> show the effect within an email ]
>
> You usually get long lines that gets wrapped, so that means 3 lines of
> screen space for one updated branches. Is the "66ffb04..4fa4d23"
> information really useful? Might someone ever care?
The reason its formatted the way it is today is someone can grab
that line into a copy-paste buffer and throw it onto a "git-log"
or "gitk" command line with ease to see what new stuff has come in.
Me, I just use the reflog if I care (`origin@{1}..origin`) to see
what a fetch changed in the tracking branch.
However I *don't* need the remote branch name or the remote URL,
especially if we are storing it into a tracking branch. That's most
likely coming from a configured remote that the user fetches from
frequently. I don't think about Linus' URL, I think about the fact
that in my linux-2.6 repository his directory is my origin remote.
Maybe something like this would be more useful:
* origin: fast-forwarded: 66ffb04..4fa4d23
Or if you are using refs/remotes style tracking branches:
* origin/master: fast-forwarded: 66ffb04..4fa4d23
* origin/pu: forcing update: 66ffb04..4fa4d23
Too terse? Yea, probably. But it is a whole lot shorter.
My other pet peeve here is the display from send-pack and
receive-pack when you push a ref. Hello?
updating 'refs/heads/master'
from de61e42b539ffbd28d2a2ba827bb0eb79767057b
to d7e56dbc4f60f6bd238e8612783541d89f006fb7
...
refs/heads/master: de61e42b539ffbd28d2a2ba827bb0eb79767057b -> d7e56dbc4f60f6bd238e8612783541d89f006fb7
That's like 4 too many SHA-1 strings for the average user.
--
Shawn.
^ permalink raw reply
* Re: [PATCH] Change 'Deltifying objects' to 'Delta compressing objects'
From: Jeff King @ 2007-10-19 3:32 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Shawn O. Pearce, git
In-Reply-To: <alpine.LFD.0.9999.0710182312160.19446@xanadu.home>
On Thu, Oct 18, 2007 at 11:24:41PM -0400, Nicolas Pitre wrote:
> Frankly, I think effort should be spent on the refs update display at
> this point. Something that looks like:
Also agreed.
> * refs/heads/origin: fast forward to branch 'master' of git://gi
> t.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6
> old..new: 66ffb04..4fa4d23
>
> [...]
>
> You usually get long lines that gets wrapped, so that means 3 lines of
> screen space for one updated branches. Is the "66ffb04..4fa4d23"
> information really useful? Might someone ever care?
I have used it occasionally when tracking repos to see what new commits
have happened. Usually I use a separate branch to mark "what I've seen"
(i.e., fetch, gitk origin..master, pull), but if it's a branch that I'm
not actively tracking, the display is useful.
What is really useless in that line is the fact that _every_ ref is
going to have the name of the remote, even though we only support
fetching from one remote at a time. Perhaps something like:
Fetching from git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6
* refs/heads/origin: fast forward to branch 'master'
although that URL is almost a line by itself. :)
Also, why do we abbreviate "refs/heads/master" from the remote, but we
don't abbreviate refs/heads/origin for the local? Maybe something like:
* local heads/origin -> remote heads/master (fast forward)
or for separate remote
* local remotes/origin/master -> remote heads/master (fast forward)
Thoughts?
-Peff
^ permalink raw reply
* Re: [PATCH] Change 'Deltifying objects' to 'Delta compressing objects'
From: Nicolas Pitre @ 2007-10-19 3:27 UTC (permalink / raw)
To: Jeff King; +Cc: Shawn O. Pearce, git
In-Reply-To: <20071019031535.GB9274@coredump.intra.peff.net>
On Thu, 18 Oct 2007, Jeff King wrote:
> On Thu, Oct 18, 2007 at 11:11:26PM -0400, Nicolas Pitre wrote:
>
> > I think we might sidestep the issue entirely by remaining somewhat vague
> > and simply saying "compressing objects" for that phase. This is the
> > part responsible for the reduction of a Git repository from 3GB down to
> > 200MB anyway.
>
> OK. I liked it a little more specific, but perhaps users really don't
> care what's going on. And it seems there is some support of simply
> "compressing", so that's probably reasonable.
>
> I think that is going to make the statistics line doubly confusing,
> though, since we never even use the word "delta" (or any wacky
> verbifications based on it), and then they get a line about numbers of
> new and reused deltas.
Well, at least the statistics are real, and we're not inventing anything
here.
Nicolas
^ permalink raw reply
* Re: Proposed git mv behavioral change
From: Michael Witten @ 2007-10-19 3:26 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20071019032407.GA10622@coredump.intra.peff.net>
On 18 Oct 2007, at 11:24:07 PM, Jeff King wrote:
> On Thu, Oct 18, 2007 at 11:19:59PM -0400, Shawn O. Pearce wrote:
>
>> touch the working tree". Here we want to touch the working tree
>> in the sense of moving the file. So --cached is not the correct
>> option name.
>
> Doesn't "mv" do that?
We only want to move it, not also add its dirty contents.
^ permalink raw reply
* Re: [PATCH] Change 'Deltifying objects' to 'Delta compressing objects'
From: Nicolas Pitre @ 2007-10-19 3:24 UTC (permalink / raw)
To: Jeff King; +Cc: Shawn O. Pearce, git
In-Reply-To: <20071019030749.GA9274@coredump.intra.peff.net>
On Thu, 18 Oct 2007, Jeff King wrote:
> On Thu, Oct 18, 2007 at 11:01:02PM -0400, Nicolas Pitre wrote:
>
> > Maybe the "Removing unused objects" should use the common progress
> > infrastructure? It could even use the delayed interface, just like when
> > checking out files, so no progress at all is displayed when that
> > operation completes within a certain delay. And the removal of unused
> > objects is usually quick.
>
> Are you volunteering (I think you know the progress code best)?
> Otherwise, I will get to it, but probably not tonight.
If I do it that won't be today either.
> > But I like the statistics. They might be pretty handy to diagnoze
> > performance issues on remote servers for example.
>
> They are by far the most useful of the three lines I mentioned, but I
> just wonder if they are a bit meaningless and cluttery for light users.
> We can always cut the others and see how it looks.
Frankly, I think effort should be spent on the refs update display at
this point. Something that looks like:
* refs/heads/origin: fast forward to branch 'master' of git://gi
t.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6
old..new: 66ffb04..4fa4d23
[ note that I arbitrarily cut the long line before the 80th column to
show the effect within an email ]
You usually get long lines that gets wrapped, so that means 3 lines of
screen space for one updated branches. Is the "66ffb04..4fa4d23"
information really useful? Might someone ever care?
Nicolas
^ permalink raw reply
* Re: Proposed git mv behavioral change
From: Jeff King @ 2007-10-19 3:24 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Michael Witten, git
In-Reply-To: <20071019031959.GE14735@spearce.org>
On Thu, Oct 18, 2007 at 11:19:59PM -0400, Shawn O. Pearce wrote:
> touch the working tree". Here we want to touch the working tree
> in the sense of moving the file. So --cached is not the correct
> option name.
Doesn't "mv" do that?
-Peff
^ permalink raw reply
* Re: Proposed git mv behavioral change
From: Shawn O. Pearce @ 2007-10-19 3:19 UTC (permalink / raw)
To: Michael Witten; +Cc: git
In-Reply-To: <A2C1BF08-4CC8-4F98-9CA8-B81B2FBFE9E4@mit.edu>
Michael Witten <mfwitten@MIT.EDU> wrote:
>
> On 18 Oct 2007, at 9:54:19 PM, Shawn O. Pearce wrote:
>
> >Elsewhere in git we use the --cached command line option to mean
> >"only make the change in the index".
>
> It seems like --cached should be phased out in favor of --index/ed
No, --index is something else.
But I was originally *way* wrong to propose --cached for this usage
in git-mv. --cached means "apply *ONLY* to the index" and "do *NOT*
touch the working tree". Here we want to touch the working tree
in the sense of moving the file. So --cached is not the correct
option name.
--index is used in Git for places were we update *both* the index
and the working directory (git-apply --index). So actually I should
have suggested "git-mv --index". Whoops.
--
Shawn.
^ permalink raw reply
* Re: [PATCH] Change 'Deltifying objects' to 'Delta compressing objects'
From: Shawn O. Pearce @ 2007-10-19 3:17 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Jeff King, git
In-Reply-To: <alpine.LFD.0.9999.0710182247130.19446@xanadu.home>
Nicolas Pitre <nico@cam.org> wrote:
> On Thu, 18 Oct 2007, Shawn O. Pearce wrote:
>
> > I really don't have an opinion either way. Actually I think the
> > entire progress system of git-pack-objects should be reduced even
> > further so that users aren't exposed to the internal phases we
> > are going through. Most of them just don't care. They just
> > want to know when its going to be done[*1*].
>
> Well, with my latest patches in that area, the typical progress on
> screen has been cut in half. And the different phases are intertaining.
Yup. Your patches were a big improvement. But I'm now sitting here
wondering if we shouldn't just allow a progress meter to overwrite
the prior one. Then you only see the current task and progress,
or the final output if we have nothing further to say about that.
Hmmph. Maybe something like this:
I like how it comes out in the end, but it really screws with the
sideband protocol. Like horribly. I got a whole ton of "remote:
remote: remote: remote: remote: remote:" during a remote clone.
diff --git a/progress.c b/progress.c
index 7629e05..099fc14 100644
--- a/progress.c
+++ b/progress.c
@@ -37,8 +37,6 @@ static void clear_progress_signal(void)
static int display(struct progress *progress, unsigned n, int done)
{
- char *eol;
-
if (progress->delay) {
if (!progress_update || --progress->delay)
return 0;
@@ -55,18 +53,26 @@ static int display(struct progress *progress, unsigned n, int done)
}
progress->last_value = n;
- eol = done ? ", done. \n" : " \r";
- if (progress->total) {
+ if (done) {
+ size_t c = strlen(progress->title);
+ while (c--)
+ fputc(' ', stderr);
+ fputs(" ", stderr);
+ for (; n > 0; n /= 10)
+ fputs(" ", stderr);
+ fputc('\r', stderr);
+ return 1;
+ } else if (progress->total) {
unsigned percent = n * 100 / progress->total;
if (percent != progress->last_percent || progress_update) {
progress->last_percent = percent;
fprintf(stderr, "%s: %3u%% (%u/%u)%s", progress->title,
- percent, n, progress->total, eol);
+ percent, n, progress->total, " \r");
progress_update = 0;
return 1;
}
} else if (progress_update) {
- fprintf(stderr, "%s: %u%s", progress->title, n, eol);
+ fprintf(stderr, "%s: %u%s", progress->title, n, " \r");
progress_update = 0;
return 1;
}
--
Shawn.
^ permalink raw reply related
* Re: [PATCH] Change 'Deltifying objects' to 'Delta compressing objects'
From: Jeff King @ 2007-10-19 3:15 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Shawn O. Pearce, git
In-Reply-To: <alpine.LFD.0.9999.0710182306300.19446@xanadu.home>
On Thu, Oct 18, 2007 at 11:11:26PM -0400, Nicolas Pitre wrote:
> I think we might sidestep the issue entirely by remaining somewhat vague
> and simply saying "compressing objects" for that phase. This is the
> part responsible for the reduction of a Git repository from 3GB down to
> 200MB anyway.
OK. I liked it a little more specific, but perhaps users really don't
care what's going on. And it seems there is some support of simply
"compressing", so that's probably reasonable.
I think that is going to make the statistics line doubly confusing,
though, since we never even use the word "delta" (or any wacky
verbifications based on it), and then they get a line about numbers of
new and reused deltas.
-Peff
^ permalink raw reply
* Re: [PATCH] Change 'Deltifying objects' to 'Delta compressing objects'
From: Nicolas Pitre @ 2007-10-19 3:11 UTC (permalink / raw)
To: Jeff King; +Cc: Shawn O. Pearce, git
In-Reply-To: <20071019025913.GA9227@coredump.intra.peff.net>
On Thu, 18 Oct 2007, Jeff King wrote:
> On Thu, Oct 18, 2007 at 10:45:31PM -0400, Nicolas Pitre wrote:
>
> > Yet that progress display isn't solely about "delta compressing". It
> > also includes the search for best object match in order to keep the
> > smallest delta possible.
>
> In fact, isn't that progress meter _solely_ about finding the best
> matches? The actual deltification (that is, the creation of the deltas
> and writing of them to the packfile happens during the writing phase --
> unless, of course, we've cached the deltas during the search phase).
Well, to find which combination is the smallest you actually have to
create deltas.
> Perhaps one of:
>
> Finding deltas
> Finding delta candidates
> Matching objects
>
> or something similar (though I don't especially like any of them, I
> think you get the idea).
I think we might sidestep the issue entirely by remaining somewhat vague
and simply saying "compressing objects" for that phase. This is the
part responsible for the reduction of a Git repository from 3GB down to
200MB anyway.
Nicolas
^ permalink raw reply
* Re: [PATCH] Change 'Deltifying objects' to 'Delta compressing objects'
From: Jeff King @ 2007-10-19 3:07 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Shawn O. Pearce, git
In-Reply-To: <alpine.LFD.0.9999.0710182251110.19446@xanadu.home>
On Thu, Oct 18, 2007 at 11:01:02PM -0400, Nicolas Pitre wrote:
> > - When fetching, one progress meter says "Indexing" which, while
> > technically true, is almost certainly blocking on "Downloading". In
>
> I have some WIP for that.
Great, I won't start work on it, then.
> Maybe the "Removing unused objects" should use the common progress
> infrastructure? It could even use the delayed interface, just like when
> checking out files, so no progress at all is displayed when that
> operation completes within a certain delay. And the removal of unused
> objects is usually quick.
Are you volunteering (I think you know the progress code best)?
Otherwise, I will get to it, but probably not tonight.
> But I like the statistics. They might be pretty handy to diagnoze
> performance issues on remote servers for example.
They are by far the most useful of the three lines I mentioned, but I
just wonder if they are a bit meaningless and cluttery for light users.
We can always cut the others and see how it looks.
-Peff
^ permalink raw reply
* Re: Proposed git mv behavioral change
From: Michael Witten @ 2007-10-19 2:54 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20071019015419.GV14735@spearce.org>
On 18 Oct 2007, at 9:54:19 PM, Shawn O. Pearce wrote:
> Elsewhere in git we use the --cached command line option to mean
> "only make the change in the index".
It seems like --cached should be phased out in favor of --index/ed
^ 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