On Sat, 5 Jan 2019 at 07:07, Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> wrote:
>
> hold_locked_index() assumes the index path at $GIT_DIR/index. This is
> not good for places that take an arbitrary index_state instead of
> the_index, which is basically everywhere except builtin/.
>
> Replace it with repo_hold_locked_index(). hold_locked_index() remains
> as a wrapper around repo_hold_locked_index() to reduce changes in builtin/
> diff --git a/builtin/clone.c b/builtin/clone.c
> index 7c7f98c72c..ddb3230d21 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -8,6 +8,7 @@
> * Clone a repository into a different directory that does not yet exist.
> */
>
> +#define USE_THE_INDEX_COMPATIBILITY_MACROS
I think this should be in patch 10/10.
> diff --git a/cache.h b/cache.h
> index ca36b44ee0..634c9ce325 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -433,6 +433,7 @@ void validate_cache_entries(const struct index_state *istate);
> #define unmerge_cache_entry_at(at) unmerge_index_entry_at(&the_index, at)
> #define unmerge_cache(pathspec) unmerge_index(&the_index, pathspec)
> #define read_blob_data_from_cache(path, sz) read_blob_data_from_index(&the_index, (path), (sz))
> +#define hold_locked_index(lock_file, flags) repo_hold_locked_index(the_repository, (lock_file), (flags))
> #endif
>
> #define TYPE_BITS 3
> @@ -833,7 +834,6 @@ extern struct cache_entry *refresh_cache_entry(struct index_state *, struct cach
> */
> extern void update_index_if_able(struct index_state *, struct lock_file *);
>
> -extern int hold_locked_index(struct lock_file *, int);
The reason this moves is it gets protected with "#ifndef
NO_THE_INDEX_COMPATIBILITY_MACROS". Ok.
> -int hold_locked_index(struct lock_file *lk, int lock_flags)
> -{
> - return hold_lock_file_for_update(lk, get_index_file(), lock_flags);
> -}
> +int repo_hold_locked_index(struct repository *repo,
> + struct lock_file *lf,
> + int flags)
> +{
> + return hold_lock_file_for_update(lf, repo->index_file, flags);
> +}
`get_index_file()` BUGs if `the_repository->index_file` is NULL, but
other than that, this looks like a faithful conversion. Do we want to
keep that check here?
Martin
