Git development
 help / color / mirror / Atom feed
* Re: [PATCH] Include a git-push example for creating a remote branch
From: Miles Bader @ 2007-09-06  5:57 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Junio C Hamano, git
In-Reply-To: <20070906050127.GS18160@spearce.org>

On 9/6/07, Shawn O. Pearce <spearce@spearce.org> wrote:
> Miles Bader <miles.bader@necel.com> wrote:
> > > Many users get confused when `git push origin master:foo` works
> > > when foo already exists on the remote repository but are confused
> > > when foo doesn't exist as a branch and this form does not create
> > > the branch foo.
> >
> > Hmm, what _does_ it do in that case...?
>
> error: dst refspec experimental does not match any existing ref on the remote and does not start with refs/.

Hmm, I'm assuming people don't want to default to just creating a new
remote ref ('cause it might be too easy to muck up a remote archive
that way), but it seems like it would be nice to have a "prettier" way
to create a remote ref than explicitly giving the whole ref path on
the remote side.

Maybe I"m weird, but I tend to think of the refs/... syntax as being
for "only if you're doing something funny" cases.

I'm thinking of something like:

  ... create new local branch "zoink" ...
  git push --create origin zoink

-Miles

-- 
Do not taunt Happy Fun Ball.

^ permalink raw reply

* Re: People unaware of the importance of "git gc"?
From: David Kastrup @ 2007-09-06  5:55 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Nicolas Pitre, Nix, Steven Grimm, Linus Torvalds,
	Git Mailing List
In-Reply-To: <7v1wdciy3w.fsf@gitster.siamese.dyndns.org>

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

> Nicolas Pitre <nico@cam.org> writes:
>
>>> This patch does not add invocation of the "auto repacking".  It
>>> is left to key Porcelain commands that could produce tons of
>>> loose objects to add a call to "git gc --auto" after they are
>>> done their work.  Obvious candidates are:
>>> 
>>> 	git add
>>
>> Nope!  'git add' creates loose objects which are not yet reachable from 
>> anywhere.  They won't get repacked until a commit is made.
>
> Bzzt, I am releaved to see you are sometimes wrong ;-)
>
> They are reachable from the index and are not subject to
> pruning.

Hm.  Isn't it possible to work with several index files at once?  I
seem to remember that even git-add does this itself.  So what is it
that protects objects in such a temporary index from being garbage
collected by a different git process running on the same repository?

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply

* Re: Significant performance waste in git-svn and friends
From: Mike Hommey @ 2007-09-06  5:52 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vd4wwj16d.fsf@gitster.siamese.dyndns.org>

On Wed, Sep 05, 2007 at 01:40:42PM -0700, Junio C Hamano <gitster@pobox.com> wrote:
> Mike Hommey <mh@glandium.org> writes:
> 
> > The same things obviously apply to git-cvsimport and other scripts
> > calling git-hash-object a lot.
> 
> I *obviously* hate this patch, as it makes this Porcelain
> command to be aware of the internal representation too much.

The patch was not supposed to be applied. I said it was lame ;)
It was more of a proof of concept.

Anyways, thinking a bit more about it, I was wondering if it wouldn't be
a good idea to have Git.pm have a "native" implementation (by native I
mean a .so module) for low-level plumbing tools such as hash-object,
cat-file and such.

Obviously, reinventing the wheel is not good, so this native
implementation would be using a "git library" API, such as what has
been done under SoC (though I don't know if this API exposes low-level
plumbing functions)

> I wonder if letting fast-import handle the object creation is an
> option, though.

It could, probably. The reason I didn't use it is that it was way
quicker to hack a 10 lines patch to create the blobs by hand than it
would have been to fork a fast-import object at the correct place during
git-svn initialization and piping to it at the appropriate times.

My goal was only to check how faster this would make it not to fork a
git-hash-object per blob.

Mike

^ permalink raw reply

* Re: Calculating tree nodes
From: Junio C Hamano @ 2007-09-06  5:21 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Jon Smirl, Git Mailing List
In-Reply-To: <20070906032026.GO18160@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> writes:

> The latter (build and test log) would generally be very large.
> We would *not* want to cluster them.  But we might want to store
> next to the commit a very small pointer to the note itself.  Such
> as the note's SHA-1.  Or its offset within the packfile's index.
> This would make locating those notes very cheap, while not having
> a huge impact on the common case of commit traversal.
>
> Likewise we might want to pack a tag's SHA-1 alongside of the commit
> it points at, as parsing the commit would immediately give us all
> annotated tags that refer to that commit.  Tags are (usually) few
> and far between.  But tools like git-describe are commonly used and
> would benefit from not needing to build the commit->tag hashtable.

