public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox•com>
To: "Nguyễn Thái Ngọc Duy" <pclouds@gmail•com>
Cc: git@vger•kernel.org, "René Scharfe" <rene.scharfe@lsrfire•ath.cx>
Subject: Re: [PATCH v2 3/3] archive: do not read .gitattributes in working directory
Date: Thu, 09 Apr 2009 01:49:35 -0700	[thread overview]
Message-ID: <7vws9u2ov4.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: 1239260490-6318-4-git-send-email-pclouds@gmail.com

Nguyễn Thái Ngọc Duy  <pclouds@gmail•com> writes:

> @@ -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);

Why use unpack_trees with oneway_merge?  You won't be doing "is this file
up-to-date in the work tree?", and you won't be writing the index out
either, so there is nothing gained by keeping the cached stat information
fresh, which is the major justification of using that mechanism.  I think
using tree.c::read_tree() would be more appropriate.

> diff --git a/builtin-tar-tree.c b/builtin-tar-tree.c
> index 0713bca..69a93fc 100644
> --- a/builtin-tar-tree.c
> +++ b/builtin-tar-tree.c
> @@ -36,6 +36,11 @@ int cmd_tar_tree(int argc, const char **argv, const char *prefix)
>  		argv++;
>  		argc--;
>  	}
> +	if (2 <= argc && !strcmp(argv[1], "--fix-attributes")) {
> +		nargv[nargc++] = argv[1];
> +		argv++;
> +		argc--;
> +	}

I am not sure if it is worth backporting this new option to tar-tree which
is an essentially backward-compatibility interface, and worse yet, doing
it poorly (i.e. --fix-attributes must come after --remote= for unexplained
reason).

It would affect a bit more tests, but I think you would want to test both
the new "normal" mode of operation (generate archives with "git archive"
and "git tar-tree" without options and compare, for example), instead of
adding --fix-attributes everywhere.

  reply	other threads:[~2009-04-09  8:51 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-09  7:01 [PATCH v2 0/3] support "in-tree attributes" for git-archive Nguyễn Thái Ngọc Duy
2009-04-09  7:01 ` [PATCH v2 1/3] archive: add shortcuts for --format and --prefix Nguyễn Thái Ngọc Duy
2009-04-09  7:01   ` [PATCH v2 2/3] attr: add GIT_ATTR_INDEX "direction" Nguyễn Thái Ngọc Duy
2009-04-09  7:01     ` [PATCH v2 3/3] archive: do not read .gitattributes in working directory Nguyễn Thái Ngọc Duy
2009-04-09  8:49       ` Junio C Hamano [this message]
2009-04-09 10:53         ` Nguyen Thai Ngoc Duy
2009-04-11 19:22           ` Junio C Hamano
2009-04-13 10:41           ` René Scharfe
2009-04-13 12:18             ` René Scharfe
2009-04-13 13:08               ` René Scharfe
2009-04-14  5:11                 ` Junio C Hamano
2009-04-14 21:15                   ` René Scharfe
2009-04-16  0:26                     ` Junio C Hamano
2009-04-09  8:47   ` [PATCH v2 1/3] archive: add shortcuts for --format and --prefix Junio C Hamano
2009-04-09 10:38     ` Nguyen Thai Ngoc Duy

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=7vws9u2ov4.fsf@gitster.siamese.dyndns.org \
    --to=gitster@pobox$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=pclouds@gmail$(echo .)com \
    --cc=rene.scharfe@lsrfire$(echo .)ath.cx \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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