Git development
 help / color / mirror / Atom feed
* [PATCH 2/4] Documentation: remove a redundant elaboration
From: Anders Melchiorsen @ 2009-01-18 14:36 UTC (permalink / raw)
  To: gitster; +Cc: git, Anders Melchiorsen
In-Reply-To: <1232289418-25627-2-git-send-email-mail@cup.kalibalik.dk>

The comment in parentheses is wrong, as one has to leave out both the
colon and <dst>. This situation is covered by the section a few lines
down:

  A parameter <ref> without a colon pushes the <ref> from the source
  repository to the destination repository under the same name.

So, just remove the parentheses.

Signed-off-by: Anders Melchiorsen <mail@cup.kalibalik.dk>
---
 Documentation/git-push.txt |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 8bfa7cb..6d3c711 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -44,8 +44,7 @@ tip of `master` branch); see linkgit:git-rev-parse[1]) that you
 want to push.  The <dst> side represents the destination location.
 +
 The local ref that matches <src> is used
-to fast forward the remote ref that matches <dst> (or, if no <dst> was
-specified, the same ref that <src> referred to locally).  If
+to fast forward the remote ref that matches <dst>.  If
 the optional leading plus `+` is used, the remote ref is updated
 even if it does not result in a fast forward update.
 +
-- 
1.6.0.2.514.g23abd3

^ permalink raw reply related

* [PATCH 3/4] Documentation: mention branches rather than heads
From: Anders Melchiorsen @ 2009-01-18 14:36 UTC (permalink / raw)
  To: gitster; +Cc: git, Anders Melchiorsen
In-Reply-To: <1232289418-25627-3-git-send-email-mail@cup.kalibalik.dk>

Most of the git push page talks about branches, so make it consistent
also in this paragraph.

Signed-off-by: Anders Melchiorsen <mail@cup.kalibalik.dk>
---
 Documentation/git-push.txt |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 6d3c711..a7a6f4c 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -57,8 +57,8 @@ Pushing an empty <src> allows you to delete the <dst> ref from
 the remote repository.
 +
 The special refspec `:` (or `+:` to allow non-fast forward updates)
-directs git to push "matching" heads: for every head that exists on
-the local side, the remote side is updated if a head of the same name
+directs git to push "matching" branches: for every branch that exists on
+the local side, the remote side is updated if a branch of the same name
 already exists on the remote side.  This is the default operation mode
 if no explicit refspec is found (that is neither on the command line
 nor in any Push line of the corresponding remotes file---see below).
-- 
1.6.0.2.514.g23abd3

^ permalink raw reply related

* Re: [PATCH] http-push: support full URI in handle_remote_ls_ctx()
From: Johannes Schindelin @ 2009-01-18 14:51 UTC (permalink / raw)
  To: Kirill A. Korinskiy; +Cc: Junio C Hamano, git
In-Reply-To: <1232278116-6631-1-git-send-email-catap@catap.ru>

Hi,

On Sun, 18 Jan 2009, Kirill A. Korinskiy wrote:

> The program calls remote_ls() to get list of files from the server
> over HTTP; handle_remote_ls_ctx() is used to parse its response to
> populate "struct remote_ls_ctx" that is returned from remote_ls().
> 
> The handle_remote_ls_ctx() function assumed that the server returns a
> local path in href field, but RFC 4918 (14.7) demand of support full
> URI (e.g. "http://localhost:8080/repo.git").
> 
> This resulted in push failure (e.g. git-http-push issues a PROPFIND
> request to "/repo.git/alhost:8080/repo.git/refs/" to the server).
> 
> Signed-off-by: Kirill A. Korinskiy <catap@catap.ru>
> ---
>  http-push.c |   24 ++++++++++++++++++------
>  1 files changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/http-push.c b/http-push.c
> index 7c6460919bf3eba10c46cede11ffdd9c53fd2dd2..2cb925a9ad857b6d79858d5187f14072167282e7 100644

You mean this patch is not vs 
junio/next(bbe7a8ed3dac72b7a1372cd92f68f47965c10100) or junio/master or 
junio/maint(both a4b7d08663504a57008f66a39fffe293f62c1d08) but 
tags/v1.6.1-rc4~13?

Ciao,
Dscho

^ permalink raw reply

* Re: What's cooking in git.git (Jan 2009, #03; Wed, 14)
From: Kirill Smelkov @ 2009-01-18 14:54 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0901161253411.3586@pacific.mpi-cbg.de>

On Fri, Jan 16, 2009 at 12:54:28PM +0100, Johannes Schindelin wrote:
> Hi,
> 
> On Fri, 16 Jan 2009, Junio C Hamano wrote:
> 
> > I thought there is somebody on this list who insists his name is of form:
> > 
> > 	From: A U Thor (MonikeR) <a.u@thor.xz>
> 
> It is Philippe Bruhat (BooK), who sometimes forgets the closing 
> parenthesis, and who is listed in .mailmap without the moniker.

So now I don't understand what to do.

>From one hand RFC822 says '(...)' is a comment, and from the other hand,
we have a use case where one guy wants this to stay.

For the record, here is the questionable patch. Any suggestion?

Thanks,
Kirill


commit 49bebfbe18dac296e5e246884bd98c1f90be9676
Author: Kirill Smelkov <kirr@landau.phys.spbu.ru>
Date:   Tue Jan 13 01:21:04 2009 +0300

    mailinfo: more smarter removal of rfc822 comments from 'From'
    
    As described in RFC822 (3.4.3 COMMENTS, and  A.1.4.), comments, as e.g.
    
        John (zzz) Doe <john.doe@xz> (Comment)
    
    should "NOT [be] included in the destination mailbox"
    
    We need this functionality to pass all RFC2047 based tests in the next commit.
    
    Signed-off-by: Kirill Smelkov <kirr@landau.phys.spbu.ru>

diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c
index dacc8ac..958c905 100644
--- a/builtin-mailinfo.c
+++ b/builtin-mailinfo.c
@@ -29,6 +29,9 @@ static struct strbuf **p_hdr_data, **s_hdr_data;
 #define MAX_HDR_PARSED 10
 #define MAX_BOUNDARIES 5
 
+static void cleanup_space(struct strbuf *sb);
+
+
 static void get_sane_name(struct strbuf *out, struct strbuf *name, struct strbuf *email)
 {
 	struct strbuf *src = name;
@@ -120,6 +123,33 @@ static void handle_from(const struct strbuf *from)
 		strbuf_setlen(&f, f.len - 1);
 	}
 
+	/* This still could not be finished for emails like
+	 *
+	 *	"John (zzz) Doe <john.doe@xz> (Comment)"
+	 *
+	 * The email part had already been removed, so let's kill comments as
+	 * well -- RFC822 says comments should not be present in destination
+	 * mailbox (3.4.3. Comments  and  A.1.4.)
+	 */
+	while (1) {
+		char *ta;
+
+		at = strchr(f.buf, '(');
+		if (!at)
+			break;
+		ta = strchr(at, ')');
+		if (!ta)
+			break;
+
+		strbuf_remove(&f, at - f.buf, ta-at + (*ta ? 1 : 0));
+	}
+
+	/* and let's finally cleanup spaces that were around (possibly
+	 * internal) comments
+	 */
+	cleanup_space(&f);
+	strbuf_trim(&f);
+
 	get_sane_name(&name, &f, &email);
 	strbuf_release(&f);
 }
diff --git a/t/t5100/sample.mbox b/t/t5100/sample.mbox
index 38725f3..4f80b82 100644
--- a/t/t5100/sample.mbox
+++ b/t/t5100/sample.mbox
@@ -2,10 +2,10 @@
 	
     
 From nobody Mon Sep 17 00:00:00 2001
-From: A
+From: A (zzz)
       U
       Thor
-      <a.u.thor@example.com>
+      <a.u.thor@example.com> (Comment)
 Date: Fri, 9 Jun 2006 00:44:16 -0700
 Subject: [PATCH] a commit.
 

^ permalink raw reply related

* Re: [PATCH] http-push: update memory allocation for http headers
From: Johannes Schindelin @ 2009-01-18 14:55 UTC (permalink / raw)
  To: Ray Chuan; +Cc: git
In-Reply-To: <be6fef0d0901180516q19327c18oa1d7ccc36ce3a87@mail.gmail.com>

Hi,

On Sun, 18 Jan 2009, Ray Chuan wrote:

