git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Nguyen Thai Ngoc Duy <pclouds@gmail.com>
Date
Apr 16, 2009, 12:38 UTC
Message-ID
<fcaeb9bf0904160538w4a826c57r40e9cf1c52b0b6ff@mail.gmail.com>
In-Reply-To
<fcaeb9bf0904160350m14dce828oe88bda6562d98f48@mail.gmail.com>
On Thu, Apr 16, 2009 at 8:50 PM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
Show 9 quoted lines
> 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

Here it is (again on top of your patch). Need to read directories twice, but not as bad as read_tree_recursive() twice.

-- 
Duy


diff --git a/archive.c b/archive.c
index 0ce628b..8df53a8 100644
--- a/archive.c
+++ b/archive.c
@@ -97,6 +97,43 @@ struct archiver_context {
 	write_archive_entry_fn_t write_entry;
 };
 
+static int read_gitattr_to_index(struct tree *tree, const char *base, int baselen, struct archiver_args *args)
+{
+	struct tree_desc desc;
+	struct name_entry entry;
+	struct cache_entry *ce;
+	unsigned int size;
+	int pathlen;
+
+	if (parse_tree(tree))
+		return -1;
+
+	init_tree_desc(&desc, tree->buffer, tree->size);
+
+	while (tree_entry(&desc, &entry)) {
+		if (S_ISDIR(entry.mode) || S_ISGITLINK(entry.mode))
+			continue;
+		if (strcmp(entry.path, GITATTRIBUTES_FILE))
+			continue;
+		pathlen = tree_entry_len(entry.path, entry.sha1);
+		baselen -= args->baselen; /* remove user prefix */
+		if (baselen)
+			baselen++; /* slash */
+		size = cache_entry_size(baselen + pathlen);
+		ce = xcalloc(1, size);
+		ce->ce_mode = create_ce_mode(entry.mode);
+		ce->ce_flags = create_ce_flags(baselen + pathlen, 0);
+		if (baselen) {
+			memcpy(ce->name, base + args->baselen, baselen-1);
+			ce->name[baselen-1] = '/';
+		}
+		memcpy(ce->name + baselen, entry.path, pathlen + 1);
+		hashcpy(ce->sha1, entry.sha1);
+		return add_cache_entry(ce, ADD_CACHE_OK_TO_ADD | ADD_CACHE_SKIP_DFCHECK);
+	}
+	return 0;
+}
+
 static int write_archive_entry(const unsigned char *sha1, const char *base,
 		int baselen, const char *filename, unsigned mode, int stage,
 		void *context)
