Re: [PATCH 4/5] archive: do not read .gitattributes in working directory

[Nguyen Thai Ngoc Duy <pclouds@gmail.com>]

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

Sorry I got distracted by Vista+x11 stuff and had not work on this.

2009/4/16 Junio C Hamano <gitster@pobox.com>:
> @@ -168,6 +171,22 @@ int write_archive_entries(struct archiver_args *args,
>        context.args = args;
>        context.write_entry = write_entry;
>
> +       /*
> +        * Setup index and instruct attr to read index only
> +        */
> +       if (!args->worktree_attributes) {
> +               memset(&opts, 0, sizeof(opts));
> +               opts.index_only = 1;
> +               opts.head_idx = -1;
> +               opts.src_index = &the_index;
> +               opts.dst_index = &the_index;
> +               opts.fn = oneway_merge;
> +               init_tree_desc(&t, args->tree->buffer, args->tree->size);
> +               if (unpack_trees(1, &t, &opts))
> +                       return -1;
> +               git_attr_set_direction(GIT_ATTR_INDEX, &the_index);
> +       }
> +
>        err =  read_tree_recursive(args->tree, args->base, args->baselen, 0,
>                        args->pathspec, write_archive_entry, &context);
>        if (err == READ_TREE_RECURSIVE)

Squash the attached patch on top of this patch (I have yet to install
mutt), we may avoid a few possible lstat()s during unpack_trees(),
also less memory allocation.

I was thinking about loading .gitattributes inside write_archive_entry
too, to avoid calling read_tree_recursive twice, but it requires
.gitattributes to be traversed first. Won't work if there are files
.abc, .def...

If read_tree_recusive() expose its tree to read_tree_fn_t, we can then
look ahead and load .gitattributes, but that requires changing
read_tree_fn_t interface. I'll see if it's feasible to make a
customized read_tree_recusive() just for archive.c

Oh and a feature request (not really because I don't use it myself):
support submodules in "git archive", anyone?

> @@ -36,6 +36,14 @@ int cmd_tar_tree(int argc, const char **argv, const char *prefix)
>                argv++;
>                argc--;
>        }
> +       if (2 <= argc && !strcmp(argv[1], "--fix-attributes")) {
> +               argv++;
> +               argc--;
> +       }

Does this part needed? Gotta fix tar_tree_usage and git-tar-tree.txt
too. I'd vote remove it.
-- 
Duy

[-- Attachment #2: attr-1.patch --]
[-- Type: text/x-patch, Size: 1927 bytes --]

diff --git a/archive.c b/archive.c
index 0ce628b..f79d005 100644
--- a/archive.c
+++ b/archive.c
@@ -147,12 +147,34 @@ static int write_archive_entry(const unsigned char *sha1, const char *base,
 	return err;
 }
 
+static int read_gitattr_to_index(const unsigned char *sha1, const char *base, int baselen, const char *pathname, unsigned mode, int stage, void *context)
+{
+	struct cache_entry *ce;
+	unsigned int size;
+	int len;
+	static int gitattr_len = 0;
+
+	if (S_ISDIR(mode))
+		return READ_TREE_RECURSIVE;
+
+	len = strlen(pathname);
+	if (!gitattr_len)
+		gitattr_len = strlen(GITATTRIBUTES_FILE);
+	if (len < gitattr_len || strcmp(pathname+len-gitattr_len, GITATTRIBUTES_FILE))
+		return 0;
+	size = cache_entry_size(len);
+	ce = xcalloc(1, size);
+	ce->ce_mode = create_ce_mode(mode);
+	ce->ce_flags = create_ce_flags(len, stage);
+	memcpy(ce->name, pathname, len+1);
+	hashcpy(ce->sha1, sha1);
+	return add_cache_entry(ce, ADD_CACHE_OK_TO_ADD | ADD_CACHE_SKIP_DFCHECK);
+}
+
 int write_archive_entries(struct archiver_args *args,
 		write_archive_entry_fn_t write_entry)
 {
 	struct archiver_context context;
-	struct unpack_trees_options opts;
-	struct tree_desc t;
 	int err;
 
 	if (args->baselen > 0 && args->base[args->baselen - 1] == '/') {
@@ -170,20 +192,8 @@ int write_archive_entries(struct archiver_args *args,
 
 	context.args = args;
 	context.write_entry = write_entry;
-
-	/*
-	 * Setup index and instruct attr to read index only
-	 */
 	if (!args->worktree_attributes) {
-		memset(&opts, 0, sizeof(opts));
-		opts.index_only = 1;
-		opts.head_idx = -1;
-		opts.src_index = &the_index;
-		opts.dst_index = &the_index;
-		opts.fn = oneway_merge;
-		init_tree_desc(&t, args->tree->buffer, args->tree->size);
-		if (unpack_trees(1, &t, &opts))
-			return -1;
+		read_tree_recursive(args->tree, NULL, 0, 0, NULL, read_gitattr_to_index, NULL);
 		git_attr_set_direction(GIT_ATTR_INDEX, &the_index);
 	}