> In 753bc91 (Remove the requirement opaquelocktoken uri scheme), the
> header strings were with the removal "opaquelocktoken:" (16
> characters).
> 
> Unfortunately, this was without the corresponding change in memory
> allocation. In this patch we update these allocations.

Rather than doing this (which will result in as error-prone code), why 
don't you replace those things by strbufs?

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH (topgit) 0/2] tg-patch: fix pagination
From: Kirill Smelkov @ 2009-01-18 15:04 UTC (permalink / raw)
  To: git; +Cc: pasky, martin f krafft
In-Reply-To: <cover.1231254832.git.kirr@landau.phys.spbu.ru>

On Tue, Jan 06, 2009 at 06:16:37PM +0300, Kirill Smelkov wrote:
> Kirill Smelkov (2):
>   Implement setup_pager just like in git
>   tg-patch: fix pagination
> 
>  tg-patch.sh |    3 +++
>  tg.sh       |   53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 56 insertions(+), 0 deletions(-)

Petr, Martin,

What's the state of this patch series?

It fixes `tg patch`, and it seems we addressed all uglyness in
setup_pager, so is there any other reason not to merge this?

Thanks,
Kirill

^ permalink raw reply

* Re: [WIP Patch 03/12] Two new functions for the http API
From: Johannes Schindelin @ 2009-01-18 15:03 UTC (permalink / raw)
  To: Mike Hommey; +Cc: git, gitster
In-Reply-To: <1232265877-3649-4-git-send-email-mh@glandium.org>

Hi,

On Sun, 18 Jan 2009, Mike Hommey wrote:

> http_get_strbuf and http_get_file allow respectively to retrieve contents of
> an URL to a strbuf or an opened file handle.

Yesss!  Why have you hidden them that long?

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH (topgit)] tg-patch: add support for generating patches against worktree and index
From: Kirill Smelkov @ 2009-01-18 15:06 UTC (permalink / raw)
  To: martin f krafft; +Cc: Petr Baudis, Git Mailing List
In-Reply-To: <20090108211149.GA19983@roro3>

On Fri, Jan 09, 2009 at 12:11:49AM +0300, Kirill Smelkov wrote:
> I'm sorry, but I've found a mistake in my code for case:
> 
> diff --git a/tg-patch.sh b/tg-patch.sh
> index db1ad09..d701c54 100644
> --- a/tg-patch.sh
> +++ b/tg-patch.sh
> @@ -17,8 +17,8 @@ while [ -n "$1" ]; do
>         case "$arg" in
>         -i)
>                 topic='(i)'
> -               diff_opts="$diff_opts --cached";;
> -               diff_committed_only=;
> +               diff_opts="$diff_opts --cached";
> +               diff_committed_only=;;
>         -w)
>                 topic='(w)'
>                 diff_committed_only=;;
> 
> 
> So here is corrected patch:
> 
> 
> From: Kirill Smelkov <kirr@landau.phys.spbu.ru>
> To: Petr Baudis <pasky@suse.cz>
> Cc: martin f krafft <madduck@madduck.net>
> Cc: Git Mailing List <git@vger.kernel.org>
> Subject: [PATCH (topgit)] tg-patch: add support for generating patches against worktree and index
> 
> This implements `tg patch -i` and `tg patch -w` to see current patch as
> generated against not-yet-committed index and worktree.
> 
> 
> NOTE: unfortunately `git cat-file blob <file>` does not provide an option
> to cat file from worktree (only from an object or from index), so I had to
> unroll my own `cat file topic:file` with special support for '(i)' and
> '(w)' topics.
> 
> Signed-off-by: Kirill Smelkov <kirr@landau.phys.spbu.ru>

[...]

Petr, Martin,

What's the state of this patch?

I don't understand why this gets ignored. Maybe I do something wrong?
...


Thanks,
Kirill

^ permalink raw reply

* Re: [PATCH v4 0/7] customizable --color-words
From: Santi Béjar @ 2009-01-18 15:05 UTC (permalink / raw)
  To: Thomas Rast
  Cc: git, Junio C Hamano, Johannes Schindelin, Boyd Stephen Smith Jr.,
	Teemu Likonen
In-Reply-To: <1232209788-10408-1-git-send-email-trast@student.ethz.ch>

2009/1/17 Thomas Rast <trast@student.ethz.ch>:
> Johannes Schindelin wrote:
>> Thomas, could you pick up the patches from my 'my-next' branch and
>> maintain an "official" topic branch?
>
> I cherry-picked the three commits you had there, and rebuilt on top.
> I pushed them to
>
>  git://repo.or.cz/git/trast.git tr/word-diff-p2
>
> again (js/word-diff-p1 again points directly at your half).

I've tested tr/word-diff-p2 and I have not found any issues. I've even
tested that nothing changed from the tradicional word diff to:

git log -p --color-words="[^[:space:]]+"

for the whole git history.

At the end I've found that a general regex that works best for me is:

"[[:alpha:]]+|[[:digit:]]+|[^[:alnum:][:space:]]"

and that is what I tested.

Santi

^ permalink raw reply

* Re: [WIP Patch 04/12] Use the new http API in http_fetch_ref()
From: Johannes Schindelin @ 2009-01-18 15:10 UTC (permalink / raw)
  To: Mike Hommey; +Cc: git, gitster
In-Reply-To: <1232265877-3649-5-git-send-email-mh@glandium.org>

Hi,

On Sun, 18 Jan 2009, Mike Hommey wrote:

> diff --git a/http.c b/http.c
> index 82534cf..0c9504b 100644
> --- a/http.c
> +++ b/http.c
> @@ -604,34 +604,17 @@ int http_fetch_ref(const char *base, struct ref *ref)
>  {
>  	char *url;
>  	struct strbuf buffer = STRBUF_INIT;
> -	struct active_request_slot *slot;
> -	struct slot_results results;
> -	int ret;
> +	int ret = -1;
>  
>  	url = quote_ref_url(base, ref->name);
> -	slot = get_active_slot();
> -	slot->results = &results;
> -	curl_easy_setopt(slot->curl, CURLOPT_FILE, &buffer);
> -	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);
> -	curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, NULL);
> -	curl_easy_setopt(slot->curl, CURLOPT_URL, url);
> -	if (start_active_slot(slot)) {
> -		run_active_slot(slot);
> -		if (results.curl_result == CURLE_OK) {
> -			strbuf_rtrim(&buffer);
> -			if (buffer.len == 40)
> -				ret = get_sha1_hex(buffer.buf, ref->old_sha1);
> -			else if (!prefixcmp(buffer.buf, "ref: ")) {
> -				ref->symref = xstrdup(buffer.buf + 5);
> -				ret = 0;
> -			} else
> -				ret = 1;
> -		} else {
> -			ret = error("Couldn't get %s for %s\n%s",
> -				    url, ref->name, curl_errorstr);
> +	if (http_get_strbuf(url, &buffer, HTTP_NO_CACHE) == HTTP_OK) {
> +		strbuf_rtrim(&buffer);
> +		if (buffer.len == 40)
> +			ret = get_sha1_hex(buffer.buf, ref->old_sha1);
> +		else if (!prefixcmp(buffer.buf, "ref: ")) {
> +			ref->symref = xstrdup(buffer.buf + 5);
> +			ret = 0;
>  		}
> -	} else {
> -		ret = error("Unable to start request");
>  	}

Why not keep that error?

BTW I had to scratch my head for a few seconds why you do not need to 
set "else ret=1;" for the HTTP_OK case; you set ret = 1 in the beginning.  
I'd rather put that back for clarity.

Ciao,
Dscho

^ permalink raw reply

* Re: [WIP Patch 05/12] Use the new http API in get_refs_via_curl()
From: Johannes Schindelin @ 2009-01-18 15:12 UTC (permalink / raw)
  To: Mike Hommey; +Cc: git, gitster
In-Reply-To: <1232265877-3649-6-git-send-email-mh@glandium.org>

Hi,

On Sun, 18 Jan 2009, Mike Hommey wrote:

> @@ -450,26 +447,8 @@ static struct ref *get_refs_via_curl(struct transport *transport)
>  	refs_url = xmalloc(strlen(transport->url) + 11);
>  	sprintf(refs_url, "%s/info/refs", transport->url);
>  
> -	slot = get_active_slot();
> -	slot->results = &results;
> -	curl_easy_setopt(slot->curl, CURLOPT_FILE, &buffer);
> -	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);
> -	curl_easy_setopt(slot->curl, CURLOPT_URL, refs_url);
> -	curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, NULL);
> -
> -	if (start_active_slot(slot)) {
> -		run_active_slot(slot);
> -		if (results.curl_result != CURLE_OK) {
> -			strbuf_release(&buffer);
> -			if (missing_target(&results))
> -				die("%s not found: did you run git update-server-info on the server?", refs_url);
> -			else
> -				die("%s download error - %s", refs_url, curl_errorstr);
> -		}
> -	} else {
> -		strbuf_release(&buffer);
> -		die("Unable to start HTTP request");
> -	}
> +	if (http_get_strbuf(refs_url, &buffer, HTTP_NO_CACHE) != HTTP_OK)
> +		goto cleanup;

