git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Brandon Williams <bmwill@google.com>
To: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 16/24] attr: remove index from git_attr_set_direction()
Date: Mon, 13 Aug 2018 10:22:53 -0700	[thread overview]
Message-ID: <20180813172253.GC240194@google.com> (raw)
In-Reply-To: <20180813161441.16824-17-pclouds@gmail.com>

On 08/13, Nguyễn Thái Ngọc Duy wrote:
> Since attr checking API now take the index, there's no need to set an
> index in advance with this call. Most call sites are straightforward
> because they either pass the_index or NULL (which defaults back to
> the_index previously). There's only one suspicious call site in
> unpack-trees.c where it sets a different index.
> 
> This code in unpack-trees is about to check out entries from the
> new/temporary index after merging is done in it. The attributes will
> be used by entry.c code to do crlf conversion if needed. entry.c now
> respects struct checkout's istate field, and this field is correctly
> set in unpack-trees.c, there should be no regression from this change.

Thanks for fixing this! The API is now that much cleaner :)

> 
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  archive.c            |  2 +-
>  attr.c               | 15 +++------------
>  attr.h               |  3 +--
>  builtin/check-attr.c |  2 +-
>  unpack-trees.c       |  4 ++--
>  5 files changed, 8 insertions(+), 18 deletions(-)
> 
> diff --git a/archive.c b/archive.c
> index c81e35bf23..aca9179d03 100644
> --- a/archive.c
> +++ b/archive.c
> @@ -274,7 +274,7 @@ int write_archive_entries(struct archiver_args *args,
>  		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);
> +		git_attr_set_direction(GIT_ATTR_INDEX);
>  	}
>  
>  	err = read_tree_recursive(args->tree, "", 0, 0, &args->pathspec,
> diff --git a/attr.c b/attr.c
> index 863fad3bd1..98e4953f6e 100644
> --- a/attr.c
> +++ b/attr.c
> @@ -708,10 +708,8 @@ static struct attr_stack *read_attr_from_array(const char **list)
>   * another thread could potentially be calling into the attribute system.
>   */
>  static enum git_attr_direction direction;
> -static const struct index_state *use_index;
>  
> -void git_attr_set_direction(enum git_attr_direction new_direction,
> -			    const struct index_state *istate)
> +void git_attr_set_direction(enum git_attr_direction new_direction)
>  {
>  	if (is_bare_repository() && new_direction != GIT_ATTR_INDEX)
>  		BUG("non-INDEX attr direction in a bare repo");
> @@ -720,7 +718,6 @@ void git_attr_set_direction(enum git_attr_direction new_direction,
>  		drop_all_attr_stacks();
>  
>  	direction = new_direction;
> -	use_index = istate;
>  }
>  
>  static struct attr_stack *read_attr_from_file(const char *path, int macro_ok)
> @@ -750,17 +747,11 @@ static struct attr_stack *read_attr_from_index(const struct index_state *istate,
>  	struct attr_stack *res;
>  	char *buf, *sp;
>  	int lineno = 0;
> -	const struct index_state *to_read_from;
>  
> -	/*
> -	 * Temporary workaround for c24f3abace (apply: file commited
> -	 * with CRLF should roundtrip diff and apply - 2017-08-19)
> -	 */
> -	to_read_from = use_index ? use_index : istate;
> -	if (!to_read_from)
> +	if (!istate)
>  		return NULL;
>  
> -	buf = read_blob_data_from_index(to_read_from, path, NULL);
> +	buf = read_blob_data_from_index(istate, path, NULL);
>  	if (!buf)
>  		return NULL;
>  
> diff --git a/attr.h b/attr.h
> index 3daca3c0cb..01dab4a126 100644
> --- a/attr.h
> +++ b/attr.h
> @@ -77,8 +77,7 @@ enum git_attr_direction {
>  	GIT_ATTR_CHECKOUT,
>  	GIT_ATTR_INDEX
>  };
> -void git_attr_set_direction(enum git_attr_direction new_direction,
> -			    const struct index_state *istate);
> +void git_attr_set_direction(enum git_attr_direction new_direction);
>  
>  void attr_start(void);
>  
> diff --git a/builtin/check-attr.c b/builtin/check-attr.c
> index f7b59993d3..c05573ff9c 100644
> --- a/builtin/check-attr.c
> +++ b/builtin/check-attr.c
> @@ -120,7 +120,7 @@ int cmd_check_attr(int argc, const char **argv, const char *prefix)
>  	}
>  
>  	if (cached_attrs)
> -		git_attr_set_direction(GIT_ATTR_INDEX, NULL);
> +		git_attr_set_direction(GIT_ATTR_INDEX);
>  
>  	doubledash = -1;
>  	for (i = 0; doubledash < 0 && i < argc; i++) {
> diff --git a/unpack-trees.c b/unpack-trees.c
> index 14e9043f9d..f25089b878 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -353,7 +353,7 @@ static int check_updates(struct unpack_trees_options *o)
>  	progress = get_progress(o);
>  
>  	if (o->update)
> -		git_attr_set_direction(GIT_ATTR_CHECKOUT, index);
> +		git_attr_set_direction(GIT_ATTR_CHECKOUT);
>  
>  	if (should_update_submodules() && o->update && !o->dry_run)
>  		load_gitmodules_file(index, NULL);
> @@ -413,7 +413,7 @@ static int check_updates(struct unpack_trees_options *o)
>  	stop_progress(&progress);
>  	errs |= finish_delayed_checkout(&state);
>  	if (o->update)
> -		git_attr_set_direction(GIT_ATTR_CHECKIN, NULL);
> +		git_attr_set_direction(GIT_ATTR_CHECKIN);
>  	return errs != 0;
>  }
>  
> -- 
> 2.18.0.1004.g6639190530
> 

-- 
Brandon Williams

  reply	other threads:[~2018-08-13 17:22 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-13 16:14 [PATCH 00/24] Kill the_index part3 Nguyễn Thái Ngọc Duy
2018-08-13 16:14 ` [PATCH 01/24] diff.c: move read_index() code back to the caller Nguyễn Thái Ngọc Duy
2018-08-13 16:14 ` [PATCH 02/24] cache-tree: wrap the_index based wrappers with #ifdef Nguyễn Thái Ngọc Duy
2018-08-13 21:18   ` Junio C Hamano
2018-08-13 16:14 ` [PATCH 03/24] attr: remove an implicit dependency on the_index Nguyễn Thái Ngọc Duy
2018-08-13 17:12   ` Brandon Williams
2018-08-13 16:14 ` [PATCH 04/24] convert.c: " Nguyễn Thái Ngọc Duy
2018-08-13 21:21   ` Junio C Hamano
2018-08-13 16:14 ` [PATCH 05/24] dir.c: remove an implicit dependency on the_index in pathspec code Nguyễn Thái Ngọc Duy
2018-08-13 17:17   ` Brandon Williams
2018-08-13 18:40     ` Duy Nguyen
2018-08-13 16:14 ` [PATCH 06/24] preload-index.c: use the right index instead of the_index Nguyễn Thái Ngọc Duy
2018-08-13 16:14 ` [PATCH 07/24] ls-files: correct index argument to get_convert_attr_ascii() Nguyễn Thái Ngọc Duy
2018-08-15 18:56   ` Stefan Beller
2018-08-13 16:14 ` [PATCH 08/24] unpack-trees: remove 'extern' on function declaration Nguyễn Thái Ngọc Duy
2018-08-15 19:10   ` Stefan Beller
2018-08-15 19:21     ` Duy Nguyen
2018-08-15 19:25       ` Stefan Beller
2018-08-13 16:14 ` [PATCH 09/24] unpack-trees: add a note about path invalidation Nguyễn Thái Ngọc Duy
2018-08-13 16:14 ` [PATCH 10/24] unpack-trees: don't shadow global var the_index Nguyễn Thái Ngọc Duy
2018-08-13 16:14 ` [PATCH 11/24] unpack-trees: convert clear_ce_flags* to avoid the_index Nguyễn Thái Ngọc Duy
2018-08-13 16:14 ` [PATCH 12/24] unpack-trees: avoid the_index in verify_absent() Nguyễn Thái Ngọc Duy
2018-08-13 16:14 ` [PATCH 13/24] pathspec.c: use the right index instead of the_index Nguyễn Thái Ngọc Duy
2018-08-13 16:14 ` [PATCH 14/24] submodule.c: " Nguyễn Thái Ngọc Duy
2018-08-13 16:14 ` [PATCH 15/24] entry.c: " Nguyễn Thái Ngọc Duy
2018-08-13 16:14 ` [PATCH 16/24] attr: remove index from git_attr_set_direction() Nguyễn Thái Ngọc Duy
2018-08-13 17:22   ` Brandon Williams [this message]
2018-08-13 16:14 ` [PATCH 17/24] grep: use the right index instead of the_index Nguyễn Thái Ngọc Duy
2018-08-13 16:14 ` [PATCH 18/24] archive.c: avoid access to the_index Nguyễn Thái Ngọc Duy
2018-08-13 16:14 ` [PATCH 19/24] archive-*.c: use the right repository Nguyễn Thái Ngọc Duy
2018-08-13 16:14 ` [PATCH 20/24] resolve-undo.c: use the right index instead of the_index Nguyễn Thái Ngọc Duy
2018-08-13 16:14 ` [PATCH 21/24] apply.c: pass struct apply_state to more functions Nguyễn Thái Ngọc Duy
2018-08-13 16:14 ` [PATCH 22/24] apply.c: make init_apply_state() take a struct repository Nguyễn Thái Ngọc Duy
2018-08-13 16:14 ` [PATCH 23/24] apply.c: remove implicit dependency on the_index Nguyễn Thái Ngọc Duy
2018-08-13 16:14 ` [PATCH 24/24] blame.c: " Nguyễn Thái Ngọc Duy
2018-08-13 17:28 ` [PATCH 00/24] Kill the_index part3 Brandon Williams
2018-08-13 21:24   ` Junio C Hamano
2018-08-15 19:48     ` Stefan Beller

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=20180813172253.GC240194@google.com \
    --to=bmwill@google.com \
    --cc=git@vger.kernel.org \
    --cc=pclouds@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).