git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Steffen Prohaska <prohaska@zib.de>
To: gitster@pobox.com, dmitry.kakurin@gmail.com
Cc: git@vger.kernel.org, Steffen Prohaska <prohaska@zib.de>
Subject: [PATCH 1/2] attr: fix attribute handling if .gitattributes is involved
Date: Sun, 12 Aug 2007 22:34:34 +0200	[thread overview]
Message-ID: <11869508753328-git-send-email-prohaska@zib.de> (raw)

If we checkout .gitattributes, we must not try to parse
.gitattributes. If we tried to read it, it may not be
present because checkout_entry unlinks files before checkout.
But we would record that no attributes are present for this
directory, which is wrong. And worse, we would never try again.

This fix skips read_attr_from_file if the path triggering
the read ends with .gitattributes. This is a bit more than we
need, but it helps to checkout all .gitattributes before any
other file, without starting the attr machinery.

This solves part of the problem of correct attributes during
checkout. For example

   rm .gitattributes other
   git-checkout-index -f .gitattributes other

will work as expected, while

   rm .gitattributes other
   git-checkout-index -f other .gitattributes

will not. The problem in the second case is that .gitattributes
is not yet present when it is needed.

The second case could be fixed by ordering files such that
all .gitattributes are checked out before any other file. But
this is perhaps too expensive and not really needed. If the user
deliberately chooses to checkout .gitattributes after another
file, he will not benefit from the attr machinery.

However, for checkout-index --all we should fix the problem,
which is done by the next patch, building on top of this one.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 attr.c |   58 ++++++++++++++++++++++++++++++++++++++++++----------------
 1 files changed, 42 insertions(+), 16 deletions(-)

This together with the patch that follows fixes a problem, which
is most likely to bite Windows users. I recognized it by setting
autocrlf globally to true and doing a fresh checkout of msysgit.
The checkout contained etc/termcap converted to CRLF, although
is was marked as '-crlf' in etc/.gitattributes.

If we believe autocrlf is a reasonable default for Windows users,
we really should use it ourselves, to find such problems. 

The fixed problem is not really critical but may be quite annoying,
and complex to understand.

    Steffen

diff --git a/attr.c b/attr.c
index a071254..e942f6c 100644
--- a/attr.c
+++ b/attr.c
@@ -383,6 +383,18 @@ static void bootstrap_attr_stack(void)
 	}
 }
 
+static int ends_with_gitattributes (const char* path)
+{
+	int attributes_len = strlen (GITATTRIBUTES_FILE);
+	int path_len = strlen (path);
+	if (path_len >= attributes_len
+	     && strcmp (path + path_len - attributes_len, GITATTRIBUTES_FILE) == 0)
+	{
+		return 1;
+	}
+	return 0;
+}
+
 static void prepare_attr_stack(const char *path, int dirlen)
 {
 	struct attr_stack *elem, *info;
@@ -430,23 +442,37 @@ static void prepare_attr_stack(const char *path, int dirlen)
 
 	/*
 	 * Read from parent directories and push them down
+	 *
+	 * But don't try to read if path ends with .gitattributes.
+	 * In this case we could fail and record no attributes for
+	 * a directory. We better wait and see if we need the
+	 * attributes later.
+	 *
+	 * We skip all .gitattributes, even in higher directories.
+	 * Thus, we can checkout all .gitattributes in any order
+	 * before the attr machinery starts to work. .gitattributes
+	 * should not be controlled by .gitattributes in the working
+	 * tree anyway.
+	 *
 	 */
-	while (1) {
-		char *cp;
-
-		len = strlen(attr_stack->origin);
-		if (dirlen <= len)
-			break;
-		memcpy(pathbuf, path, dirlen);
-		memcpy(pathbuf + dirlen, "/", 2);
-		cp = strchr(pathbuf + len + 1, '/');
-		strcpy(cp + 1, GITATTRIBUTES_FILE);
-		elem = read_attr_from_file(pathbuf, 0);
-		*cp = '\0';
-		elem->origin = strdup(pathbuf);
-		elem->prev = attr_stack;
-		attr_stack = elem;
-		debug_push(elem);
+	if (!ends_with_gitattributes (path)) {
+		while (1) {
+			char *cp;
+
+			len = strlen(attr_stack->origin);
+			if (dirlen <= len)
+				break;
+			memcpy(pathbuf, path, dirlen);
+			memcpy(pathbuf + dirlen, "/", 2);
+			cp = strchr(pathbuf + len + 1, '/');
+			strcpy(cp + 1, GITATTRIBUTES_FILE);
+			elem = read_attr_from_file(pathbuf, 0);
+			*cp = '\0';
+			elem->origin = strdup(pathbuf);
+			elem->prev = attr_stack;
+			attr_stack = elem;
+			debug_push(elem);
+		}
 	}
 
 	/*
-- 
1.5.3.rc4.96.g6ceb

             reply	other threads:[~2007-08-12 20:34 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-08-12 20:34 Steffen Prohaska [this message]
2007-08-12 20:34 ` [PATCH 2/2] checkout: fix attribute handling in checkout all Steffen Prohaska
2007-08-12 21:50   ` Junio C Hamano
2007-08-12 22:26     ` Steffen Prohaska
2007-08-13  6:14     ` Junio C Hamano
2007-08-13  6:32       ` Marius Storm-Olsen
2007-08-13  6:50         ` Steffen Prohaska
2007-08-13  7:15           ` Marius Storm-Olsen
2007-08-13  7:32             ` Steffen Prohaska
2007-08-13  8:39               ` Marius Storm-Olsen
2007-08-13  8:51                 ` Steffen Prohaska
2007-08-13 14:35                   ` Dmitry Kakurin
2007-08-14  8:40         ` [PATCH 1/2] attr.c: refactoring Junio C Hamano
2007-08-14  8:41         ` [PATCH 2/2] attr.c: read .gitattributes from index as well Junio C Hamano
2007-08-13  6:46       ` [PATCH 2/2] checkout: fix attribute handling in checkout all Steffen Prohaska
2007-08-13 16:14         ` Johannes Schindelin
2007-08-13  7:24       ` David Kastrup
2007-08-13 14:55         ` git-update-ref bug? (was: [PATCH 2/2] checkout: fix attribute handling in checkout all) David Kastrup
2007-08-13 20:12         ` [PATCH 2/2] checkout: fix attribute handling in checkout all Junio C Hamano
2007-08-13  1:51 ` [PATCH 1/2] attr: fix attribute handling if .gitattributes is involved Brian Downing

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=11869508753328-git-send-email-prohaska@zib.de \
    --to=prohaska@zib.de \
    --cc=dmitry.kakurin@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).