I am not so sure.

It is a good idea to have an internal API that takes an object
and gives back a set of objects (be they notes to commit or tags
point at object) related to it, but you need to always deal with
loose objects and v2 packs, so you will have to traverse all the
tags and find what they point at _anyway_ to implement the API.
Having a specialized table in v4 does not mean you can look at
that table and nothing else, so I do not immediately see a gain.

^ permalink raw reply

* [PATCH] git-apply: do not read past the end of buffer
From: Junio C Hamano @ 2007-09-06  5:06 UTC (permalink / raw)
  To: git; +Cc: Linus Torvalds

When the preimage we are patching is shorter than what the patch
text expects, we tried to match the buffer contents at the
"original" line with the fragment in full, without checking we
have enough data to match in the preimage.  This caused the size
of a later memmove() to wrap around and attempt to scribble
almost the entire address space.  Not good.

The code that follows the part this patch touches tries to match
the fragment with line offsets.  Curiously, that code does not
have the problem --- it guards against reading past the end of
the preimage.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * This is an ancient part of the code; I am somewhat surprised
   that nobody has ever reported it earlier.

 builtin-apply.c         |    3 +-
 t/t4123-apply-shrink.sh |   58 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+), 1 deletions(-)
 create mode 100755 t/t4123-apply-shrink.sh

diff --git a/builtin-apply.c b/builtin-apply.c
index 25b1447..976ec77 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -1514,7 +1514,8 @@ static int find_offset(const char *buf, unsigned long size, const char *fragment
 	}
 
 	/* Exact line number? */
