Re: check-attr doesn't respect recursive definitions

Jeff King [off-list ref] writes:

On Tue, Apr 02, 2013 at 09:11:02AM -0700, Junio C Hamano wrote:

quoted

Jeff King [off-list ref] writes:

quoted

Yes, it is the expected behavior, though I cannot offhand think of
anything that would break if we did apply it recursively.

Conceptually that breaks our brain.  "All files in doc/ directories
are text" and "doc/ directory is text" are two different things, no?

In some systems, yes, but git does not have any notion of "doc/" as an
item (after all, we track content in files, not directories), so I do
not see what it means to specify a directory except to say "everything
under it has this property".

That was true back when gitattributes (and ignore) was defined to
apply only to the paths we track.  But export-ignore abuses the
attrtibute system, allows a directory to be specified in the match
pattern, and we declared that is a kosher use by the patch that
caused 1.8.1.X regression, no?  So "Git does not have any notion of
"doc/' as an item" is no longer true, I think.

On Tue, Apr 02, 2013 at 09:43:30AM -0700, Junio C Hamano wrote:

quoted

In some systems, yes, but git does not have any notion of "doc/" as an
item (after all, we track content in files, not directories), so I do
not see what it means to specify a directory except to say "everything
under it has this property".

That was true back when gitattributes (and ignore) was defined to
apply only to the paths we track.  But export-ignore abuses the
attrtibute system, allows a directory to be specified in the match
pattern, and we declared that is a kosher use by the patch that
caused 1.8.1.X regression, no?  So "Git does not have any notion of
"doc/' as an item" is no longer true, I think.

Yes, but as I explained later, the meaning of "apply an attribute to
dir" in such cases is always equivalent to "apply attribute
recursively to dir/*". So I do not think we are violating that rule to
recursively apply all attributes.

But let's take a step back. I think Jan is trying to do a very
reasonable thing: come up with the same set of paths that git-archive
would. What's the best way to solve that? Recursive application of
attributes is one way, but is there another way we could help with
solving that?

Using:

  git ls-tree --name-only -zrt HEAD |
  git check-attr --stdin -z export-ignore

means we can find out that "foo/" is ignored. But he would have to
manually post-process the output to see that "foo/bar" is below "foo".
Not impossible, but I just wonder if git can be more helpful in figuring
this out.

-Peff

On Tue, Apr 02, 2013 at 12:51:28PM -0400, Jeff King wrote:

But let's take a step back. I think Jan is trying to do a very
reasonable thing: come up with the same set of paths that git-archive
would. What's the best way to solve that? Recursive application of
attributes is one way, but is there another way we could help with
solving that?

Using:

  git ls-tree --name-only -zrt HEAD |
  git check-attr --stdin -z export-ignore

means we can find out that "foo/" is ignored. But he would have to
manually post-process the output to see that "foo/bar" is below "foo".
Not impossible, but I just wonder if git can be more helpful in figuring
this out.

One way to solve the problem is something like the patch below, which
allows "git archive --format=lstree" to give an lstree-like listing, but
with export-ignore attributes applied.

It feels weirdly specific, though, like there should be a more general
solution.

---
 Makefile         |  1 +
 archive-lstree.c | 43 +++++++++++++++++++++++++++++++++++
 archive.c        |  1 +
 archive.h        |  1 +
 quote.c          |  4 ++--
 quote.h          |  1 +
 6 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index e9749ca..5e63d72 100644
--- a/Makefile
+++ b/Makefile

@@ -772,6 +772,7 @@ LIB_OBJS += archive.o
 LIB_OBJS += alias.o
 LIB_OBJS += alloc.o
 LIB_OBJS += archive.o
+LIB_OBJS += archive-lstree.o
 LIB_OBJS += archive-tar.o
 LIB_OBJS += archive-zip.o
 LIB_OBJS += argv-array.o

diff --git a/archive-lstree.c b/archive-lstree.c
new file mode 100644
index 0000000..5ca1bbc
--- /dev/null
+++ b/archive-lstree.c

@@ -0,0 +1,43 @@
+#include "cache.h"
+#include "archive.h"
+#include "quote.h"
+#include "commit.h"
+#include "blob.h"
+
+static int write_lstree_entry(struct archiver_args *args,
+			      const unsigned char *sha1,
+			      const char *path, size_t pathlen,
+			      unsigned int mode)
+{
+	const char *type;
+
+	if (S_ISDIR(mode))
+		return 0;
+	else if (S_ISGITLINK(mode))
+		type = commit_type;
+	else
+		type = blob_type;
+
+	printf("%06o %s %s\t", mode, type, sha1_to_hex(sha1));
+	quote_c_style_counted(path, pathlen, NULL, stdout, 0);
+	printf("\n");
+
+	return 0;
+}
+
+static int write_lstree_archive(const struct archiver *ar,
+				struct archiver_args *args)
+{
+	return write_archive_entries(args, write_lstree_entry);
+}
+
+static struct archiver lstree_archiver = {
+	"lstree",
+	write_lstree_archive,
+	ARCHIVER_REMOTE
+};
+
+void init_lstree_archiver(void)
+{
+	register_archiver(&lstree_archiver);
+}

diff --git a/archive.c b/archive.c
index 3f00da6..4966db7 100644
--- a/archive.c
+++ b/archive.c

@@ -420,6 +420,7 @@ int write_archive(int argc, const char **argv, const char *prefix,
 	git_config(git_default_config, NULL);
 	init_tar_archiver();
 	init_zip_archiver();
+	init_lstree_archiver();
 
 	argc = parse_archive_args(argc, argv, &ar, &args, name_hint, remote);
 	if (nongit) {

diff --git a/archive.h b/archive.h
index 895afcd..1428fc4 100644
--- a/archive.h
+++ b/archive.h

@@ -27,6 +27,7 @@ extern void init_zip_archiver(void);
 
 extern void init_tar_archiver(void);
 extern void init_zip_archiver(void);
+extern void init_lstree_archiver(void);
 
 typedef int (*write_archive_entry_fn_t)(struct archiver_args *args,
 					const unsigned char *sha1,

diff --git a/quote.c b/quote.c
index 911229f..da6b7e4 100644
--- a/quote.c
+++ b/quote.c

@@ -206,8 +206,8 @@ static size_t next_quote_pos(const char *s, ssize_t maxlen)
  *     of name, enclosed with double quotes if asked and needed only.
  *     Return value is the same as in (1).
  */
-static size_t quote_c_style_counted(const char *name, ssize_t maxlen,
-                                    struct strbuf *sb, FILE *fp, int no_dq)
+size_t quote_c_style_counted(const char *name, ssize_t maxlen,
+			     struct strbuf *sb, FILE *fp, int no_dq)
 {
 #undef EMIT
 #define EMIT(c)                                 \

diff --git a/quote.h b/quote.h
index 133155a..f2b8acb 100644
--- a/quote.h
+++ b/quote.h

@@ -55,6 +55,7 @@ extern size_t quote_c_style(const char *name, struct strbuf *, FILE *, int no_dq
 
 extern int unquote_c_style(struct strbuf *, const char *quoted, const char **endp);
 extern size_t quote_c_style(const char *name, struct strbuf *, FILE *, int no_dq);
+extern size_t quote_c_style_counted(const char *name, ssize_t len, struct strbuf *, FILE *, int no_dq);
 extern void quote_two_c_style(struct strbuf *, const char *, const char *, int);
 
 extern void write_name_quoted(const char *name, FILE *, int terminator);