@@ -119,11 +156,25 @@ static int write_archive_entry(const unsigned char *sha1, const char *base,
 	strbuf_addstr(&path, filename);
 	path_without_prefix = path.buf + args->baselen;
 
-	setup_archive_check(check);
-	if (!git_checkattr(path_without_prefix, ARRAY_SIZE(check), check)) {
-		if (ATTR_TRUE(check[0].value))
-			return 0;
-		convert = ATTR_TRUE(check[1].value);
+	if (S_ISDIR(mode)) {
+		/*
+		 * we want to read .gitattributes before any entry is processed
+		 * so every time we get a directory entry, we look ahead to see
+		 * if there is .gitattributes and load it
+		 *
+		 * later when the directory is processed, .gitattributes is
+		 * already ready in index for git_checkattr()
+		 */
+		if (!args->worktree_attributes)
+			read_gitattr_to_index(lookup_tree(sha1), base, baselen, args);
+	}
+	else {
+		setup_archive_check(check);
+		if (!git_checkattr(path_without_prefix, ARRAY_SIZE(check), check)) {
+			if (ATTR_TRUE(check[0].value))
+				return 0;
+			convert = ATTR_TRUE(check[1].value);
+		}
 	}
 
 	if (S_ISDIR(mode) || S_ISGITLINK(mode)) {
@@ -151,8 +202,6 @@ 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] == '/') {
@@ -171,19 +220,9 @@ 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 .gitattributes at root if any */
+		read_gitattr_to_index(args->tree, args->base, args->baselen, args);
 		git_attr_set_direction(GIT_ATTR_INDEX, &the_index);
 	}
 
@@ -209,13 +248,23 @@ static const struct archiver *lookup_archiver(const char *name)
 }
 
 static void parse_pathspec_arg(const char **pathspec,
-		struct archiver_args *ar_args)
+		struct archiver_args *ar_args,
+		const char *prefix)
 {
-	ar_args->pathspec = get_pathspec(ar_args->base, pathspec);
+	struct strbuf s = STRBUF_INIT;
+	if (ar_args->base)
+		strbuf_addstr(&s, ar_args->base);
+	if (prefix)
+		strbuf_addstr(&s, prefix);
+	ar_args->pathspec = get_pathspec(s.len ? s.buf : NULL, pathspec);
+	/*
+	 * s.buf must never be freed because
+	 * get_pathspec does not duplicate it
+	 */
 }
 
 static void parse_treeish_arg(const char **argv,
-		struct archiver_args *ar_args, const char *prefix)
+		struct archiver_args *ar_args)
 {
 	const char *name = argv[0];
 	const unsigned char *commit_sha1;
@@ -240,18 +289,6 @@ static void parse_treeish_arg(const char **argv,
 	if (tree == NULL)
 		die("not a tree object");
 
-	if (prefix) {
-		unsigned char tree_sha1[20];
-		unsigned int mode;
-		int err;
-
-		err = get_tree_entry(tree->object.sha1, prefix,
-				     tree_sha1, &mode);
-		if (err || !S_ISDIR(mode))
-			die("current working directory is untracked");
-
-		tree = parse_tree_indirect(tree_sha1);
-	}
 	ar_args->tree = tree;
 	ar_args->commit_sha1 = commit_sha1;
 	ar_args->commit = commit;
@@ -360,8 +397,8 @@ int write_archive(int argc, const char **argv, const char *prefix,
 	if (setup_prefix && prefix == NULL)
 		prefix = setup_git_directory();
 
-	parse_treeish_arg(argv, &args, prefix);
-	parse_pathspec_arg(argv + 1, &args);
+	parse_treeish_arg(argv, &args);
+	parse_pathspec_arg(argv + 1, &args, prefix);
 
 	git_config(git_default_config, NULL);
 
Previous: Nguyen Thai Ngoc DuyNext: René Scharfe
Message 12 of 19 in “archive attribute series”
  1. 0/5 archive attribute seriesJunio C Hamano, Apr 16, 2009
  2. 1/5 archive tests: do not use .gitattributes in working directoryJunio C Hamano, Apr 16, 2009
  3. 2/5 attr: add GIT_ATTR_INDEX "direction"Junio C Hamano, Apr 16, 2009
  4. 3/5 unpack-trees: do not muck with attributes when we are not checking outJunio C Hamano, Apr 16, 2009
  5. 4/5 archive: do not read .gitattributes in working directoryJunio C Hamano, Apr 16, 2009
  6. 5/5 archive test: test new --fix-attributes featureJunio C Hamano, Apr 16, 2009
  7. René ScharfeApr 17, 2009
  8. Junio C HamanoApr 16, 2009
  9. Jeff KingApr 16, 2009
  10. Jakub NarebskiApr 16, 2009
  11. Nguyen Thai Ngoc DuyApr 16, 2009
  12. Nguyen Thai Ngoc DuyApr 16, 2009
  13. René ScharfeApr 17, 2009
  14. René ScharfeApr 17, 2009
  15. 1/5 archive tests: do not use .gitattributes in working directoryRené Scharfe, Apr 17, 2009
  16. 2/5 attr: add GIT_ATTR_INDEX "direction"René Scharfe, Apr 17, 2009
  17. 3/5 unpack-trees: do not muck with attributes when we are not checking outRené Scharfe, Apr 17, 2009
  18. 4/5 archive: do not read .gitattributes in working directoryRené Scharfe, Apr 17, 2009
  19. 5/5 archive test: attributesRené Scharfe, Apr 17, 2009

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.