-	if (!memcmp(buf + start, fragment, fragsize))
+	if ((start + fragsize <= size) &&
+	    !memcmp(buf + start, fragment, fragsize))
 		return start;
 
 	/*
diff --git a/t/t4123-apply-shrink.sh b/t/t4123-apply-shrink.sh
new file mode 100755
index 0000000..984157f
--- /dev/null
+++ b/t/t4123-apply-shrink.sh
@@ -0,0 +1,58 @@
+#!/bin/sh
+
+test_description='apply a patch that is larger than the preimage'
+
+. ./test-lib.sh
+
+cat >F  <<\EOF
+1
+2
+3
+4
+5
+6
+7
+8
+999999
+A
+B
+C
+D
+E
+F
+G
+H
+I
+J
+
+EOF
+
+test_expect_success setup '
+
+	git add F &&
+	mv F G &&
+	sed -e "s/1/11/" -e "s/999999/9/" -e "s/H/HH/" <G >F &&
+	git diff >patch &&
+	sed -e "/^\$/d" <G >F &&
+	git add F
+
+'
+
+test_expect_success 'apply should fail gracefully' '
+
+	if git apply --index patch
+	then
+		echo Oops, should not have succeeded
+		false
+	else
+		status=$?
+		echo "Status was $status"
+		if test -f .git/index.lock
+		then
+			echo Oops, should not have crashed
+			false
+		fi
+	fi
+'
+
+test_done

^ permalink raw reply related

* Re: [RFC] Convert builin-mailinfo.c to use The Better String Library.
From: Miles Bader @ 2007-09-06  5:03 UTC (permalink / raw)
  To: Dmitry Kakurin; +Cc: Matthieu Moy, Git
In-Reply-To: <4AFD7EAD1AAC4E54A416BA3F6E6A9E52@ntdev.corp.microsoft.com>

Dmitry Kakurin <dmitry.kakurin@gmail.com> writes:
> When I first looked at Git source code two things struck me as odd:
> 1. Pure C as opposed to C++. No idea why. Please don't talk about
> portability, it's BS.

Just to piss you off.

-Miles

-- 
Love is a snowmobile racing across the tundra.  Suddenly it flips over,
pinning you underneath.  At night the ice weasels come.  --Nietzsche

^ permalink raw reply

* Re: [PATCH] Include a git-push example for creating a remote branch
From: Shawn O. Pearce @ 2007-09-06  5:01 UTC (permalink / raw)
  To: Miles Bader; +Cc: Junio C Hamano, git
In-Reply-To: <buoodggo0l2.fsf@dhapc248.dev.necel.com>

Miles Bader <miles.bader@necel.com> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> > Many users get confused when `git push origin master:foo` works
> > when foo already exists on the remote repository but are confused
> > when foo doesn't exist as a branch and this form does not create
> > the branch foo.
> 
> Hmm, what _does_ it do in that case...?

error: dst refspec experimental does not match any existing ref on the remote and does not start with refs/.

-- 
Shawn.

^ permalink raw reply

* Re: [RFC] Convert builin-mailinfo.c to use The Better String Library.
From: Shawn O. Pearce @ 2007-09-06  4:59 UTC (permalink / raw)
  To: Dmitry Kakurin; +Cc: Matthieu Moy, Git
In-Reply-To: <4AFD7EAD1AAC4E54A416BA3F6E6A9E52@ntdev.corp.microsoft.com>

Dmitry Kakurin <dmitry.kakurin@gmail.com> wrote:
> When I first looked at Git source code two things struck me as odd:
> 1. Pure C as opposed to C++. No idea why. Please don't talk about 
> portability, it's BS.

Git's creator (Linus) codes in C, not C++.  He has at various times
stated reasons why he does not use C++.  I'm sure one can find such
messages with a bit of searching on mailing lists that he frequents.
He has his reasons.  I also happen to agree with at least some
of them.  :)

Git evolved from that initial prototype that Linus created.  I'm not
sure how much code survives from that initial few versions that
Linus managed before Junio took over, but nobody wanted to rewrite
things that already work so it just stayed in C.
"If it works, don't fix it."

C works.  We (now) have 83,215 lines of it.  Its not going away
anytime soon in Git.  It is also a relatively simple language that
a large number of open source programmers know.  This makes it easy
for them to get involved in the project.  Instead of say Haskell,
which has a smaller community.  Or Tcl/Tk as we recently found out
in the Git User Survey.  :-\

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] Include a git-push example for creating a remote branch
From: Miles Bader @ 2007-09-06  4:54 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Junio C Hamano, git
In-Reply-To: <20070906044408.GA588@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> writes:
> Many users get confused when `git push origin master:foo` works
> when foo already exists on the remote repository but are confused
> when foo doesn't exist as a branch and this form does not create
> the branch foo.

Hmm, what _does_ it do in that case...?

-Miles

-- 
If you can't beat them, arrange to have them beaten.  [George Carlin]

^ permalink raw reply

* Re: [RFC] Convert builin-mailinfo.c to use The Better String Library.
From: Dmitry Kakurin @ 2007-09-06  4:48 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: Git
In-Reply-To: <vpq642pkoln.fsf@bauges.imag.fr>

[ snip ]

When I first looked at Git source code two things struck me as odd:
1. Pure C as opposed to C++. No idea why. Please don't talk about 
portability, it's BS.
2. Brute-force, direct string manipulation. It's both verbose and 
error-prone. This makes it hard to follow high-level code logic.

- Dmitry

^ permalink raw reply

* [PATCH] Include a git-push example for creating a remote branch
From: Shawn O. Pearce @ 2007-09-06  4:44 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Many users get confused when `git push origin master:foo` works
when foo already exists on the remote repository but are confused
when foo doesn't exist as a branch and this form does not create
the branch foo.

This new example highlights the trick of including refs/heads/
in front of the desired branch name to create a branch.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 Documentation/git-push.txt |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 0dd9caf..7b8e075 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -117,6 +117,12 @@ git push origin master:satellite/master::
 	the ref that matches `satellite/master` (most likely, it would
 	be `refs/remotes/satellite/master`) in `origin` repository with it.
 
+git push origin master:refs/heads/experimental::
+	Create the branch `experimental` in the `origin` repository
+	by copying the current `master` branch.  This form is usually
+	needed to create a new branch in the remote repository as
+	there is no `experimental` branch to match.
+
 Author
 ------
 Written by Junio C Hamano <junkio@cox.net>, later rewritten in C
-- 
1.5.3.1.840.g0fedbc

^ permalink raw reply related

* Re: [PATCH 2/2] git-tag -s must fail if gpg is broken and cannot sign tags
From: Shawn O. Pearce @ 2007-09-06  4:26 UTC (permalink / raw)
  To: Junio C Hamano, Carlos Rica; +Cc: git
In-Reply-To: <20070906042115.GA343@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> wrote:
> If the user has misconfigured `user.signingkey` in their .git/config
> or just doesn't have any secret keys on their keyring and they ask
> for a signed tag with `git tag -s` we better make sure the resulting
> tag was actually signed by gpg.
...
>  I think this and my prior contrib/workdir patch should both go into
>  maint.  This one in particular; it hurt us today when an automated
>  tool that runs `git tag -s` didn't notice the GnuPG problems.

I'm sorry, despite the subject of this email this is actually a
*one* patch series.  The 2/2 is because I applied and tested this
on top of the contrib/workdir patch I was talking about above and
my email sending script thought it was a two patch series.

-- 
Shawn.

^ permalink raw reply

* [PATCH 2/2] git-tag -s must fail if gpg is broken and cannot sign tags
From: Shawn O. Pearce @ 2007-09-06  4:21 UTC (permalink / raw)
  To: Junio C Hamano, Carlos Rica; +Cc: git

If the user has misconfigured `user.signingkey` in their .git/config
or just doesn't have any secret keys on their keyring and they ask
for a signed tag with `git tag -s` we better make sure the resulting
tag was actually signed by gpg.

Prior versions of builtin git-tag allowed this failure to slip
by without error as they were not checking the return value of
the finish_command() so they did not notice when gpg exited with
an error exit status.  They also did not fail if gpg produced an
empty output or if read_in_full received an error from the read
system call while trying to read the pipe back from gpg.

Finally we did not actually honor any return value from the do_sign
function as it returns ssize_t but was being stored into an unsigned
long.  This caused the compiler to optimize out the die condition,
allowing git-tag to continue along and create the tag object.

With these issues fixed `git-tag -s` will now fail to create the
tag and will report a non-zero exit status to its caller, thereby
allowing automated helper scripts to detect (and recover from)
failure if gpg is not working properly.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---

 I think this and my prior contrib/workdir patch should both go into
 maint.  This one in particular; it hurt us today when an automated
 tool that runs `git tag -s` didn't notice the GnuPG problems.

 builtin-tag.c  |    8 +++++---
 t/t7004-tag.sh |    7 +++++++
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/builtin-tag.c b/builtin-tag.c
index 348919c..aadf850 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -217,7 +217,8 @@ static ssize_t do_sign(char *buffer, size_t size, size_t max)
 	gpg.close_in = 0;
 	len = read_in_full(gpg.out, buffer + size, max - size);
 
-	finish_command(&gpg);
+	if (finish_command(&gpg) || !len || len < 0)
+		return error("gpg failed to sign the tag");
 
 	if (len == max - size)
 		return error("could not read the entire signature from gpg.");
@@ -310,9 +311,10 @@ static void create_tag(const unsigned char *object, const char *tag,
 	size += header_len;
 
 	if (sign) {
-		size = do_sign(buffer, size, max_size);
-		if (size < 0)
+		ssize_t r = do_sign(buffer, size, max_size);
+		if (r < 0)
 			die("unable to sign the tag");
+		size = r;
 	}
 
 	if (write_sha1_file(buffer, size, tag_type, result) < 0)
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 606d4f2..0d07bc3 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -990,6 +990,13 @@ test_expect_success \
 	git diff expect actual
 '
 
+# try to sign with bad user.signingkey
+git config user.signingkey BobTheMouse
+test_expect_failure \
+	'git-tag -s fails if gpg is misconfigured' \
+	'git tag -s -m tail tag-gpg-failure'
+git config --unset user.signingkey
+
 # try to verify without gpg:
 
 rm -rf gpghome
-- 
1.5.3.1.840.g0fedbc

^ permalink raw reply related

* [PATCH] Don't allow contrib/workdir/git-new-workdir to trash existing dirs
From: Shawn O. Pearce @ 2007-09-06  3:33 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Recently I found that doing a sequence like the following:

  git-new-workdir a b
  ...
  git-new-workdir a b

by accident will cause a (and now also b) to have an infinite cycle
in its refs directory.  This is caused by git-new-workdir trying
to create the "refs" symlink over again, only during the second
time it is being created within a's refs directory and is now also
pointing back at a's refs.

This causes confusion in git as suddenly branches are named things
like "refs/refs/refs/refs/refs/refs/refs/heads/foo" instead of the
more commonly accepted "refs/heads/foo".  Plenty of commands start
to see ambiguous ref names and others just take ages to compute.

git-clone has the same safety check, so git-new-workdir should
behave just like it.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 contrib/workdir/git-new-workdir |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/contrib/workdir/git-new-workdir b/contrib/workdir/git-new-workdir
index c6e154a..2838546 100755
--- a/contrib/workdir/git-new-workdir
+++ b/contrib/workdir/git-new-workdir
@@ -48,6 +48,12 @@ then
 		"a complete repository."
 fi
 
+# don't recreate a workdir over an existing repository
+if test -e "$new_workdir"
+then
+	die "destination directory '$new_workdir' already exists."
+fi
+
 # make sure the the links use full paths
 git_dir=$(cd "$git_dir"; pwd)
 
-- 
1.5.3.1.840.g0fedbc

^ permalink raw reply related

* Re: Calculating tree nodes
From: Shawn O. Pearce @ 2007-09-06  3:20 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jon Smirl, Git Mailing List
In-Reply-To: <7vbqcinxdb.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> 
> > There's nothing stopping us from creating additional indexes.
> > ...
> > But we can also store the notes alongside the commits in the
> > packfile, so that if the data for the commit has been paged in
...
> 
> I would agree with your main thrust "nobody prevents you from
> building additional index", but on a tangent, I am skeptical
> about adding too much to pack v4.  Especially "clustering the
> notes" part.
...
> Now, hopefully many operations do not need notes either,
> although notes themselves can store _anything_ so each of them
> could be large and/or each commit could have large number of
> them.  I suspect clustering notes along with the commit they
> annotate would break the locality of access for common case.

I'm inclined to agree.

Its something I've thought about doing.  I haven't even prototyped
code for it.  Let alone shown numbers that say one way or the other.

One of the notes proposals was talking about having lots of different
classes of notes.  E.g. a "Signed-off-by" class and a "build and test
log results" class.

The former would generally be very small and may even want to be
shown most of the time the commit body is displayed (e.g. in gitk,
git-log).  These would be good candidates to cluster alongside the
commit.  Indeed they are clustered there today, just hung inside
of the commit object itself.  Nobody is bitching about the hit they
cause on the common case of `pack-objects`.  :)

The latter (build and test log) would generally be very large.
We would *not* want to cluster them.  But we might want to store
next to the commit a very small pointer to the note itself.  Such
as the note's SHA-1.  Or its offset within the packfile's index.
This would make locating those notes very cheap, while not having
a huge impact on the common case of commit traversal.

Likewise we might want to pack a tag's SHA-1 alongside of the commit
it points at, as parsing the commit would immediately give us all
annotated tags that refer to that commit.  Tags are (usually) few
and far between.  But tools like git-describe are commonly used and
would benefit from not needing to build the commit->tag hashtable.
OK, well, git-describe cheats and uses the struct object hashtable,
but whatever.

You get my point.  I think.  And I got yours about not making the
common case worse than it already is today.

-- 
Shawn.

^ permalink raw reply

* Re: People unaware of the importance of "git gc"?
From: Shawn O. Pearce @ 2007-09-06  2:56 UTC (permalink / raw)
  To: Steven Grimm
  Cc: Junio C Hamano, Nicolas Pitre, Nix, Linus Torvalds,
	Git Mailing List
In-Reply-To: <46DF6AA8.60804@midwinter.com>

Steven Grimm <koreth@midwinter.com> wrote:
> Shawn O. Pearce wrote:
> >But this suffers from the same fate if the user sets gc.auto too
> >small and doesn't realize that the reason Git is always repacking
> >is because over the last 6 months they have been unlucky enough to
> >stage the magic number of unreachable blobs into the 17 directory
> >and they have *never* run `git gc --prune` because the auto thing
> >is working just fine for them and they don't realize they need to
> >prune every once in a blue moon.
> 
> Check the modification times on those files and don't count ones that 
> are older than the last git-gc run, maybe? That'd take care of the problem.

Eh, that could mean a bunch of stat calls that it would be nice
to avoid.  The counter Junio (and git-gui) implements just does
a readdir().  Reasonably cheap.

Maybe just save a ".git/gc_last_auto" with the last object count
of .git/objects/17, after repacking.  If the count is over the
gc.auto limit *and* is still over the limit after subtracting the
".git/gc_last_auto" value then consider that auto is required.

This way the file is only consulted if we are really thinking
about running a repack, and its only written to if we actually do
the repack.  So we only take the extra penalty if we are going to
be taking a *really* big extra penalty by repacking.

-- 
Shawn.

^ permalink raw reply

* Re: People unaware of the importance of "git gc"?
From: Shawn O. Pearce @ 2007-09-06  2:52 UTC (permalink / raw)
  To: Russ Dill; +Cc: git, Junio C Hamano, Nicolas Pitre
In-Reply-To: <loom.20070906T044017-727@post.gmane.org>

Russ Dill <Russ.Dill@gmail.com> wrote:
> > Ok, how about doing something like this?
> 
> git add? merge? rebase? No, I have a sneakier place to invoke gc.
> 
> Whenever $EDITOR gets invoked. Heck, whenever git is waiting for any user input,
> do some gc in the background, it'd just have to be incremental so that we could
> pick up where we left off.

Heh.  That is a really good idea.  I've been thinking about doing
some automatic generational style GC type repacking controls in
git-gui, and doing them when git-gui is sitting idle and has not
been used in the past couple of minutes.

This is along the same vein of thought.  I like it.  Often it
takes me a while to come up with a good commit message even if
I am using command line commit.

But git-rebase/git-am can cause a huge number of objects to be
created, especially if you are pushing a large stack of patches
around.  So it may still be a good idea to trigger `gc --auto`
at the end of those operations.
 
> Similarly, you could mix it in with git pull/push so that while we are waiting
> on the network, we can do some packing.

Here's a better thought:

If we are pushing somewhere, and the push size is "large-ish" and
we aren't pushing a thin pack (its currently considered not nice
to the remote end so it doesn't happen by default) and the objects
we are packing are mostly all loose maybe we should also save a
copy of that packfile locally, then prune *only* those loose objects
back.

Not every git user pushes their work.  But many do.  And those
that push usually will do so in bursts, are already expecting to
wait for the network latency, and usually are pushing the majority
of the things that are loose.  Such users will probably never see
the `gc --auto` trip in places like commit/am/merge as they would
already be clearing their ODB with the push.

-- 
Shawn.

^ permalink raw reply

* Re: People unaware of the importance of "git gc"?
From: Steven Grimm @ 2007-09-06  2:49 UTC (permalink / raw)
  To: Shawn O. Pearce
  Cc: Junio C Hamano, Nicolas Pitre, Nix, Linus Torvalds,
	Git Mailing List
In-Reply-To: <20070906024555.GJ18160@spearce.org>

Shawn O. Pearce wrote:
> But this suffers from the same fate if the user sets gc.auto too
> small and doesn't realize that the reason Git is always repacking
> is because over the last 6 months they have been unlucky enough to
> stage the magic number of unreachable blobs into the 17 directory
> and they have *never* run `git gc --prune` because the auto thing
> is working just fine for them and they don't realize they need to
> prune every once in a blue moon.
>   

Check the modification times on those files and don't count ones that 
are older than the last git-gc run, maybe? That'd take care of the problem.

-Steve

^ permalink raw reply

* Re: People unaware of the importance of "git gc"?
From: Shawn O. Pearce @ 2007-09-06  2:45 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Nicolas Pitre, Nix, Steven Grimm, Linus Torvalds,
	Git Mailing List
In-Reply-To: <7vr6lcj2zi.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> wrote:
> Implement git gc --auto
... 

Danger...  If the user sets `gc.auto` to a low enough value and
they are also unlucky enough to have a few truely unreachable (thus
pruneable) objects in .git/objects/17/ then this is going to run
a bunch of gc work on every commit they make.

I'm actually running into this problem in git-gui.  On Windows
it suggests a repack if there is one object in .git/objects/42/.
Some users have been unlucky enough to stage a file, have it
hash into that directory, then restage a different version of it.
The prior one is never considered reachable (it was never committed),
but will now *always* cause git-gui to suggest a repack on every
startup.  For all time.

Yea, I need to fix that.

But this suffers from the same fate if the user sets gc.auto too
small and doesn't realize that the reason Git is always repacking
is because over the last 6 months they have been unlucky enough to
stage the magic number of unreachable blobs into the 17 directory
and they have *never* run `git gc --prune` because the auto thing
is working just fine for them and they don't realize they need to
prune every once in a blue moon.

-- 
Shawn.

^ permalink raw reply

* Re: People unaware of the importance of "git gc"?
From: Russ Dill @ 2007-09-06  2:44 UTC (permalink / raw)
  To: git
In-Reply-To: <7vr6lcj2zi.fsf@gitster.siamese.dyndns.org>


> Ok, how about doing something like this?
> 

git add? merge? rebase? No, I have a sneakier place to invoke gc.

Whenever $EDITOR gets invoked. Heck, whenever git is waiting for any user input,
do some gc in the background, it'd just have to be incremental so that we could
pick up where we left off.

Similarly, you could mix it in with git pull/push so that while we are waiting
on the network, we can do some packing.

Course, this wouldn't work for all repositories.

^ permalink raw reply

* Re: Invoke "git gc --auto" from commit, merge, am and rebase.
From: Shawn O. Pearce @ 2007-09-06  2:39 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Nicolas Pitre, Nix, Steven Grimm, Linus Torvalds,
	Git Mailing List
In-Reply-To: <7vmyw0hixs.fsf_-_@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> wrote:
> The point of auto gc is to pack new objects created in loose
> format, so a good rule of thumb is where we do update-ref after
> creating a new commit.
...
>  git-am.sh                  |    2 ++
>  git-commit.sh              |    1 +
>  git-merge.sh               |    1 +
>  git-rebase--interactive.sh |    2 ++
>  4 files changed, 6 insertions(+), 0 deletions(-)
...
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index abc2b1c..8258b7a 100755
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -307,6 +307,8 @@ do_next () {
>  	rm -rf "$DOTEST" &&
>  	warn "Successfully rebased and updated $HEADNAME."
>  
> +	git gc --auto
> +
>  	exit
>  }

Why bother with git-rebase--interactive.sh?  It calls two tools,
git-cherry-pick (which calls git-commit) and git-commit to do its
per-commit dirty work.  So on every step of `git rebase -i` we are
now running `git gc --auto`.  No need to also run it at the end.

Note this is also true of `git rebase -m` as that uses the wonderful
feature of `git commit -C $oldid` per commit to make the new commit.
 
-- 
Shawn.

^ permalink raw reply

* Re: [RFC] Convert builin-mailinfo.c to use The Better String Library.
From: Miles Bader @ 2007-09-06  2:30 UTC (permalink / raw)
  To: Matthieu Moy
  Cc: Kristian Høgsberg, Lukas Sandström, Git Mailing List,
	Junio C Hamano
In-Reply-To: <vpq642pkoln.fsf@bauges.imag.fr>

Matthieu Moy <Matthieu.Moy@imag.fr> writes:
> and if people decide that git needs a non-trivial string library,
> writting/testing more code in strbuf.c would probably be more work
> than just reading what bsstring code does to become familiar enough
> with it to even be able to maintain it later.

>From what I've seen (by perusing the bstring website), bstring is kind
of ugly though....

-Miles

-- 
"Suppose we've chosen the wrong god. Every time we go to church we're
just making him madder and madder." -- Homer Simpson

^ permalink raw reply

* Re: [PATCH 1/3] git-gui/Makefile: Replace libdir with gitgui_libdir
From: Shawn O. Pearce @ 2007-09-06  2:32 UTC (permalink / raw)
  To: Dmitry V. Levin; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <20070905232153.GA331@nomad.office.altlinux.org>

"Dmitry V. Levin" <ldv@altlinux.org> wrote:
> On GNU/Linux, libdir is used to mean "/usr/lib or /usr/lib64"
> depending on architecture.  Different libdir meaning breaks
> idiomatic expressions like rpm specfile "make libdir=%_libdir".
> 
> Signed-off-by: Dmitry V. Levin <ldv@altlinux.org>
> ---
>  git-gui/Makefile |   16 ++++++++--------
>  1 files changed, 8 insertions(+), 8 deletions(-)

Although I could apply this with `am -3` I'm NACK'ing this right
now because...
 
> diff --git a/git-gui/Makefile b/git-gui/Makefile
> index 1bac6fe..f143b2c 100644
> --- a/git-gui/Makefile
> +++ b/git-gui/Makefile
> @@ -76,8 +76,8 @@ SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
>  TCL_PATH_SQ = $(subst ','\'',$(TCL_PATH))
>  TCLTK_PATH_SQ = $(subst ','\'',$(TCLTK_PATH))
>  
> -libdir   ?= $(sharedir)/git-gui/lib
> -libdir_SQ = $(subst ','\'',$(libdir))
> +gitgui_libdir   ?= $(sharedir)/git-gui/lib
> +gitgui_libdir_SQ = $(subst ','\'',$(gitgui_libdir))

git-gui is its own project with its own Makefile.  Junio includes
it in git.git to help widen its audience, and because it is quite
portable and easy for him to include.   In the future git-gui will
become a proper subproject of git.git.

If you want to define libdir in git's toplevel Makefile *and*
that definition is being exported down into git-gui's Makefile
then I think git's toplevel Makefile should arrange for it to
not export libdir down into git-gui.

Of course one has to wonder if that is even possible in GNU make.
I'm not a GNU make expert so I don't really know.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH 1/2] git-commit: Disallow unchanged tree in non-merge mode
From: Shawn O. Pearce @ 2007-09-06  2:25 UTC (permalink / raw)
  To: Dmitry V. Levin; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <20070905234941.GA643@nomad.office.altlinux.org>

"Dmitry V. Levin" <ldv@altlinux.org> wrote:
> Do not commit an unchanged tree in non-merge mode.

A laudable goal.  git-gui also does this.  Turns out the other
checks within git-gui prevent the user from ever getting that far.
I probably should remove the empty tree check as it costs CPU time
to get the old tree.  But I'd rather have the safety check.
 
> The idea is that amend should not commit an unchanged tree,
> one should just remove the top commit using git-reset instead.

NO.  `git commit --amend` is *often* used for fixing the commit
message.  Or adding additional detail.  Forcing the user to do
a `git reset --soft HEAD^ && git commit --amend` just because
you don't want git-commit to make an "empty commit" (which it
doesn't usually like to do now anyway!) is a major step back
in functionality.

NACK.
 
> diff --git a/git-commit.sh b/git-commit.sh
> index 1d04f1f..800f96c 100755
> --- a/git-commit.sh
> +++ b/git-commit.sh
> @@ -629,6 +629,16 @@ then
>  		tree=$(GIT_INDEX_FILE="$TMP_INDEX" git write-tree) &&
>  		rm -f "$TMP_INDEX"
>  	fi &&
> +	if test -n "$current" -a ! -f "$GIT_DIR/MERGE_HEAD"
> +	then
> +		current_tree="$(git cat-file commit "$current${amend:+^}" 2>/dev/null |
> +				sed -e '/^tree \+/!d' -e 's///' -e q)"

The better way to get the old tree would be this:

		current_tree="$(git rev-parse "$current${amend:+^}^{tree}" 2>/dev/null

as it avoids the tool from needing to know about the internal
representation of a commit object.  It also avoids an entire
fork+exec of a sed process.

> +		if test "$tree" = "$current_tree"
> +		then
> +			echo >&2 "nothing to commit${amend:+ (use \"git reset HEAD^\" to remove the top commit)}"

That message is a bad idea.  Doing a mixed mode reset will also
reset the index, causing the user to lose any changes that had
already been staged.  This may actually be difficult for him/her to
recover from if they have used `git add -i` or git-gui to stage only
certain hunks of files, or if their working tree has been further
modified after the commit but they want to go back and amend the
message only of the prior commit.

-- 
Shawn.

^ permalink raw reply

* Re: Significant performance waste in git-svn and friends
From: Shawn O. Pearce @ 2007-09-06  2:19 UTC (permalink / raw)
  To: David Kastrup; +Cc: Junio C Hamano, Mike Hommey, git
In-Reply-To: <85bqcghktl.fsf@lola.goethe.zz>

David Kastrup <dak@gnu.org> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Mike Hommey <mh@glandium.org> writes:
> >
> >> The same things obviously apply to git-cvsimport and other scripts
> >> calling git-hash-object a lot.
> >
> > I wonder if letting fast-import handle the object creation is an
> > option, though.
> 
> I think it would be saner to give git-hash-object an operation mode
> that makes it usable as a pipe-controlled daemon, so that one needs
> not fork and exec for interning another object.  That way, porcelain
> commands could keep one bidirectional pipe (feed object type and
> source and whether to use -w into git-hash-project, receive object id)
> to git-hash-object around until they finish.

Aside from getting the hashes back from fast-import, that's what
fast-import is for.  I could also make it disable writing.  Hmm.
Junio and I were just talking about making fast-import send the
marks table back out on stdout.  This would make it easier for a
frontend process to stream a whole bunch of objects into the process,
then get back all of their SHA-1s.  Less context switches and more
parallel operation.

-- 
Shawn.

^ 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