Git development
 help / color / mirror / Atom feed
* Re: [PATCH 2/2] xdi_diff: trim common trailing lines
From: Junio C Hamano @ 2007-12-14 19:15 UTC (permalink / raw)
  To: Peter Baumann; +Cc: Nicolas Pitre, git
In-Reply-To: <20071214090614.GB15610@xp.machine.xx>

Peter Baumann <waste.manager@gmx.de> writes:

> So you are loosing some values in your trim_common_tail function by
> making ctx only an int. (Not sure that it matters, but I noticed it
> while glancing over your code).

While it is true that this does not matter in practice (because the
context value initially comes from the end user via -U parameter that is
stored in a field of type int in diff_options structure), I agre that it
is the right thing to do to use the same type as underlying xdiff
library uses at the interface level.  From the layering point of view.
xdiff-interface.[ch] are meant to be a thin usability wrapper, it should
not needlessly deviate from how the underlying xdiff operates.

^ permalink raw reply

* Re: testsuite failures in mainline...
From: Junio C Hamano @ 2007-12-14 19:15 UTC (permalink / raw)
  To: David Miller; +Cc: git
In-Reply-To: <20071214.104312.103638776.davem@davemloft.net>

David Miller <davem@davemloft.net> writes:

> I've been seeing this for I think a week or two, and I can't figure
> out if it's some local problem of mine.  I even reran the testsuite
> with "PATH=$(pwd):$PATH" just in case it was picking up my existing
> 1.5.3.7 installation for some reason, but it still fails even in that
> case.
>
> Is the following a known issue?

No, you are the first to report it here.  I do not push out the tip of
'master' and 'next' unless all tests pass in my primary repository on my
private machine and in a repository under my k.org account, so you can
at least assume all tests has passed for _some_ people.  It of course
does not guarantee there isn't timing related bugs I am not catching. 

> *** t3200-branch.sh ***
> ...
> * FAIL 20: test tracking setup via config
>         git config branch.autosetupmerge true &&
>              git config remote.local.url . &&
>              git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
>              (git show-ref -q refs/remotes/local/master || git-fetch local) &&
>              git branch my3 local/master &&
>              test $(git config branch.my3.remote) = local &&
>              test $(git config branch.my3.merge) = refs/heads/master

When I can reproduce a breakage, in our tests, I'd run

	cd t && sh -x t3200-branch.sh -i -v

(replace "t3200-*" with the failing test) and see which one of the steps
chained with && is breaking first.

^ permalink raw reply

* config.c fixes
From: Kristian Høgsberg @ 2007-12-14 19:28 UTC (permalink / raw)
  To: gitster; +Cc: git

Hi,