Why not die() as the original code?

Ciao,
Dscho

^ permalink raw reply

* Re: [WIP Patch 07/12] Use the new http API in http-push.c:fetch_indices()
From: Johannes Schindelin @ 2009-01-18 15:14 UTC (permalink / raw)
  To: Mike Hommey; +Cc: git, gitster
In-Reply-To: <1232265877-3649-8-git-send-email-mh@glandium.org>

Hi,

On Sun, 18 Jan 2009, Mike Hommey wrote:

> @@ -1018,28 +1016,10 @@ static int fetch_indices(void)
>  	url = xmalloc(strlen(remote->url) + 20);
>  	sprintf(url, "%sobjects/info/packs", remote->url);
>  
> -	slot = get_active_slot();
> -	slot->results = &results;
> -	curl_easy_setopt(slot->curl, CURLOPT_FILE, &buffer);
> -	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);
> -	curl_easy_setopt(slot->curl, CURLOPT_URL, url);
> -	curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, NULL);
> -	if (start_active_slot(slot)) {
> -		run_active_slot(slot);
> -		if (results.curl_result != CURLE_OK) {
> -			strbuf_release(&buffer);
> -			free(url);
> -			if (results.http_code == 404)
> -				return 0;
> -			else
> -				return error("%s", curl_errorstr);
> -		}
> -	} else {
> -		strbuf_release(&buffer);
> -		free(url);
> -		return error("Unable to start request");
> +	if (http_get_strbuf(url, &buffer, 0) != HTTP_OK) {
> +		ret = -1;
> +		goto cleanup;
>  	}

Let's make that

		ret = error("%s", curl_errorstr);

okay?

Ciao,
Dscho

^ permalink raw reply

* [PATCH v10 0/5] git checkout: optimise away lots of lstat() calls
From: Kjetil Barvik @ 2009-01-18 15:14 UTC (permalink / raw)
  To: git; +Cc: Kjetil Barvik

Changes since version 9

--- patch 1/5 ---
bug fix (missed optimisation):
If we have track_flags = FL_SYMLINK|FL_DIR, and:

     cache  =>   A/B/C/D/S   (a symlink)
     name   =>   A/B/C       (a directory)

The cache would return with a (cached) FL_DIR result as expected,
_but_ it would set the cache to "A/B/C", so we lose some information.

--- patch 4/5 ---
bug fix: if we get a match from the cache for the name to invalidate,
we reset the cache if we are not allowed to track directories (inside
the invalidate_lstat_cache() function).


Kjetil Barvik (5):
  lstat_cache(): more cache effective symlink/directory detection
  lstat_cache(): introduce has_symlink_or_noent_leading_path() function
  lstat_cache(): introduce has_dirs_only_path() function
  lstat_cache(): introduce invalidate_lstat_cache() function
  lstat_cache(): introduce clear_lstat_cache() function

 cache.h        |    4 +
 entry.c        |   34 +++-----
 symlinks.c     |  263 ++++++++++++++++++++++++++++++++++++++++++++++---------
 unpack-trees.c |    4 +-
 4 files changed, 238 insertions(+), 67 deletions(-)

^ permalink raw reply

* [PATCH v10 1/5] lstat_cache(): more cache effective symlink/directory detection
From: Kjetil Barvik @ 2009-01-18 15:14 UTC (permalink / raw)
  To: git; +Cc: Kjetil Barvik
In-Reply-To: <1232291694-18083-1-git-send-email-barvik@broadpark.no>

Make the cache functionality more effective.  Previously when A/B/C/D
was in the cache and A/B/C/E/file.c was called for, there was no match
at all from the cache.  Now we use the fact that the paths "A", "A/B"
and "A/B/C" are already tested, and we only need to do an lstat() call
on "A/B/C/E".

We only cache/store the last path regardless of its type.  Since the
cache functionality is always used with alphabetically sorted names
(at least it seems so for me), there is no need to store both the last
symlink-leading path and the last real-directory path.  Note that if
the cache is not called with (mostly) alphabetically sorted names,
neither the old, nor this new one, would be very effective.

Previously, when symlink A/B/C/S was cached/stored in the symlink-
leading path, and A/B/C/file.c was called for, it was not easy to use
the fact that we already knew that the paths "A", "A/B" and "A/B/C"
are real directories.

Avoid copying the first path components of the name 2 zillion times
when we test new path components.  Since we always cache/store the
last path, we can copy each component as we test those directly into
the cache.  Previously we ended up doing a memcpy() for the full
path/name right before each lstat() call, and when updating the cache
for each time we have tested a new path component.

We also use less memory, that is, PATH_MAX bytes less memory on the
stack and PATH_MAX bytes less memory on the heap.

Thanks to Junio C Hamano, Linus Torvalds and Rene Scharfe for valuable
comments to this patch!

Signed-off-by: Kjetil Barvik <barvik@broadpark.no>
---
 symlinks.c |  165 +++++++++++++++++++++++++++++++++++++++++++++---------------
 1 files changed, 125 insertions(+), 40 deletions(-)

diff --git a/symlinks.c b/symlinks.c
index 5a5e781..49fb4d8 100644
--- a/symlinks.c
+++ b/symlinks.c
@@ -1,64 +1,149 @@
 #include "cache.h"
 
-struct pathname {
-	int len;
+static struct cache_def {
 	char path[PATH_MAX];
-};
+	int len;
+	int flags;
+} cache;
 
-/* Return matching pathname prefix length, or zero if not matching */
-static inline int match_pathname(int len, const char *name, struct pathname *match)
+/*
+ * Returns the length (on a path component basis) of the longest
+ * common prefix match of 'name' and the cached path string.
+ */
+static inline int longest_match_lstat_cache(int len, const char *name)
 {
-	int match_len = match->len;
-	return (len > match_len &&
-		name[match_len] == '/' &&
-		!memcmp(name, match->path, match_len)) ? match_len : 0;
+	int max_len, match_len = 0, i = 0;
+
+	max_len = len < cache.len ? len : cache.len;
+	while (i < max_len && name[i] == cache.path[i]) {
+		if (name[i] == '/')
+			match_len = i;
+		i++;
+	}
+	/* Is the cached path string a substring of 'name'? */
+	if (i == cache.len && cache.len < len && name[cache.len] == '/')
+		match_len = cache.len;
+	/* Is 'name' a substring of the cached path string? */
+	else if ((i == len && len < cache.len && cache.path[len] == '/') ||
+		 (i == len && len == cache.len))
+		match_len = len;
+	return match_len;
 }
 
-static inline void set_pathname(int len, const char *name, struct pathname *match)
+static inline void reset_lstat_cache(void)
 {
-	if (len < PATH_MAX) {
-		match->len = len;
-		memcpy(match->path, name, len);
-		match->path[len] = 0;
-	}
+	cache.path[0] = '\0';
+	cache.len = 0;
+	cache.flags = 0;
 }
 
-int has_symlink_leading_path(int len, const char *name)
+#define FL_DIR      (1 << 0)
+#define FL_SYMLINK  (1 << 1)
+#define FL_LSTATERR (1 << 2)
+#define FL_ERR      (1 << 3)
+
+/*
+ * Check if name 'name' of length 'len' has a symlink leading
+ * component, or if the directory exists and is real.
+ *
+ * To speed up the check, some information is allowed to be cached.
+ * This can be indicated by the 'track_flags' argument.
+ */
+static int lstat_cache(int len, const char *name,
+		       int track_flags)
 {
-	static struct pathname link, nonlink;
-	char path[PATH_MAX];
+	int match_len, last_slash, last_slash_dir;
+	int match_flags, ret_flags, save_flags, max_len;
 	struct stat st;
-	char *sp;
-	int known_dir;
 
 	/*
-	 * See if the last known symlink cache matches.
+	 * Check to see if we have a match from the cache for the
+	 * symlink path type.
 	 */
-	if (match_pathname(len, name, &link))
-		return 1;
-
+	match_len = last_slash = longest_match_lstat_cache(len, name);
+	match_flags = cache.flags & track_flags & FL_SYMLINK;
+	if (match_flags && match_len == cache.len)
+		return match_flags;
 	/*
-	 * Get rid of the last known directory part
+	 * If we now have match_len > 0, we would know that the
+	 * matched part will always be a directory.
+	 *
+	 * Also, if we are tracking directories and 'name' is a
+	 * substring of the cache on a path component basis, we can
+	 * return immediately.
 	 */
-	known_dir = match_pathname(len, name, &nonlink);
+	match_flags = track_flags & FL_DIR;
+	if (match_flags && len == match_len)
+		return match_flags;
 
-	while ((sp = strchr(name + known_dir + 1, '/')) != NULL) {
-		int thislen = sp - name ;
-		memcpy(path, name, thislen);
-		path[thislen] = 0;
+	/*
+	 * Okay, no match from the cache so far, so now we have to
+	 * check the rest of the path components.
+	 */
+	ret_flags = FL_DIR;
+	last_slash_dir = last_slash;
+	max_len = len < PATH_MAX ? len : PATH_MAX;
+	while (match_len < max_len) {
+		do {
+			cache.path[match_len] = name[match_len];
+			match_len++;
+		} while (match_len < max_len && name[match_len] != '/');
+		if (match_len >= max_len)
+			break;
+		last_slash = match_len;
+		cache.path[last_slash] = '\0';
 
-		if (lstat(path, &st))
-			return 0;
-		if (S_ISDIR(st.st_mode)) {
-			set_pathname(thislen, path, &nonlink);
-			known_dir = thislen;
+		if (lstat(cache.path, &st)) {
+			ret_flags = FL_LSTATERR;
+		} else if (S_ISDIR(st.st_mode)) {
+			last_slash_dir = last_slash;
 			continue;
-		}
-		if (S_ISLNK(st.st_mode)) {
-			set_pathname(thislen, path, &link);
-			return 1;
+		} else if (S_ISLNK(st.st_mode)) {
+			ret_flags = FL_SYMLINK;
+		} else {
+			ret_flags = FL_ERR;
 		}
 		break;
 	}
-	return 0;
+
+	/*
+	 * At the end update the cache.  Note that max 2 different
+	 * path types, FL_SYMLINK and FL_DIR, can be cached for the
+	 * moment!
+	 */
+	save_flags = ret_flags & track_flags & FL_SYMLINK;
+	if (save_flags && last_slash > 0 && last_slash < PATH_MAX) {
+		cache.path[last_slash] = '\0';
+		cache.len = last_slash;
+		cache.flags = save_flags;
+	} else if (track_flags & FL_DIR &&
+		   last_slash_dir > 0 && last_slash_dir < PATH_MAX) {
+		/*
+		 * We have a separate test for the directory case,
+		 * since it could be that we have found a symlink and
+		 * the track_flags says that we cannot cache this
+		 * fact, so the cache would then have been left empty
+		 * in this case.
+		 *
+		 * But if we are allowed to track real directories, we
+		 * can still cache the path components before the last
+		 * one (the found symlink component).
+		 */
+		cache.path[last_slash_dir] = '\0';
+		cache.len = last_slash_dir;
+		cache.flags = FL_DIR;
+	} else {
+		reset_lstat_cache();
+	}
+	return ret_flags;
+}
+
+/*
+ * Return non-zero if path 'name' has a leading symlink component
+ */
+int has_symlink_leading_path(int len, const char *name)
+{
+	return lstat_cache(len, name,
+			   FL_SYMLINK|FL_DIR) &
+		FL_SYMLINK;
 }
-- 
1.6.1.83.gd727f

^ permalink raw reply related

* [PATCH v10 2/5] lstat_cache(): introduce has_symlink_or_noent_leading_path() function
From: Kjetil Barvik @ 2009-01-18 15:14 UTC (permalink / raw)
  To: git; +Cc: Kjetil Barvik
In-Reply-To: <1232291694-18083-1-git-send-email-barvik@broadpark.no>

In some cases, especially inside the unpack-trees.c file, and inside
the verify_absent() function, we can avoid some unnecessary calls to
lstat(), if the lstat_cache() function can also be told to keep track
of non-existing directories.

So we update the lstat_cache() function to handle this new fact,
introduce a new wrapper function, and the result is that we save lots
of lstat() calls for a removed directory which previously contained
lots of files, when we call this new wrapper of lstat_cache() instead
of the old one.

We do similar changes inside the unlink_entry() function, since if we
can already say that the leading directory component of a pathname
does not exist, it is not necessary to try to remove a pathname below
it!

Thanks to Junio C Hamano, Linus Torvalds and Rene Scharfe for valuable
comments to this patch!

Signed-off-by: Kjetil Barvik <barvik@broadpark.no>
---
 cache.h        |    1 +
 symlinks.c     |   94 +++++++++++++++++++++++++++++++++++--------------------
 unpack-trees.c |    4 +-
 3 files changed, 63 insertions(+), 36 deletions(-)

diff --git a/cache.h b/cache.h
index 8e1af26..518e4c7 100644
--- a/cache.h
+++ b/cache.h
@@ -717,6 +717,7 @@ struct checkout {
 
 extern int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *topath);
 extern int has_symlink_leading_path(int len, const char *name);
+extern int has_symlink_or_noent_leading_path(int len, const char *name);
 
 extern struct alternate_object_database {
 	struct alternate_object_database *next;
diff --git a/symlinks.c b/symlinks.c
index 49fb4d8..c69556a 100644
--- a/symlinks.c
+++ b/symlinks.c
@@ -4,6 +4,7 @@ static struct cache_def {
 	char path[PATH_MAX];
 	int len;
 	int flags;
+	int track_flags;
 } cache;
 
 /*
@@ -30,21 +31,23 @@ static inline int longest_match_lstat_cache(int len, const char *name)
 	return match_len;
 }
 
-static inline void reset_lstat_cache(void)
+static inline void reset_lstat_cache(int track_flags)
 {
 	cache.path[0] = '\0';
 	cache.len = 0;
 	cache.flags = 0;
+	cache.track_flags = track_flags;
 }
 
 #define FL_DIR      (1 << 0)
-#define FL_SYMLINK  (1 << 1)
-#define FL_LSTATERR (1 << 2)
-#define FL_ERR      (1 << 3)
+#define FL_NOENT    (1 << 1)
+#define FL_SYMLINK  (1 << 2)
+#define FL_LSTATERR (1 << 3)
+#define FL_ERR      (1 << 4)
 
 /*
  * Check if name 'name' of length 'len' has a symlink leading
- * component, or if the directory exists and is real.
+ * component, or if the directory exists and is real, or not.
  *
  * To speed up the check, some information is allowed to be cached.
  * This can be indicated by the 'track_flags' argument.
@@ -56,25 +59,35 @@ static int lstat_cache(int len, const char *name,
 	int match_flags, ret_flags, save_flags, max_len;
 	struct stat st;
 
-	/*
-	 * Check to see if we have a match from the cache for the
-	 * symlink path type.
-	 */
-	match_len = last_slash = longest_match_lstat_cache(len, name);
-	match_flags = cache.flags & track_flags & FL_SYMLINK;
-	if (match_flags && match_len == cache.len)
-		return match_flags;
-	/*
-	 * If we now have match_len > 0, we would know that the
-	 * matched part will always be a directory.
-	 *
-	 * Also, if we are tracking directories and 'name' is a
-	 * substring of the cache on a path component basis, we can
-	 * return immediately.
-	 */
-	match_flags = track_flags & FL_DIR;
-	if (match_flags && len == match_len)
-		return match_flags;
+	if (cache.track_flags != track_flags) {
+		/*
+		 * As a safeguard we clear the cache if the value of
+		 * track_flags does not match with the last supplied
+		 * value.
+		 */
+		reset_lstat_cache(track_flags);
+		match_len = last_slash = 0;
+	} else {
+		/*
+		 * Check to see if we have a match from the cache for
+		 * the 2 "excluding" path types.
+		 */
+		match_len = last_slash = longest_match_lstat_cache(len, name);
+		match_flags = cache.flags & track_flags & (FL_NOENT|FL_SYMLINK);
+		if (match_flags && match_len == cache.len)
+			return match_flags;
+		/*
+		 * If we now have match_len > 0, we would know that
+		 * the matched part will always be a directory.
+		 *
+		 * Also, if we are tracking directories and 'name' is
+		 * a substring of the cache on a path component basis,
+		 * we can return immediately.
+		 */
+		match_flags = track_flags & FL_DIR;
+		if (match_flags && len == match_len)
+			return match_flags;
+	}
 
 	/*
 	 * Okay, no match from the cache so far, so now we have to
@@ -95,6 +108,8 @@ static int lstat_cache(int len, const char *name,
 
 		if (lstat(cache.path, &st)) {
 			ret_flags = FL_LSTATERR;
+			if (errno == ENOENT)
+				ret_flags |= FL_NOENT;
 		} else if (S_ISDIR(st.st_mode)) {
 			last_slash_dir = last_slash;
 			continue;
@@ -107,11 +122,11 @@ static int lstat_cache(int len, const char *name,
 	}
 
 	/*
-	 * At the end update the cache.  Note that max 2 different
-	 * path types, FL_SYMLINK and FL_DIR, can be cached for the
-	 * moment!
+	 * At the end update the cache.  Note that max 3 different
+	 * path types, FL_NOENT, FL_SYMLINK and FL_DIR, can be cached
+	 * for the moment!
 	 */
-	save_flags = ret_flags & track_flags & FL_SYMLINK;
+	save_flags = ret_flags & track_flags & (FL_NOENT|FL_SYMLINK);
 	if (save_flags && last_slash > 0 && last_slash < PATH_MAX) {
 		cache.path[last_slash] = '\0';
 		cache.len = last_slash;
@@ -120,20 +135,20 @@ static int lstat_cache(int len, const char *name,
 		   last_slash_dir > 0 && last_slash_dir < PATH_MAX) {
 		/*
 		 * We have a separate test for the directory case,
-		 * since it could be that we have found a symlink and
-		 * the track_flags says that we cannot cache this
-		 * fact, so the cache would then have been left empty
-		 * in this case.
+		 * since it could be that we have found a symlink or a
+		 * non-existing directory and the track_flags says
+		 * that we cannot cache this fact, so the cache would
+		 * then have been left empty in this case.
 		 *
 		 * But if we are allowed to track real directories, we
 		 * can still cache the path components before the last
-		 * one (the found symlink component).
+		 * one (the found symlink or non-existing component).
 		 */
 		cache.path[last_slash_dir] = '\0';
 		cache.len = last_slash_dir;
 		cache.flags = FL_DIR;
 	} else {
-		reset_lstat_cache();
+		reset_lstat_cache(track_flags);
 	}
 	return ret_flags;
 }
@@ -147,3 +162,14 @@ int has_symlink_leading_path(int len, const char *name)
 			   FL_SYMLINK|FL_DIR) &
 		FL_SYMLINK;
 }
+
+/*
+ * Return non-zero if path 'name' has a leading symlink component or
+ * if some leading path component does not exists.
+ */
+int has_symlink_or_noent_leading_path(int len, const char *name)
+{
+	return lstat_cache(len, name,
+			   FL_SYMLINK|FL_NOENT|FL_DIR) &
+		(FL_SYMLINK|FL_NOENT);
+}
diff --git a/unpack-trees.c b/unpack-trees.c
index 15c9ef5..16bc2ca 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -61,7 +61,7 @@ static void unlink_entry(struct cache_entry *ce)
 	char *cp, *prev;
 	char *name = ce->name;
 
-	if (has_symlink_leading_path(ce_namelen(ce), ce->name))
+	if (has_symlink_or_noent_leading_path(ce_namelen(ce), ce->name))
 		return;
 	if (unlink(name))
 		return;
@@ -580,7 +580,7 @@ static int verify_absent(struct cache_entry *ce, const char *action,
 	if (o->index_only || o->reset || !o->update)
 		return 0;
 
-	if (has_symlink_leading_path(ce_namelen(ce), ce->name))
+	if (has_symlink_or_noent_leading_path(ce_namelen(ce), ce->name))
 		return 0;
 
 	if (!lstat(ce->name, &st)) {
-- 
1.6.1.83.gd727f

^ permalink raw reply related

* [PATCH v10 3/5] lstat_cache(): introduce has_dirs_only_path() function
From: Kjetil Barvik @ 2009-01-18 15:14 UTC (permalink / raw)
  To: git; +Cc: Kjetil Barvik
In-Reply-To: <1232291694-18083-1-git-send-email-barvik@broadpark.no>

The create_directories() function in entry.c currently calls stat()
or lstat() for each path component of the pathname 'path' each and every
time.  For the 'git checkout' command, this function is called on each
file for which we must do an update (ce->ce_flags & CE_UPDATE), so we get
lots and lots of calls.

To fix this, we make a new wrapper to the lstat_cache() function, and
call the wrapper function instead of the calls to the stat() or the
lstat() functions.  Since the paths given to the create_directories()
function, is sorted alphabetically, the new wrapper would be very
cache effective in this situation.

To support it we must update the lstat_cache() function to be able to
say that "please test the complete length of 'name'", and also to give
it the length of a prefix, where the cache should use the stat()
function instead of the lstat() function to test each path component.

Thanks to Junio C Hamano, Linus Torvalds and Rene Scharfe for valuable
comments to this patch!

Signed-off-by: Kjetil Barvik <barvik@broadpark.no>
---
 cache.h    |    1 +
 entry.c    |   34 +++++++++++--------------------
 symlinks.c |   64 ++++++++++++++++++++++++++++++++++++++++++++---------------
 3 files changed, 60 insertions(+), 39 deletions(-)

diff --git a/cache.h b/cache.h
index 518e4c7..110b9f9 100644
--- a/cache.h
+++ b/cache.h
@@ -718,6 +718,7 @@ struct checkout {
 extern int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *topath);
 extern int has_symlink_leading_path(int len, const char *name);
 extern int has_symlink_or_noent_leading_path(int len, const char *name);
+extern int has_dirs_only_path(int len, const char *name, int prefix_len);
 
 extern struct alternate_object_database {
 	struct alternate_object_database *next;
diff --git a/entry.c b/entry.c
index aa2ee46..01a683e 100644
--- a/entry.c
+++ b/entry.c
@@ -8,35 +8,25 @@ static void create_directories(const char *path, const struct checkout *state)
 	const char *slash = path;
 
 	while ((slash = strchr(slash+1, '/')) != NULL) {
-		struct stat st;
-		int stat_status;
-
 		len = slash - path;
 		memcpy(buf, path, len);
 		buf[len] = 0;
 
-		if (len <= state->base_dir_len)
-			/*
-			 * checkout-index --prefix=<dir>; <dir> is
-			 * allowed to be a symlink to an existing
-			 * directory.
-			 */
-			stat_status = stat(buf, &st);
-		else
-			/*
-			 * if there currently is a symlink, we would
-			 * want to replace it with a real directory.
-			 */
-			stat_status = lstat(buf, &st);
-
-		if (!stat_status && S_ISDIR(st.st_mode))
+		/*
+		 * For 'checkout-index --prefix=<dir>', <dir> is
+		 * allowed to be a symlink to an existing directory,
+		 * and we set 'state->base_dir_len' below, such that
+		 * we test the path components of the prefix with the
+		 * stat() function instead of the lstat() function.
+		 */
+		if (has_dirs_only_path(len, buf, state->base_dir_len))
 			continue; /* ok, it is already a directory. */
 
 		/*
-		 * We know stat_status == 0 means something exists
-		 * there and this mkdir would fail, but that is an
-		 * error codepath; we do not care, as we unlink and
-		 * mkdir again in such a case.
+		 * If this mkdir() would fail, it could be that there
+		 * is already a symlink or something else exists
+		 * there, therefore we then try to unlink it and try
+		 * one more time to create the directory.
 		 */
 		if (mkdir(buf, 0777)) {
 			if (errno == EEXIST && state->force &&
diff --git a/symlinks.c b/symlinks.c
index c69556a..918e24a 100644
--- a/symlinks.c
+++ b/symlinks.c
@@ -1,10 +1,11 @@
 #include "cache.h"
 
 static struct cache_def {
-	char path[PATH_MAX];
+	char path[PATH_MAX + 1];
 	int len;
 	int flags;
 	int track_flags;
+	int prefix_len_stat_func;
 } cache;
 
 /*
@@ -31,12 +32,13 @@ static inline int longest_match_lstat_cache(int len, const char *name)
 	return match_len;
 }
 
-static inline void reset_lstat_cache(int track_flags)
+static inline void reset_lstat_cache(int track_flags, int prefix_len_stat_func)
 {
 	cache.path[0] = '\0';
 	cache.len = 0;
 	cache.flags = 0;
 	cache.track_flags = track_flags;
+	cache.prefix_len_stat_func = prefix_len_stat_func;
 }
 
 #define FL_DIR      (1 << 0)
@@ -44,28 +46,35 @@ static inline void reset_lstat_cache(int track_flags)
 #define FL_SYMLINK  (1 << 2)
 #define FL_LSTATERR (1 << 3)
 #define FL_ERR      (1 << 4)
+#define FL_FULLPATH (1 << 5)
 
 /*
  * Check if name 'name' of length 'len' has a symlink leading
  * component, or if the directory exists and is real, or not.
  *
  * To speed up the check, some information is allowed to be cached.
- * This can be indicated by the 'track_flags' argument.
+ * This can be indicated by the 'track_flags' argument, which also can
+ * be used to indicate that we should check the full path.
+ *
+ * The 'prefix_len_stat_func' parameter can be used to set the length
+ * of the prefix, where the cache should use the stat() function
+ * instead of the lstat() function to test each path component.
  */
 static int lstat_cache(int len, const char *name,
-		       int track_flags)
+		       int track_flags, int prefix_len_stat_func)
 {
 	int match_len, last_slash, last_slash_dir;
-	int match_flags, ret_flags, save_flags, max_len;
+	int match_flags, ret_flags, save_flags, max_len, ret;
 	struct stat st;
 
-	if (cache.track_flags != track_flags) {
+	if (cache.track_flags != track_flags ||
+	    cache.prefix_len_stat_func != prefix_len_stat_func) {
 		/*
-		 * As a safeguard we clear the cache if the value of
-		 * track_flags does not match with the last supplied
-		 * value.
+		 * As a safeguard we clear the cache if the values of
+		 * track_flags and/or prefix_len_stat_func does not
+		 * match with the last supplied values.
 		 */
-		reset_lstat_cache(track_flags);
+		reset_lstat_cache(track_flags, prefix_len_stat_func);
 		match_len = last_slash = 0;
 	} else {
 		/*
@@ -101,12 +110,17 @@ static int lstat_cache(int len, const char *name,
 			cache.path[match_len] = name[match_len];
 			match_len++;
 		} while (match_len < max_len && name[match_len] != '/');
-		if (match_len >= max_len)
+		if (match_len >= max_len && !(track_flags & FL_FULLPATH))
 			break;
 		last_slash = match_len;
 		cache.path[last_slash] = '\0';
 
-		if (lstat(cache.path, &st)) {
+		if (last_slash <= prefix_len_stat_func)
+			ret = stat(cache.path, &st);
+		else
+			ret = lstat(cache.path, &st);
+
+		if (ret) {
 			ret_flags = FL_LSTATERR;
 			if (errno == ENOENT)
 				ret_flags |= FL_NOENT;
@@ -127,12 +141,12 @@ static int lstat_cache(int len, const char *name,
 	 * for the moment!
 	 */
 	save_flags = ret_flags & track_flags & (FL_NOENT|FL_SYMLINK);
-	if (save_flags && last_slash > 0 && last_slash < PATH_MAX) {
+	if (save_flags && last_slash > 0 && last_slash <= PATH_MAX) {
 		cache.path[last_slash] = '\0';
 		cache.len = last_slash;
 		cache.flags = save_flags;
 	} else if (track_flags & FL_DIR &&
-		   last_slash_dir > 0 && last_slash_dir < PATH_MAX) {
+		   last_slash_dir > 0 && last_slash_dir <= PATH_MAX) {
 		/*
 		 * We have a separate test for the directory case,
 		 * since it could be that we have found a symlink or a
@@ -148,18 +162,20 @@ static int lstat_cache(int len, const char *name,
 		cache.len = last_slash_dir;
 		cache.flags = FL_DIR;
 	} else {
-		reset_lstat_cache(track_flags);
+		reset_lstat_cache(track_flags, prefix_len_stat_func);
 	}
 	return ret_flags;
 }
 
+#define USE_ONLY_LSTAT  0
+
 /*
  * Return non-zero if path 'name' has a leading symlink component
  */
 int has_symlink_leading_path(int len, const char *name)
 {
 	return lstat_cache(len, name,
-			   FL_SYMLINK|FL_DIR) &
+			   FL_SYMLINK|FL_DIR, USE_ONLY_LSTAT) &
 		FL_SYMLINK;
 }
 
@@ -170,6 +186,20 @@ int has_symlink_leading_path(int len, const char *name)
 int has_symlink_or_noent_leading_path(int len, const char *name)
 {
 	return lstat_cache(len, name,
-			   FL_SYMLINK|FL_NOENT|FL_DIR) &
+			   FL_SYMLINK|FL_NOENT|FL_DIR, USE_ONLY_LSTAT) &
 		(FL_SYMLINK|FL_NOENT);
 }
+
+/*
+ * Return non-zero if all path components of 'name' exists as a
+ * directory.  If prefix_len > 0, we will test with the stat()
+ * function instead of the lstat() function for a prefix length of
+ * 'prefix_len', thus we then allow for symlinks in the prefix part as
+ * long as those points to real existing directories.
+ */
+int has_dirs_only_path(int len, const char *name, int prefix_len)
+{
+	return lstat_cache(len, name,
+			   FL_DIR|FL_FULLPATH, prefix_len) &
+		FL_DIR;
+}
-- 
1.6.1.83.gd727f

^ permalink raw reply related

* [PATCH v10 5/5] lstat_cache(): introduce clear_lstat_cache() function
From: Kjetil Barvik @ 2009-01-18 15:14 UTC (permalink / raw)
  To: git; +Cc: Kjetil Barvik
In-Reply-To: <1232291694-18083-1-git-send-email-barvik@broadpark.no>

If you want to completely clear the contents of the lstat_cache(), then
call this new function.

Signed-off-by: Kjetil Barvik <barvik@broadpark.no>
---
 cache.h    |    1 +
 symlinks.c |    8 ++++++++
 2 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/cache.h b/cache.h
index efcceec..8e22c27 100644
--- a/cache.h
+++ b/cache.h
@@ -720,6 +720,7 @@ extern int has_symlink_leading_path(int len, const char *name);
 extern int has_symlink_or_noent_leading_path(int len, const char *name);
 extern int has_dirs_only_path(int len, const char *name, int prefix_len);
 extern void invalidate_lstat_cache(int len, const char *name);
+extern void clear_lstat_cache(void);
 
 extern struct alternate_object_database {
 	struct alternate_object_database *next;
diff --git a/symlinks.c b/symlinks.c
index dbdfec4..83cecd7 100644
--- a/symlinks.c
+++ b/symlinks.c
@@ -195,6 +195,14 @@ void invalidate_lstat_cache(int len, const char *name)
 	}
 }
 
+/*
+ * Completely clear the contents of the cache
+ */
+void clear_lstat_cache(void)
+{
+	reset_lstat_cache(0, 0);
+}
+
 #define USE_ONLY_LSTAT  0
 
 /*
-- 
1.6.1.83.gd727f

^ permalink raw reply related

* [PATCH v10 4/5] lstat_cache(): introduce invalidate_lstat_cache() function
From: Kjetil Barvik @ 2009-01-18 15:14 UTC (permalink / raw)
  To: git; +Cc: Kjetil Barvik
In-Reply-To: <1232291694-18083-1-git-send-email-barvik@broadpark.no>

In some cases it could maybe be necessary to say to the cache that
"Hey, I deleted/changed the type of this pathname and if you currently
have it inside your cache, you should deleted it".

This patch introduce a function which support this.

Signed-off-by: Kjetil Barvik <barvik@broadpark.no>
---
 cache.h    |    1 +
 symlinks.c |   44 ++++++++++++++++++++++++++++++++++++--------
 2 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/cache.h b/cache.h
index 110b9f9..efcceec 100644
--- a/cache.h
+++ b/cache.h
@@ -719,6 +719,7 @@ extern int checkout_entry(struct cache_entry *ce, const struct checkout *state,
 extern int has_symlink_leading_path(int len, const char *name);
 extern int has_symlink_or_noent_leading_path(int len, const char *name);
 extern int has_dirs_only_path(int len, const char *name, int prefix_len);
+extern void invalidate_lstat_cache(int len, const char *name);
 
 extern struct alternate_object_database {
 	struct alternate_object_database *next;
diff --git a/symlinks.c b/symlinks.c
index 918e24a..dbdfec4 100644
--- a/symlinks.c
+++ b/symlinks.c
@@ -12,23 +12,30 @@ static struct cache_def {
  * Returns the length (on a path component basis) of the longest
  * common prefix match of 'name' and the cached path string.
  */
-static inline int longest_match_lstat_cache(int len, const char *name)
+static inline int longest_match_lstat_cache(int len, const char *name,
+					    int *previous_slash)
 {
-	int max_len, match_len = 0, i = 0;
+	int max_len, match_len = 0, match_len_prev = 0, i = 0;
 
 	max_len = len < cache.len ? len : cache.len;
 	while (i < max_len && name[i] == cache.path[i]) {
-		if (name[i] == '/')
+		if (name[i] == '/') {
+			match_len_prev = match_len;
 			match_len = i;
+		}
 		i++;
 	}
 	/* Is the cached path string a substring of 'name'? */
-	if (i == cache.len && cache.len < len && name[cache.len] == '/')
+	if (i == cache.len && cache.len < len && name[cache.len] == '/') {
+		match_len_prev = match_len;
 		match_len = cache.len;
 	/* Is 'name' a substring of the cached path string? */
-	else if ((i == len && len < cache.len && cache.path[len] == '/') ||
-		 (i == len && len == cache.len))
+	} else if ((i == len && len < cache.len && cache.path[len] == '/') ||
+		   (i == len && len == cache.len)) {
+		match_len_prev = match_len;
 		match_len = len;
+	}
+	*previous_slash = match_len_prev;
 	return match_len;
 }
 
@@ -63,7 +70,7 @@ static inline void reset_lstat_cache(int track_flags, int prefix_len_stat_func)
 static int lstat_cache(int len, const char *name,
 		       int track_flags, int prefix_len_stat_func)
 {
-	int match_len, last_slash, last_slash_dir;
+	int match_len, last_slash, last_slash_dir, previous_slash;
 	int match_flags, ret_flags, save_flags, max_len, ret;
 	struct stat st;
 
@@ -81,7 +88,8 @@ static int lstat_cache(int len, const char *name,
 		 * Check to see if we have a match from the cache for
 		 * the 2 "excluding" path types.
 		 */
-		match_len = last_slash = longest_match_lstat_cache(len, name);
+		match_len = last_slash =
+			longest_match_lstat_cache(len, name, &previous_slash);
 		match_flags = cache.flags & track_flags & (FL_NOENT|FL_SYMLINK);
 		if (match_flags && match_len == cache.len)
 			return match_flags;
@@ -167,6 +175,26 @@ static int lstat_cache(int len, const char *name,
 	return ret_flags;
 }
 
+/*
+ * Invalidate the given 'name' from the cache, if 'name' matches
+ * completely with the cache.
+ */
+void invalidate_lstat_cache(int len, const char *name)
+{
+	int match_len, previous_slash;
+
+	match_len = longest_match_lstat_cache(len, name, &previous_slash);
+	if (len == match_len) {
+		if (cache.track_flags & FL_DIR && previous_slash > 0) {
+			cache.path[previous_slash] = '\0';
+			cache.len = previous_slash;
+			cache.flags = FL_DIR;
+		} else
+			reset_lstat_cache(cache.track_flags,
+					  cache.prefix_len_stat_func);
+	}
+}
+
 #define USE_ONLY_LSTAT  0
 
 /*
-- 
1.6.1.83.gd727f

^ permalink raw reply related

* Re: [WIP Patch 08/12] Use the new http API in update_remote_info_refs()
From: Johannes Schindelin @ 2009-01-18 15:18 UTC (permalink / raw)
  To: Mike Hommey; +Cc: git, gitster
In-Reply-To: <1232265877-3649-9-git-send-email-mh@glandium.org>

Hi,

On Sun, 18 Jan 2009, Mike Hommey wrote:

> 
> Signed-off-by: Mike Hommey <mh@glandium.org>
> ---
>  http-push.c |   29 ++++++++++-------------------
>  1 files changed, 10 insertions(+), 19 deletions(-)
> 
> diff --git a/http-push.c b/http-push.c
> index e0b4f5a..7627860 100644
> --- a/http-push.c
> +++ b/http-push.c
> @@ -1960,29 +1960,20 @@ static void update_remote_info_refs(struct remote_lock *lock)
>  static int remote_exists(const char *path)
>  {

Heh, I see where your commit subject comes from, but it should rather 
mention the function "remote_exists()"...

>  	char *url = xmalloc(strlen(remote->url) + strlen(path) + 1);
> -	struct active_request_slot *slot;
> -	struct slot_results results;
> -	int ret = -1;
> +	int ret;
>  
>  	sprintf(url, "%s%s", remote->url, path);
>  
> -	slot = get_active_slot();
> -	slot->results = &results;
> -	curl_easy_setopt(slot->curl, CURLOPT_URL, url);
> -	curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1);
> -
> -	if (start_active_slot(slot)) {
> -		run_active_slot(slot);
> -		if (results.http_code == 404)
> -			ret = 0;
> -		else if (results.curl_result == CURLE_OK)
> -			ret = 1;
> -		else
> -			fprintf(stderr, "HEAD HTTP error %ld\n", results.http_code);
> -	} else {
> -		fprintf(stderr, "Unable to start HEAD request\n");
> +	switch (http_get_strbuf(url, NULL, 0)) {
> +	case HTTP_OK:
> +		ret = 1;
> +		break;
> +	case HTTP_MISSING_TARGET:
> +		ret = 0;
> +		break;
> +	default:
> +		ret = -1;
>  	}

Does http_get_strbuf() already show the error?  Not as far as I can see, 
even if it would make sense, no?  At least you'll have to "return 
error(...)".

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH v4 0/7] customizable --color-words
From: Santi Béjar @ 2009-01-18 15:29 UTC (permalink / raw)
  To: Thomas Rast
  Cc: git, Junio C Hamano, Johannes Schindelin, Boyd Stephen Smith Jr.,
	Teemu Likonen
In-Reply-To: <adf1fd3d0901180705s260f0051wb4e3a978601618ec@mail.gmail.com>

2009/1/18 Santi Béjar <santi@agolina.net>:
> 2009/1/17 Thomas Rast <trast@student.ethz.ch>:
>> Johannes Schindelin wrote:
>>> Thomas, could you pick up the patches from my 'my-next' branch and
>>> maintain an "official" topic branch?
>>
>> I cherry-picked the three commits you had there, and rebuilt on top.
>> I pushed them to
>>
>>  git://repo.or.cz/git/trast.git tr/word-diff-p2
>>
>> again (js/word-diff-p1 again points directly at your half).
>
> I've tested tr/word-diff-p2 and I have not found any issues. I've even
> tested that nothing changed from the tradicional word diff to:
>
> git log -p --color-words="[^[:space:]]+"
>
> for the whole git history.
>

What I tested is that the new code produces the same result for this
two commands:

git log -p --color-words="[^[:space:]]+"
git log -p --color-words

The old code produced color codes before and after each word, while
the new only at the begining of the color and the end of the color. So
they cannot produce the same output but equivalent.

Santi

^ permalink raw reply

* Re: [PATCH 1/3] sha1_file: add function to insert alternate object db
From: Johannes Schindelin @ 2009-01-18 15:32 UTC (permalink / raw)
  To: Lars Hjemli; +Cc: Junio C Hamano, git
In-Reply-To: <1232275999-14852-2-git-send-email-hjemli@gmail.com>

Hi,

On Sun, 18 Jan 2009, Lars Hjemli wrote:

> diff --git a/cache.h b/cache.h
> index 8e1af26..daa2d4e 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -724,6 +724,7 @@ extern struct alternate_object_database {
>  	char base[FLEX_ARRAY]; /* more */
>  } *alt_odb_list;
>  extern void prepare_alt_odb(void);
> +extern int add_alt_odb(const char *path);
>  extern void add_to_alternates_file(const char *reference);
>  typedef int alt_odb_fn(struct alternate_object_database *, void *);
>  extern void foreach_alt_odb(alt_odb_fn, void*);
> diff --git a/sha1_file.c b/sha1_file.c
> index f08493f..19f9725 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -356,6 +356,11 @@ static void link_alt_odb_entries(const char *alt, const char *ep, int sep,
>  	}
>  }
>  
> +int add_alt_odb(const char *path)
> +{
> +	return link_alt_odb_entry(path, strlen(path), NULL, 0);

This function can return the error message "object directory %s does not 
exist; check .git/objects/info/alternates."  Maybe you want to change 
that, even if the user you are introducing might not hit that code path.

Ciao,
Dscho

^ permalink raw reply

* Re: easy way to make tracking branches?
From: Sitaram Chamarty @ 2009-01-18 15:37 UTC (permalink / raw)
  To: git
In-Reply-To: <20090118105530.GG11992@leksak.fem-net>

On 2009-01-18, Stephan Beyer <s-beyer@gmx.net> wrote:
> Now I want to make "foo" a tracking branch for "bar".
> I do:
>
> 	git config branch.foo.remote srv
> 	git config branch.foo.merge refs/heads/bar

I just do a "git pull srv" when the tracking is *not* setup,
and git reminds me what commands to use.

> And to get a comfortable git-push, I do:
>
> 	git config --add remote.srv.push foo:bar

This one you'll just have to remember, I guess :-)

> 	git checkout -b foo2 srv/bar
> 	git branch -d foo
> 	git branch -m foo
>
> which is suboptimal because deleting foo can remove some
> other settings for the branch, e.g. mergeoptions.

it also doesn't seem to set remote.srv.push, as far as I can
tell.

^ permalink raw reply

* Re: [PATCH/RFC] git-am: Make it easier to see which patch failed
From: Jeff King @ 2009-01-18 15:39 UTC (permalink / raw)
  To: Stephan Beyer; +Cc: Junio C Hamano, Jonas Flodén, git, Johannes Schindelin
In-Reply-To: <20090118094113.GE11992@leksak.fem-net>

On Sun, Jan 18, 2009 at 10:41:13AM +0100, Stephan Beyer wrote:

> > Looks sane except that I do not think you need printf nor the leading
> > blank line, i.e.
> > 
> > 	echo "Patch failed at $msgnum ($FIRSTLINE)"
> 
> Hmm, IIRC if $FIRSTLINE contains \n or something like that, it will
> interpret this as newline in some shell/echo implementations.
> 
> So printf "...%s..." "$FOO" is always sane for user input.

Yes, I'm surprised Junio doesn't remember the mass conversions we already had
to do (4b7cc26a and 293623ed). But looking at the date, I guess it _has_
been a year and a half. :)

-Peff

^ permalink raw reply

* Re: [PATCH 2/3] Teach read_tree_recursive() how to traverse into submodules
From: Johannes Schindelin @ 2009-01-18 15:48 UTC (permalink / raw)
  To: Lars Hjemli; +Cc: Junio C Hamano, git
In-Reply-To: <1232275999-14852-3-git-send-email-hjemli@gmail.com>

Hi,

On Sun, 18 Jan 2009, Lars Hjemli wrote:

> diff --git a/environment.c b/environment.c
> index e278bce..35cc557 100644
> --- a/environment.c
> +++ b/environment.c
> @@ -53,6 +53,8 @@ static char *work_tree;
>  static const char *git_dir;
>  static char *git_object_dir, *git_index_file, *git_refs_dir, *git_graft_file;
>  
> +static int traverse_gitlinks = 0;
> +
>  static void setup_git_env(void)
>  {
>  	git_dir = getenv(GIT_DIR_ENVIRONMENT);
> @@ -159,3 +161,13 @@ int set_git_dir(const char *path)
>  	setup_git_env();
>  	return 0;
>  }
> +
> +int get_traverse_gitlinks()
> +{
> +	return traverse_gitlinks;
> +}
> +
> +void set_traverse_gitlinks(int traverse)
> +{
> +	traverse_gitlinks = traverse;
> +}

If you have full accessors anyway, it is much easier and cleaner to make 
this a global variable to begin with.

However, environment.c is reserved for things that come from the config 
and can be overridden by the user.  That is certainly not the case for 
traverse_gitlinks.

But let's think about it again: should traverse_gitlinks be a global 
varible at all?  I think not.  It should be a per-call decision.

 > diff --git a/tree.c b/tree.c
> index 03e782a..87cf309 100644
> --- a/tree.c
> +++ b/tree.c
> @@ -5,6 +5,7 @@
>  #include "commit.h"
>  #include "tag.h"
>  #include "tree-walk.h"
> +#include "refs.h"
>  
>  const char *tree_type = "tree";
>  
> @@ -89,6 +90,61 @@ static int match_tree_entry(const char *base, int baselen, const char *path, uns
>  	return 0;
>  }
>  
> +/* Try to add the objectdb of a submodule */
> +int add_gitlink_odb(char *relpath)

This wants to be static.

> +{
> +	const char *odbpath;
> +	struct stat st;
> +
> +	odbpath = read_gitfile_gently(mkpath("%s/.git", relpath));
> +	if (!odbpath)
> +		odbpath = mkpath("%s/.git/objects", relpath);
> +
> +	if (stat(odbpath, &st))
> +		return 1;
> +
> +	return add_alt_odb(odbpath);
> +}
> +
> +/* Check if we should recurse into the specified submodule */
> +int traverse_gitlink(char *path, const unsigned char *commit_sha1,

This, too.

> +		     struct tree **subtree)
> +{
> +	unsigned char sha1[20];
> +	int linked_odb = 0;
> +	struct commit *commit;
> +	void *buffer;
> +	enum object_type type;
> +	unsigned long size;
> +
> +	hashcpy(sha1, commit_sha1);
> +	if (!add_gitlink_odb(path)) {
> +		linked_odb = 1;
> +		if (resolve_gitlink_ref(path, "HEAD", sha1))
> +			die("Unable to lookup HEAD in %s", path);
> +	}

Why would you want to continue if add_gitlink_odb() did not find a checked 
out submodule?

Seems you want to fall back to look in the superproject's object database.  
But I think that is wrong, as I have a superproject with many platform 
dependent submodules, only one of which is checked out, and for 
convenience, the submodules all live in the superproject's repository.

But I might misunderstand your code.

> +	commit = lookup_commit(sha1);
> +	if (!commit)
> +		die("traverse_gitlink(): internal error");

s/internal error/could not access commit '%s' of submodule '%s'",
			sha1_to_hex(sha1), path);/

> @@ -132,6 +188,30 @@ int read_tree_recursive(struct tree *tree,
>  				return -1;
>  			continue;
>  		}
> +		if (S_ISGITLINK(entry.mode) && get_traverse_gitlinks()) {

Like I said, traverse_gitlinks should be a flag to read_tree_recursive.  
So preferably, you should add a parameter 'flags' and make that option an 
enum.

> +			int retval;
> +			char *newbase;
> +			struct tree *subtree;
> +			unsigned int pathlen = tree_entry_len(entry.path, entry.sha1);

Nit: Long line.

> +
> +			newbase = xmalloc(baselen + 1 + pathlen);
> +			memcpy(newbase, base, baselen);
> +			memcpy(newbase + baselen, entry.path, pathlen);
> +			newbase[baselen + pathlen] = 0;

We have strbufs for that.

> +			if (!traverse_gitlink(newbase, entry.sha1, &subtree)) {
> +				free(newbase);
> +				continue;
> +			}
> +			newbase[baselen + pathlen] = '/';

... to avoid this off-by-one.

> +			retval = read_tree_recursive(subtree,
> +						     newbase,
> +						     baselen + pathlen + 1,
> +						     stage, match, fn, context);
> +			free(newbase);
> +			if (retval)
> +				return -1;
> +			continue;
> +		}
>  	}
>  	return 0;
>  }

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH 3/3] git-archive: add support for --submodules
From: Johannes Schindelin @ 2009-01-18 15:51 UTC (permalink / raw)
  To: Lars Hjemli; +Cc: Junio C Hamano, git
In-Reply-To: <1232275999-14852-4-git-send-email-hjemli@gmail.com>

Hi,

On Sun, 18 Jan 2009, Lars Hjemli wrote:

> @@ -320,6 +323,7 @@ static int parse_archive_args(int argc, const char **argv,
>  	args->base = base;
>  	args->baselen = strlen(base);
>  
> +	set_traverse_gitlinks(submodules);

As I said, this is a per-call thing.  So you need to add that option to 
the archiver_args struct and use it in write_archive_entries().

Ciao,
Dscho

^ 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