Re: [RFC PATCH v2 2/4] gitignore: read from index if .gitignore is assume-unchanged

2009/8/10 Johannes Schindelin [off-list ref]:

Hi,

On Mon, 10 Aug 2009, Nguyễn Thái Ngọc Duy wrote:

quoted

diff --git a/dir.c b/dir.c
index e05b850..e55344f 100644
--- a/dir.c
+++ b/dir.c

@@ -200,11 +200,36 @@ void add_exclude(const char *string, const char *base,

      which->excludes[which->nr++] = x;
 }

+static void *read_index_data(const char *path, size_t *size)

How about calling it "read_assume_unchanged_from_index()" instead?  I
suggest this because it does not read the index from the data if the path
is not marked assume unchanged...

Agree.

quoted

@@ -212,27 +237,31 @@ static int add_excludes_from_file_1(const char *fname,

[...]

      if (buf_p)
              *buf_p = buf;
-     buf[size++] = '\n';
      entry = buf;
-     for (i = 0; i < size; i++) {
-             if (buf[i] == '\n') {
+     for (i = 0; i <= size; i++) {
+             if (i == size || buf[i] == '\n') {
                      if (entry != buf + i && entry[0] != '#') {
                              buf[i - (i && buf[i-1] == '\r')] = 0;
                              add_exclude(entry, base, baselen, which);

Should this change not rather be a separate one?

You meant a separate patch? It is tied to this patch, because if bus
is read from read_index_data, it does not have extra space for '\n' at
the end.

quoted

@@ -241,17 +270,12 @@ static int add_excludes_from_file_1(const char *fname,

              }
      }
      return 0;
-
- err:
-     if (0 <= fd)
-             close(fd);
-     return -1;
 }

 void add_excludes_from_file(struct dir_struct *dir, const char *fname)
 {
      if (add_excludes_from_file_1(fname, "", 0, NULL,
-                                  &dir->exclude_list[EXC_FILE]) < 0)
+                                  &dir->exclude_list[EXC_FILE], 0) < 0)

Could you mention in the commit message why this function does not want to
check the index (I _guess_ it is because this code path only tries to read
.git/info/exclude, but it would be better to be sure).

To retain old behaviour. But I have to check its callers. Maybe we
want to check the index too.

quoted

@@ -85,6 +85,26 @@ test_expect_success \

        >output &&
      test_cmp expect output'

+test_expect_success 'setup sparse gitignore' '
+     git add .gitignore one/.gitignore one/two/.gitignore &&
+     git update-index --assume-unchanged .gitignore one/.gitignore one/two/.gitignore &&
+     rm .gitignore one/.gitignore one/two/.gitignore
+'

You're probably less sloppy than me; I'd have defined a variable like
this:

       ignores=".gitignore one/.gitignore one/two/.gitignore"

and used it for the three calls, just to make sure that I do not fsck
anything up there due to fat fingers.

I have slim ones :-) But "git add $ignores && git update-index
$ignores && rm $ignores" is easier to read.
-- 
Duy