[ This should have gone out as a cover letter for the patch series
  but some how I didn't manage to make that work ]

While strace'ing builtin-clone I saw this horror:

...
write(3, "remote", 6)                   = 6
write(3, " = ", 3)                      = 3
write(3, "o", 1)                        = 1
write(3, "r", 1)                        = 1
write(3, "i", 1)                        = 1
write(3, "g", 1)                        = 1
write(3, "i", 1)                        = 1
write(3, "n", 1)                        = 1
write(3, "\n", 1)                       = 1
munmap(0xb7f72000, 102)                 = 0
close(3)                                = 0
close(3)                                = -1 EBADF (Bad file descriptor)
...

That's just terrible.  And hey, it turns out the code is terrible too.
I think the best solution is to just parse up the entire config file
up front and keep it in a data structure, make the changes and then
write it all out at the end.  That makes it cheap to make a series of
changes too:

	config = open_config(file);
	set_key(config, key1, value1);
	set_key(config, key2, value2);
	set_key(config, key3, value3);
	commit_config(config);

Or something.  Anyway, for 1.5.4 I wrote the following two patches that
fixes the 1-bytes writes and the double close.

 config.c |  113 +++++++++++++++++++++++--------------------------------------
 1 files changed, 43 insertions(+), 70 deletions(-)

cheers,
Kristian

^ permalink raw reply

* config.c fixes
From: Kristian Høgsberg @ 2007-12-14 19:22 UTC (permalink / raw)
  To: gitster; +Cc: git

Hi,

While strace'ing builtin-clone I saw this horror:

...
write(3, "remote", 6)                   = 6
write(3, " = ", 3)                      = 3
write(3, "o", 1)                        = 1
write(3, "r", 1)                        = 1
write(3, "i", 1)                        = 1
write(3, "g", 1)                        = 1
write(3, "i", 1)                        = 1
write(3, "n", 1)                        = 1
write(3, "\n", 1)                       = 1
munmap(0xb7f72000, 102)                 = 0
close(3)                                = 0
close(3)                                = -1 EBADF (Bad file descriptor)
...

That's just terrible.  And hey, it turns out the code is terrible too.
I think the best solution is to just parse up the entire config file
up front and keep it in a data structure, make the changes and then
write it all out at the end.  That makes it cheap to make a series of
changes too:

	config = open_config(file);
	set_key(config, key1, value1);
	set_key(config, key2, value2);
	set_key(config, key3, value3);
	commit_config(config);

Or something.  Anyway, for 1.5.4 I wrote the following two patches that
fixes the 1-bytes writes and the double close.

 config.c |  113 +++++++++++++++++++++++--------------------------------------
 1 files changed, 43 insertions(+), 70 deletions(-)

cheers,
Kristian

^ permalink raw reply

* testsuite failures in mainline...
From: David Miller @ 2007-12-14 18:43 UTC (permalink / raw)
  To: git


I've been seeing this for I think a week or two, and I can't figure
out if it's some local problem of mine.  I even reran the testsuite
with "PATH=$(pwd):$PATH" just in case it was picking up my existing
1.5.3.7 installation for some reason, but it still fails even in that
case.

Is the following a known issue?

*** t3200-branch.sh ***
*   ok 1: prepare a trivial repository
*   ok 2: git branch --help should not have created a bogus branch
*   ok 3: git branch abc should create a branch
*   ok 4: git branch a/b/c should create a branch
*   ok 5: git branch -l d/e/f should create a branch and a log
*   ok 6: git branch -d d/e/f should delete a branch and a log
*   ok 7: git branch j/k should work after branch j has been deleted
*   ok 8: git branch l should work after branch l/m has been deleted
*   ok 9: git branch -m m m/m should work
*   ok 10: git branch -m n/n n should work
*   ok 11: git branch -m o/o o should fail when o/p exists
*   ok 12: git branch -m q r/q should fail when r exists
*   ok 13: git branch -m q q2 without config should succeed
*   ok 14: git branch -m s/s s should work when s/t is deleted
*   ok 15: config information was renamed, too
*   ok 16: git branch -m u v should fail when the reflog for u is a symlink
*   ok 17: test tracking setup via --track
*   ok 18: test tracking setup (non-wildcard, matching)
*   ok 19: test tracking setup (non-wildcard, not matching)
* FAIL 20: test tracking setup via config
        git config branch.autosetupmerge true &&
             git config remote.local.url . &&
             git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
             (git show-ref -q refs/remotes/local/master || git-fetch local) &&
             git branch my3 local/master &&
             test $(git config branch.my3.remote) = local &&
             test $(git config branch.my3.merge) = refs/heads/master
* FAIL 21: avoid ambiguous track
        
                git config branch.autosetupmerge true &&
                git config remote.ambi1.url = lalala &&
                git config remote.ambi1.fetch = refs/heads/lalala:refs/heads/master &&
                git config remote.ambi2.url = lilili &&
                git config remote.ambi2.fetch = refs/heads/lilili:refs/heads/master &&
                git branch all1 master &&
                test -z "$(git config branch.all1.merge)"
        
*   ok 22: test overriding tracking setup via --no-track
* FAIL 23: no tracking without .fetch entries
        git branch --track my6 s &&
             test -z "$(git config branch.my6.remote)" &&
             test -z "$(git config branch.my6.merge)"
* FAIL 24: test tracking setup via --track but deeper
        git config remote.local.url . &&
             git config remote.local.fetch refs/heads/*:refs/remotes/local/* &&
             (git show-ref -q refs/remotes/local/o/o || git-fetch local) &&
             git branch --track my7 local/o/o &&
             test "$(git config branch.my7.remote)" = local &&
             test "$(git config branch.my7.merge)" = refs/heads/o/o
* FAIL 25: test deleting branch deletes branch config
        git branch -d my7 &&
             test -z "$(git config branch.my7.remote)" &&
             test -z "$(git config branch.my7.merge)"
*   ok 26: test deleting branch without config
*   ok 27: git checkout -b g/h/i -l should create a branch and a log
* failed 5 among 27 test(s)

^ permalink raw reply

* [PATCH 1/2] Fix config lockfile handling.
From: Kristian Høgsberg @ 2007-12-14 19:22 UTC (permalink / raw)
  To: gitster; +Cc: git, Kristian Høgsberg
In-Reply-To: <1197660157-24109-1-git-send-email-krh@redhat.com>

When we commit or roll back the lock file the fd is automatically closed,
so don't do that again.  Also, just keep the lock on the stack.

Signed-off-by: Kristian Høgsberg <krh@redhat.com>
---
 config.c |   22 +++++-----------------
 1 files changed, 5 insertions(+), 17 deletions(-)

diff --git a/config.c b/config.c
index 49d2b42..0725563 100644
--- a/config.c
+++ b/config.c
@@ -754,7 +754,7 @@ int git_config_set_multivar(const char* key, const char* value,
 	int fd = -1, in_fd;
 	int ret;
 	char* config_filename;
-	struct lock_file *lock = NULL;
+	struct lock_file lock;
 	const char* last_dot = strrchr(key, '.');
 
 	config_filename = getenv(CONFIG_ENVIRONMENT);
@@ -811,8 +811,7 @@ int git_config_set_multivar(const char* key, const char* value,
 	 * The lock serves a purpose in addition to locking: the new
 	 * contents of .git/config will be written into it.
 	 */
-	lock = xcalloc(sizeof(struct lock_file), 1);
-	fd = hold_lock_file_for_update(lock, config_filename, 0);
+	fd = hold_lock_file_for_update(&lock, config_filename, 0);
 	if (fd < 0) {
 		fprintf(stderr, "could not lock config file\n");
 		free(store.key);
@@ -955,28 +954,17 @@ int git_config_set_multivar(const char* key, const char* value,
 		munmap(contents, contents_sz);
 	}
 
-	if (close(fd) || commit_lock_file(lock) < 0) {
+	if (commit_lock_file(&lock) < 0) {
 		fprintf(stderr, "Cannot commit config file!\n");
 		ret = 4;
 		goto out_free;
 	}
 
-	/* fd is closed, so don't try to close it below. */
-	fd = -1;
-	/*
-	 * lock is committed, so don't try to roll it back below.
-	 * NOTE: Since lockfile.c keeps a linked list of all created
-	 * lock_file structures, it isn't safe to free(lock).  It's
-	 * better to just leave it hanging around.
-	 */
-	lock = NULL;
 	ret = 0;
 
 out_free:
-	if (0 <= fd)
-		close(fd);
-	if (lock)
-		rollback_lock_file(lock);
+	/* If we already committed the lock file, the rollback is a no-op. */
+	rollback_lock_file(&lock);
 	free(config_filename);
 	return ret;
 
-- 
1.5.3.4

^ permalink raw reply related

* [PATCH 2/2] Use a strbuf for building up section header and key/value pair strings.
From: Kristian Høgsberg @ 2007-12-14 19:22 UTC (permalink / raw)
  To: gitster; +Cc: git, Kristian Høgsberg
In-Reply-To: <1197660157-24109-2-git-send-email-krh@redhat.com>

Avoids horrible 1-byte write(2) calls and cleans up the logic a bit.

Signed-off-by: Kristian Høgsberg <krh@redhat.com>
---
 config.c |   91 ++++++++++++++++++++++++++------------------------------------
 1 files changed, 38 insertions(+), 53 deletions(-)

diff --git a/config.c b/config.c
index 0725563..3392bc4 100644
--- a/config.c
+++ b/config.c
@@ -610,46 +610,36 @@ static int write_error(void)
 
 static int store_write_section(int fd, const char* key)
 {
-	const char *dot = strchr(key, '.');
-	int len1 = store.baselen, len2 = -1;
+	const char *dot;
+	int i, success;
+	struct strbuf sb;
 
-	dot = strchr(key, '.');
+	strbuf_init(&sb, 0);
+	dot = memchr(key, '.', store.baselen);
 	if (dot) {
-		int dotlen = dot - key;
-		if (dotlen < len1) {
-			len2 = len1 - dotlen - 1;
-			len1 = dotlen;
+		strbuf_addf(&sb, "[%.*s \"", dot - key, key);
+		for (i = dot - key + 1; i < store.baselen; i++) {
+			if (key[i] == '"')
+				strbuf_addch(&sb, '\\');
+			strbuf_addch(&sb, key[i]);
 		}
+		strbuf_addstr(&sb, "\"]\n");
+	} else {
+		strbuf_addf(&sb, "[%.*s]\n", store.baselen, key);
 	}
 
-	if (write_in_full(fd, "[", 1) != 1 ||
-	    write_in_full(fd, key, len1) != len1)
-		return 0;
-	if (len2 >= 0) {
-		if (write_in_full(fd, " \"", 2) != 2)
-			return 0;
-		while (--len2 >= 0) {
-			unsigned char c = *++dot;
-			if (c == '"')
-				if (write_in_full(fd, "\\", 1) != 1)
-					return 0;
-			if (write_in_full(fd, &c, 1) != 1)
-				return 0;
-		}
-		if (write_in_full(fd, "\"", 1) != 1)
-			return 0;
-	}
-	if (write_in_full(fd, "]\n", 2) != 2)
-		return 0;
+	success = write_in_full(fd, sb.buf, sb.len) == sb.len;
+	strbuf_release(&sb);
 
-	return 1;
+	return success;
 }
 
 static int store_write_pair(int fd, const char* key, const char* value)
 {
-	int i;
-	int length = strlen(key+store.baselen+1);
-	int quote = 0;
+	int i, success;
+	int length = strlen(key + store.baselen + 1);
+	const char *quote = "";
+	struct strbuf sb;
 
 	/*
 	 * Check to see if the value needs to be surrounded with a dq pair.
@@ -659,43 +649,38 @@ static int store_write_pair(int fd, const char* key, const char* value)
 	 * configuration parser.
 	 */
 	if (value[0] == ' ')
-		quote = 1;
+		quote = "\"";
 	for (i = 0; value[i]; i++)
 		if (value[i] == ';' || value[i] == '#')
-			quote = 1;
-	if (i && value[i-1] == ' ')
-		quote = 1;
+			quote = "\"";
+	if (i && value[i - 1] == ' ')
+		quote = "\"";
+
+	strbuf_init(&sb, 0);
+	strbuf_addf(&sb, "\t%.*s = %s", 
+		    length, key + store.baselen + 1, quote);
 
-	if (write_in_full(fd, "\t", 1) != 1 ||
-	    write_in_full(fd, key+store.baselen+1, length) != length ||
-	    write_in_full(fd, " = ", 3) != 3)
-		return 0;
-	if (quote && write_in_full(fd, "\"", 1) != 1)
-		return 0;
 	for (i = 0; value[i]; i++)
 		switch (value[i]) {
 		case '\n':
-			if (write_in_full(fd, "\\n", 2) != 2)
-				return 0;
+			strbuf_addstr(&sb, "\\n");
 			break;
 		case '\t':
-			if (write_in_full(fd, "\\t", 2) != 2)
-				return 0;
+			strbuf_addstr(&sb, "\\t");
 			break;
 		case '"':
 		case '\\':
-			if (write_in_full(fd, "\\", 1) != 1)
-				return 0;
+			strbuf_addch(&sb, '\\');
 		default:
-			if (write_in_full(fd, value+i, 1) != 1)
-				return 0;
+			strbuf_addch(&sb, value[i]);
 			break;
 		}
-	if (quote && write_in_full(fd, "\"", 1) != 1)
-		return 0;
-	if (write_in_full(fd, "\n", 1) != 1)
-		return 0;
-	return 1;
+	strbuf_addf(&sb, "%s\n", quote);
+
+	success = write_in_full(fd, sb.buf, sb.len) == sb.len;
+	strbuf_release(&sb);
+
+	return success;
 }
 
 static ssize_t find_beginning_of_line(const char* contents, size_t size,
-- 
1.5.3.4

^ permalink raw reply related

* Re: [BUG] Failed to make install-info
From: Junio C Hamano @ 2007-12-14 17:45 UTC (permalink / raw)
  To: Jon Loeliger; +Cc: Christian Couder, git
In-Reply-To: <E1J3Bg0-0004Ut-Ii@jdl.com>

Jon Loeliger <jdl@jdl.com> writes:

> So, like, the other day Junio C Hamano mumbled:
>> 
>> I personally feel that calling "install-info" is a bit too distro-ish
>> and should not be done in our Makefile vanilla build-and-install
>> procedure.
>
> Hmmm...  That's an interesting take on things
> given the top of the INSTALL file:
>
> 		    Git installation
>
>     Normally you can just do "make" followed by "make install", and that
>     will install the git programs in your own ~/bin/ directory.  If you want
>     to do a global install, you can do
>
> 	    $ make prefix=/usr all doc info ;# as yourself
> 	    # make prefix=/usr install install-doc install-info ;# as root

I was not talking about install-info target that copies generated *.info
files to $(DESTDIR)$(infodir)/.  That does not sound distro-ish at all
to me.

What I found questionable was $(INSTALL_INFO) invocation in that target
in the Makefile.

^ permalink raw reply

* Re: git-browse-help?
From: Junio C Hamano @ 2007-12-14 17:38 UTC (permalink / raw)
  To: Jeff King; +Cc: Christian Couder, git
In-Reply-To: <20071214092829.GA22725@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> The new 'git-browse-help' seems to purely be a helper script for "git
> help" (judging from the commit messages and the lack of documentation).
> Should it perhaps be "git--browse-help" or "git-help--browse" to follow
> our "this is not for users" convention (see add--interactive,
> fetch--tool, and rebase--interactive).
>
> I became aware of it because "br" used to tab-complete "branch" but now
> doesn't. :) And if we're going to change it, pre-1.5.4 is the time.

Good eyes and a sensible argument.  Go wild.

^ permalink raw reply

* Re: Something is broken in repack
From: Wolfram Gloger @ 2007-12-14 16:59 UTC (permalink / raw)
  To: dak; +Cc: wmglo, torvalds, nico, jonsmirl, gitster, gcc, git
In-Reply-To: <85r6hptecs.fsf@lola.goethe.zz>

Hi,

> Uh what?  Someone crank out his copy of "The Art of Computer
> Programming", I think volume 1.  Best fit is known (analyzed and proven
> and documented decades ago) to be one of the worst strategies for memory
> allocation.  Exactly because it leads to huge fragmentation problems.

Well, quoting http://gee.cs.oswego.edu/dl/html/malloc.html:

"As shown by Wilson et al, best-fit schemes (of various kinds and
approximations) tend to produce the least fragmentation on real loads
compared to other general approaches such as first-fit."

See [Wilson 1995] ftp://ftp.cs.utexas.edu/pub/garbage/allocsrv.ps for
more details and references.

Regards,
Wolfram.

^ permalink raw reply

* Re: Something is broken in repack
From: David Kastrup @ 2007-12-14 16:45 UTC (permalink / raw)
  To: Wolfram Gloger; +Cc: torvalds, nico, jonsmirl, gitster, gcc, git
In-Reply-To: <20071214161236.3080.qmail@md.dent.med.uni-muenchen.de>

Wolfram Gloger <wmglo@dent.med.uni-muenchen.de> writes:

> Hi,
>
>> Note that delta following involves patterns something like
>> 
>>    allocate (small) space for delta
>>    for i in (1..depth) {
>> 	allocate large space for base
>> 	allocate large space for result
>> 	.. apply delta ..
>> 	free large space for base
>> 	free small space for delta
>>    }
>> 
>> so if you have some stupid heap algorithm that doesn't try to merge and 
>> re-use free'd spaces very aggressively (because that takes CPU time!),
>
> ptmalloc2 (in glibc) _per arena_ is basically best-fit.  This is the
> best known general strategy,

Uh what?  Someone crank out his copy of "The Art of Computer
Programming", I think volume 1.  Best fit is known (analyzed and proven
and documented decades ago) to be one of the worst strategies for memory
allocation.  Exactly because it leads to huge fragmentation problems.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply

* Re: [PATCH] git-svn: handle our top-level path is deleted and later re-added
From: Eric Wong @ 2007-12-14 16:43 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Peter Baumann, Marcus D. Hanwell, git
In-Reply-To: <20071214163909.GA18300@soma>

> Subject: [PATCH] git-svn: handle our top-level path is deleted and later re-added

That should read:

Subject: [PATCH] git-svn: handle our top-level path being deleted and later re-added

> ---
>   Peter Baumann <waste.manager@gmx.de> wrote:
>   > On Thu, Dec 13, 2007 at 08:41:34AM -0800, Eric Wong wrote:

>   > > Instead of handling the full delete of everything in r68, I think I'll
>   > > make it so importing r75 will clobber all the stuff in trunk/ we ignored
>   > > deleting in r68 before adding the contents of trunk/ in r75.
>   > > 
>   > 
>   > Sorry, that last sentence I can't parse/understand. But I trust you to do the
>   > right thing.
> 
>   I hope the commit message was a better explanation.  Or the code
>   and test case I've added makes perfect sense :)

My language generation module seems broken these days.

-- 
Eric Wong

^ permalink raw reply

* [PATCH] git-svn: handle our top-level path is deleted and later re-added
From: Eric Wong @ 2007-12-14 16:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Peter Baumann, Marcus D. Hanwell, git
In-Reply-To: <20071213181312.GA4441@xp.machine.xx>

Previously, git-svn would ignore cases where the path we're
tracking is removed from the repository.  This was to prevent
heads with follow-parent from ending up with a tree full of
empty revisions (and thus breaking rename detection).

The previous behavior is fine until the path we're tracking
is re-added later on, leading to the old files being merged
in with the new files in the directory (because the old
files were never marked as deleted)

We will now only remove all the old files locally that were
deleted remotely iff we detect the directory we're in is being
created from scratch.

Thanks for Marcus D. Hanwell for the bug report and
Peter Baumann for the analysis.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
  Peter Baumann <waste.manager@gmx.de> wrote:
  > On Thu, Dec 13, 2007 at 08:41:34AM -0800, Eric Wong wrote:
  > > Peter Baumann <waste.manager@gmx.de> wrote:
  > > > On Sun, Dec 09, 2007 at 07:24:40PM -0500, Marcus D. Hanwell wrote:
  > > > /trunk and /tags are moved, too.
  > > > (/trunk/avogadro.pro becomes /avogadro/trunk/avogadro.pro):
  > > >     ------------------------------------------------------------------------
  > > >     r68 | dcurtis3 | 2007-01-03 06:47:34 +0100 (Wed, 03 Jan 2007) | 3 lines
  > > >     Changed paths:
  > > >        A /avogadro/tags (from /tags:66)
  > > >        A /avogadro/trunk (from /trunk:66)
  > > >        D /tags
  > > >        D /trunk
  > > > 
  > > >     Making room for libavogadro.
  > > > 
  > > > The above delete/move of trunk isn't recorded anywhere in the git svn import.
  > > > 'git-svn find-rev r66' doesn't produce any output!
  > > > And later git-svn thinks that /trunk and all its files are still there, so
  > > > e.g. /trunk/avogadro.pro stays in the repo forever.
  > > 
  > > Hi,
  > > 
  > > Thanks for the analysis, Peter.
  > > 
  > > git-svn ignores deletions to the directory we're tracking in order to
  > > represent renames when the follow-parent case is hit.
  > > 
  > > Unfortunately, this never took into account the directory we're tracking
  > > reappearing later in history.  I'll try to have a fix later tonight or
  > > over the weekend.
  > > 
  > > > mv /avogadro/trunk/avogadro.pro /trunk/avogadro/avogadro.pro
  > > >    ------------------------------------------------------------------------
  > > >     r75 | dcurtis3 | 2007-01-03 20:49:35 +0100 (Wed, 03 Jan 2007) | 2 lines
  > > >     Changed paths:
  > > >        D /avogadro
  > > >        D /avogadro-lib
  > > >        A /branches
  > > >        A /tags
  > > >        A /trunk
  > > >        A /trunk/avogadro (from /avogadro/trunk:74)
  > > >        A /trunk/libavogadro (from /avogadro-lib/trunk:74)
  > > > 
  > > >     Moving things around.  Conforming to a more KDE-like layout in SVN.
  > > >     I'm going nuts.  SOrry
  > > 
  > > Instead of handling the full delete of everything in r68, I think I'll
  > > make it so importing r75 will clobber all the stuff in trunk/ we ignored
  > > deleting in r68 before adding the contents of trunk/ in r75.
  > > 
  > 
  > Sorry, that last sentence I can't parse/understand. But I trust you to do the
  > right thing.

  I hope the commit message was a better explanation.  Or the code
  and test case I've added makes perfect sense :)

 git-svn.perl                                 |   14 +++++++++
 t/t9103-git-svn-tracked-directory-removed.sh |   39 ++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 0 deletions(-)
 create mode 100755 t/t9103-git-svn-tracked-directory-removed.sh

diff --git a/git-svn.perl b/git-svn.perl
index 3aa7f8c..d411a34 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -3045,6 +3045,20 @@ sub add_file {
 
 sub add_directory {
 	my ($self, $path, $cp_path, $cp_rev) = @_;
+	my $gpath = $self->git_path($path);
+	if ($gpath eq '') {
+		my ($ls, $ctx) = command_output_pipe(qw/ls-tree
+		                                     -r --name-only -z/,
+				                     $self->{c});
+		local $/ = "\0";
+		while (<$ls>) {
+			chomp;
+			$self->{gii}->remove($_);
+			print "\tD\t$_\n" unless $::_q;
+		}
+		command_close_pipe($ls, $ctx);
+		$self->{empty}->{$path} = 0;
+	}
 	my ($dir, $file) = ($path =~ m#^(.*?)/?([^/]+)$#);
 	delete $self->{empty}->{$dir};
 	$self->{empty}->{$path} = 1;
diff --git a/t/t9103-git-svn-tracked-directory-removed.sh b/t/t9103-git-svn-tracked-directory-removed.sh
new file mode 100755
index 0000000..0f0b0fd
--- /dev/null
+++ b/t/t9103-git-svn-tracked-directory-removed.sh
@@ -0,0 +1,39 @@
+#!/bin/sh
+#
+# Copyright (c) 2007 Eric Wong
+#
+
+test_description='git-svn tracking removed top-level path'
+. ./lib-git-svn.sh
+
+test_expect_success 'make history for tracking' '
+	mkdir import &&
+	mkdir import/trunk &&
+	echo hello >> import/trunk/README &&
+	svn import -m initial import $svnrepo &&
+	rm -rf import &&
+	svn co $svnrepo/trunk trunk &&
+	echo bye bye >> trunk/README &&
+	svn rm -m "gone" $svnrepo/trunk &&
+	rm -rf trunk &&
+	mkdir trunk &&
+	echo "new" > trunk/FOLLOWME &&
+	svn import -m "new trunk" trunk $svnrepo/trunk
+'
+
+test_expect_success 'clone repo with git' '
+	git svn clone -s $svnrepo x &&
+	test -f x/FOLLOWME &&
+	test ! -f x/README
+'
+
+test_expect_success 'make sure r2 still has old file' '
+	cd x &&
+		test -n "$(git svn find-rev r1)" &&
+		git reset --hard $(git svn find-rev r1) &&
+		test -f README &&
+		test ! -f FOLLOWME &&
+		test x$(git svn find-rev r2) = x
+'
+
+test_done
-- 
Eric Wong

^ permalink raw reply related

* Re: Something is broken in repack
From: Wolfram Gloger @ 2007-12-14 16:18 UTC (permalink / raw)
  To: dak; +Cc: nico, jonsmirl, gitster, gcc, git
In-Reply-To: <85d4tc8hi8.fsf@lola.goethe.zz>

Hi,

> Maybe an malloc/free/mmap wrapper that records the requested sizes and
> alloc/free order and dumps them to file so that one can make a compact
> git-free standalone test case for the glibc maintainers might be a good
> thing.

I already have such a wrapper:

http://malloc.de/malloc/mtrace-20060529.tar.gz

But note that it does interfere with the thread scheduling, so it
can't record the exact same allocation pattern as when not using the
wrapper.

Regards,
Wolfram.

^ permalink raw reply

* Re: Something is broken in repack
From: Wolfram Gloger @ 2007-12-14 16:12 UTC (permalink / raw)
  To: torvalds; +Cc: nico, jonsmirl, gitster, gcc, git
In-Reply-To: <alpine.LFD.0.9999.0712120826440.25032@woody.linux-foundation.org>

Hi,

> Note that delta following involves patterns something like
> 
>    allocate (small) space for delta
>    for i in (1..depth) {
> 	allocate large space for base
> 	allocate large space for result
> 	.. apply delta ..
> 	free large space for base
> 	free small space for delta
>    }
> 
> so if you have some stupid heap algorithm that doesn't try to merge and 
> re-use free'd spaces very aggressively (because that takes CPU time!),

ptmalloc2 (in glibc) _per arena_ is basically best-fit.  This is the
best known general strategy, but it certainly cannot be the best in
every case.

> you 
> might have memory usage be horribly inflated by the heap having all those 
> holes for all the objects that got free'd in the chain that don't get 
> aggressively re-used.

It depends how large 'large' is -- if it exceeds the mmap() threshold
(settable with mallopt(M_MMAP_THRESHOLD, ...))
the 'large' spaces will be allocated with mmap() and won't cause
any internal fragmentation.
It might pay to experiment with this parameter if it is hard to
avoid the alloc/free large space sequence.

> Threaded memory allocators then make this worse by probably using totally 
> different heaps for different threads (in order to avoid locking), so they 
> will *all* have the fragmentation issue.

Indeed.

Could someone perhaps try ptmalloc3
(http://malloc.de/malloc/ptmalloc3-current.tar.gz) on this case?

Thanks,
Wolfram.

^ permalink raw reply

* Re: Something is broken in repack
From: Wolfram Gloger @ 2007-12-14 16:03 UTC (permalink / raw)
  To: ae; +Cc: nico, jonsmirl, gitster, gcc, git
In-Reply-To: <4760E005.6040102@op5.se>

Hi,

> >>  	if (progress->total) {
> >>  		unsigned percent = n * 100 / progress->total;
> >>  		if (percent != progress->last_percent || progress_update) {
> >> +			struct mallinfo m = mallinfo();
> >>  			progress->last_percent = percent;
> >> -			fprintf(stderr, "%s: %3u%% (%u/%u)%s%s",
> >> -				progress->title, percent, n,
> >> -				progress->total, tp, eol);
> >> +			fprintf(stderr, "%s: %3u%% (%u/%u) %u/%uMB%s%s",
> >> +				progress->title, percent, n, progress->total,
> >> +				m.uordblks >> 18, m.fordblks >> 18,
> >> +				tp, eol);
> > 
> > Note: I didn't know what unit of memory those blocks represents, so the 
> > shift is most probably wrong.
> > 
> 
> Me neither, but it appears to me as if hblkhd holds the actual memory
> consumed by the process. It seems to store the information in bytes,
> which I find a bit dubious unless glibc has some internal multiplier.

mallinfo() will only give you the used memory for the main arena.
When you have separate arenas (likely when concurrent threads have
been used), the only way to get the full picture is to call
malloc_stats(), which prints to stderr.

Regards,
Wolfram.

^ permalink raw reply

* Re: StGIT: "stg refresh -e/--edit" to refresh patch and edit description no longer works
From: Jakub Narebski @ 2007-12-14 16:01 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git
In-Reply-To: <b0943d9e0712140626w3a2f55b1i83c33f93d7f8af2f@mail.gmail.com>

On Fri, 14 Dec 2007, Catalin Marinas wrote:
> On 14/12/2007, Jakub Narebski <jnareb@gmail.com> wrote:
>>
>> The command I used quite often in my StGIT workflow, namely
>> "stg refresh -e", which used to refresh patch and launch editor to edit
>> commit (patch) description no longer works in Stacked GIT 0.14.1.
>>
>> I used it when new (refreshed) changes needed also changes in commit
>> description (commit message). Now I have to run two commands:
>> "stg refresh" and "stg edit".
> 
> Well, people complained that 'refresh -e' is misleading and a separate
> command is needed. It's not difficult to move the editing
> functionality out of stgit.commands.edit and re-add the --edit option
> to refresh (while also keeping the separate edit command).

First, the "misleading" part could be corrected by renaming long option
to --edit-description or --edit-message. Second "stg refresh -e" matches
non-StGIT usage of "git commit --amend"; and it is damn useful to have
it as one command, not two. 

> I'm a bit busy with home/family commitments until the new year.

I'll wait. As I am not running StGIT master, but released version,
I would have to waith nevertheless on next release.

TIA
-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [PATCH] provide advance warning of some future pack default changes
From: Nicolas Pitre @ 2007-12-14 15:20 UTC (permalink / raw)
  To: linux; +Cc: git
In-Reply-To: <20071214112814.11083.qmail@science.horizon.com>

On Fri, 14 Dec 2007, linux@horizon.com wrote:

> >+ * From v1.5.5, the pack.indexversion config option will default to 2,
> >+   which is slightly more efficient, and makes repacking more immune to
> >+   data corruptions.  Git older than version 1.5.2 may revert to version 1
> >+   of the pack index with a manual "git index-pack" to be able to directly
> >+   access corresponding pack files.
> 
> You might want to mention that it's slightly more TIME efficient,
> but takes 16% more space (28 bytes per object rather than 24).

Well, sure, but not now.  This is just an advance warning of what 
the next release after this one will do.

> If it helps, I documented the v2 index file format (a lot stolen
> from commit c553ca25bd60dc9fd50b8bc7bd329601b81cee66 message).
> (Public domain, copyright abandoned, if it breaks you get to keep both
> pieces, yadda yadda.)

If anything:

Acked-by: Nicolas Pitre <nico@cam.org>


> diff --git a/Documentation/technical/pack-format.txt b/Documentation/technical/pack-format.txt
> index e5b31c8..a80baa4 100644
> --- a/Documentation/technical/pack-format.txt
> +++ b/Documentation/technical/pack-format.txt
> @@ -1,9 +1,9 @@
>  GIT pack format
>  ===============
>  
> -= pack-*.pack file has the following format:
> += pack-*.pack files have the following format:
>  
> -   - The header appears at the beginning and consists of the following:
> +   - A header appears at the beginning and consists of the following:
>  
>       4-byte signature:
>           The signature is: {'P', 'A', 'C', 'K'}
> @@ -34,18 +34,14 @@ GIT pack format
>  
>    - The trailer records 20-byte SHA1 checksum of all of the above.
>  
> -= pack-*.idx file has the following format:
> += Original (version 1) pack-*.idx files have the following format:
>  
>    - The header consists of 256 4-byte network byte order
>      integers.  N-th entry of this table records the number of
>      objects in the corresponding pack, the first byte of whose
> -    object name are smaller than N.  This is called the
> +    object name is less than or equal to N.  This is called the
>      'first-level fan-out' table.
>  
> -    Observation: we would need to extend this to an array of
> -    8-byte integers to go beyond 4G objects per pack, but it is
> -    not strictly necessary.
> -
>    - The header is followed by sorted 24-byte entries, one entry
>      per object in the pack.  Each entry is:
>  
> @@ -55,10 +51,6 @@ GIT pack format
>  
>      20-byte object name.
>  
> -    Observation: we would definitely need to extend this to
> -    8-byte integer plus 20-byte object name to handle a packfile
> -    that is larger than 4GB.
> -
>    - The file is concluded with a trailer:
>  
>      A copy of the 20-byte SHA1 checksum at the end of
> @@ -68,31 +60,30 @@ GIT pack format
>  
>  Pack Idx file:
>  
> -	idx
> -	    +--------------------------------+
> -	    | fanout[0] = 2                  |-.
> -	    +--------------------------------+ |
> +	--  +--------------------------------+
> +fanout	    | fanout[0] = 2 (for example)    |-.
> +table	    +--------------------------------+ |
>  	    | fanout[1]                      | |
>  	    +--------------------------------+ |
>  	    | fanout[2]                      | |
>  	    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
> -	    | fanout[255]                    | |
> -	    +--------------------------------+ |
> -main	    | offset                         | |
> -index	    | object name 00XXXXXXXXXXXXXXXX | |
> -table	    +--------------------------------+ |
> -	    | offset                         | |
> -	    | object name 00XXXXXXXXXXXXXXXX | |
> -	    +--------------------------------+ |
> -	  .-| offset                         |<+
> -	  | | object name 01XXXXXXXXXXXXXXXX |
> -	  | +--------------------------------+
> -	  | | offset                         |
> -	  | | object name 01XXXXXXXXXXXXXXXX |
> -	  | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> -	  | | offset                         |
> -	  | | object name FFXXXXXXXXXXXXXXXX |
> -	  | +--------------------------------+
> +	    | fanout[255] = total objects    |---.
> +	--  +--------------------------------+ | |
> +main	    | offset                         | | |
> +index	    | object name 00XXXXXXXXXXXXXXXX | | |
> +table	    +--------------------------------+ | |
> +	    | offset                         | | |
> +	    | object name 00XXXXXXXXXXXXXXXX | | |
> +	    +--------------------------------+<+ |
> +	  .-| offset                         |   |
> +	  | | object name 01XXXXXXXXXXXXXXXX |   |
> +	  | +--------------------------------+   |
> +	  | | offset                         |   |
> +	  | | object name 01XXXXXXXXXXXXXXXX |   |
> +	  | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~   |
> +	  | | offset                         |   |
> +	  | | object name FFXXXXXXXXXXXXXXXX |   |
> +	--| +--------------------------------+<--+
>  trailer	  | | packfile checksum              |
>  	  | +--------------------------------+
>  	  | | idxfile checksum               |
> @@ -116,3 +107,40 @@ Pack file entry: <+
>  	  20-byte base object name SHA1 (the size above is the
>  		size of the delta data that follows).
>            delta data, deflated.
> +
> +
> += Version 2 pack-*.idx files support packs larger than 4 GiB, and
> +  have some other reorganizations.  They have the format:
> +
> +  - A 4-byte magic number '\377tOc' which is an unreasonable
> +    fanout[0] value.
> +
> +  - A 4-byte version number (= 2)
> +
> +  - A 256-entry fan-out table just like v1.
> +
> +  - A table of sorted 20-byte SHA1 object names.  These are
> +    packed together without offset values to reduce the cache
> +    footprint of the binary search for a specific object name.
> +
> +  - A table of 4-byte CRC32 values of the packed object data.
> +    This is new in v2 so compressed data can be copied directly
> +    from pack to pack during repacking withough undetected
> +    data corruption.
> +
> +  - A table of 4-byte offset values (in network byte order).
> +    These are usually 31-bit pack file offsets, but large
> +    offsets are encoded as an index into the next table with
> +    the msbit set.
> +
> +  - A table of 8-byte offset entries (empty for pack files less
> +    than 2 GiB).  Pack files are organized with heavily used
> +    objects toward the front, so most object references should
> +    not need to refer to this table.
> +
> +  - The same trailer as a v1 pack file:
> +
> +    A copy of the 20-byte SHA1 checksum at the end of
> +    corresponding packfile.
> +
> +    20-byte SHA1-checksum of all of the above.
> -
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


Nicolas

^ permalink raw reply

* Re: [BUG] Failed to make install-info
From: Jon Loeliger @ 2007-12-14 14:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Christian Couder, git
In-Reply-To: <7vejdpvmlw.fsf@gitster.siamese.dyndns.org>

So, like, the other day Junio C Hamano mumbled:
> 
> I personally feel that calling "install-info" is a bit too distro-ish
> and should not be done in our Makefile vanilla build-and-install
> procedure.

Hmmm...  That's an interesting take on things
given the top of the INSTALL file:

		    Git installation

    Normally you can just do "make" followed by "make install", and that
    will install the git programs in your own ~/bin/ directory.  If you want
    to do a global install, you can do

	    $ make prefix=/usr all doc info ;# as yourself
	    # make prefix=/usr install install-doc install-info ;# as root

Granted, it's the first time I've actually tried
to install the docs on this (debian) machine too.
I've normally just read them straight out of the
doc directory. :-)

But I will have to agree with you on the problems
surrounding the One Install to Rule Them All (and
in the darkness bind9 them).

jdl

^ permalink raw reply

* Re: StGIT: "stg refresh -e/--edit" to refresh patch and edit description no longer works
From: Catalin Marinas @ 2007-12-14 14:26 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <200712141507.01552.jnareb@gmail.com>

On 14/12/2007, Jakub Narebski <jnareb@gmail.com> wrote:
> The command I used quite often in my StGIT workflow, namely
> "stg refresh -e", which used to refresh patch and launch editor to edit
> commit (patch) description no longer works in Stacked GIT 0.14.1.
>
> I used it when new (refreshed) changes needed also changes in commit
> description (commit message). Now I have to run two commands:
> "stg refresh" and "stg edit".

Well, people complained that 'refresh -e' is misleading and a separate
command is needed. It's not difficult to move the editing
functionality out of stgit.commands.edit and re-add the --edit option
to refresh (while also keeping the separate edit command).

I'm a bit busy with home/family commitments until the new year.

-- 
Catalin

^ permalink raw reply

* Re: relative objects/info/alternates
From: Jakub Narebski @ 2007-12-14 14:13 UTC (permalink / raw)
  To: Luciano Rocha; +Cc: git
In-Reply-To: <20071214140613.GA13259@bit.office.eurotux.com>

Luciano Rocha <luciano@eurotux.com> writes:

> Can I use relative $GIT_DIR/objects/info/alternates? Will the git
> commands work anywhere inside the working-copy despice the alternate
> being relative to the root of it?

>From the information in repository layout documentation:

  objects/info/alternates::
        This file records paths to alternate object stores that
        this object store borrows objects from, one pathname per
        line. Note that not only native Git tools use it locally,
        but the HTTP fetcher also tries to use it remotely; this
        will usually work if you have relative paths (relative
        to the object database, not to the repository!) in your
        alternates file, but it will not work if you use absolute
        paths unless the absolute path in filesystem and web URL
        is the same. See also 'objects/info/http-alternates'.

it looks like you can have relative paths in alternates.

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* StGIT: "stg refresh -e/--edit" to refresh patch and edit description no longer works
From: Jakub Narebski @ 2007-12-14 14:07 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git

The command I used quite often in my StGIT workflow, namely 
"stg refresh -e", which used to refresh patch and launch editor to edit 
commit (patch) description no longer works in Stacked GIT 0.14.1.

I used it when new (refreshed) changes needed also changes in commit 
description (commit message). Now I have to run two commands: 
"stg refresh" and "stg edit".

-- 
Jakub Narebski
Poland

^ permalink raw reply

* relative objects/info/alternates
From: Luciano Rocha @ 2007-12-14 14:06 UTC (permalink / raw)
  To: git

[-- Attachment #1: Type: text/plain, Size: 308 bytes --]


Hello,

Can I use relative $GIT_DIR/objects/info/alternates? Will the git
commands work anywhere inside the working-copy despice the alternate
being relative to the root of it?

Regards,
Luciano Rocha

-- 
Luciano Rocha <luciano@eurotux.com>
Eurotux Informática, S.A. <http://www.eurotux.com/>

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: [Funky] "git -p cmd" inside a bare repository
From: Nguyen Thai Ngoc Duy @ 2007-12-14 13:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Johannes Schindelin
In-Reply-To: <7vmysexdvw.fsf@gitster.siamese.dyndns.org>

On Dec 14, 2007 8:29 AM, Junio C Hamano <gitster@pobox.com> wrote:
> If you have a bare repository and try this there:
>
>         $ PAGER=head git show HEAD:gcc/ChangeLog
>
> it works as expected, but if you explicitly ask for pagination, like
> this:
>
>         $ PAGER=head git -p show HEAD:gcc/ChangeLog
>
> you would get a very funky error message:
>
>         fatal: ambiguous argument 'HEAD:gcc/ChangeLog': unknown revision or path not in the working tree.
>         Use '--' to separate paths from revisions
>
> as if we are working with a repository with working tree.
>
> I originally noticed this with ls-tree.  The symptom is a bit different:
>
>         $ git -p ls-tree HEAD
>         fatal: Not a valid object name HEAD
>
>
>
> I _think_ what is happening is that setup_pager() tries to run
> git_config(), which runs setup(), and then RUN_SETUP set for "ls-tree"
> (or "show") internal command runs setup again.  HEAD is given to
> resolve_ref() and git_path("%s", ref) makes it to ".git/HEAD", even
> though in a bare repository git_dir should be set to ".", and of course
> we cannot find such a path in the git directory.

You meant setup_git_directory() or setup_git_env()? If find that
setup_git_env() is called too soon so it caches wrong git_dir and
friends if you setup_pager().

Another option (BTW I think your patch is fine) is to call
setup_git_env one more time to refresh its cache.
-- 
Duy

^ permalink raw reply

* Re: git-cvsexportcommit fails for huge commits
From: Robin Rosenberg @ 2007-12-14 13:47 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Markus Klinik, git
In-Reply-To: <20071214091546.GA20907@coredump.intra.peff.net>

fredag 14 december 2007 skrev Jeff King:
> On Thu, Dec 13, 2007 at 11:45:54PM -0500, Jeff King wrote:
> 
> > So it seems that we could probably go with something more like 64K, and
> > then only truly pathological cases should trigger the behavior.
> 
> So here is a cleaned up patch. It bumps the maximum size to 64kB, adds
> scalar support (nobody uses it, but it makes sense for the interface to
> match that of safe_pipe_capture -- I am even tempted to just replace
> safe_pipe_capture entirely and convert the few other callers), and
> cleans up the unused safe_pipe_capture_blob.

Wouldn't using the POSIX::ARG_MAX constant work?

-- robin

^ 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