Kill the_index part3
cover Kill the_index part3
This is the third part of killing the_index (at least outside builtin/). Part 1 [1] is dropped. Part 2 is nd/no-extern on 'pu'. This part is built on top of nd/no-extern.
This series would actually break 'pu' because builtin/stash.c uses three functions that are updated here. So we would need something like the following patch to make it build again.
I don't know if that adds too much work on Junio. If it does, I guess I'll hold this off for a while until builtin/stash.c gets merged because reordering these patches, pushing the patches that break stash.c away, really takes a lot of work.
[1] https://public-inbox.org/git/[email protected]/
builtin/stash.c +3 -3
@@ -294,7 +294,7 @@ static int apply_patch_from_buf(struct strbuf *patch, int cached, int reverse,const char *patch_path =".git/stash_patch.patch" ;FILE *patch_file; -if (init_apply_state (&state,NULL )) +if (init_apply_state (&state, the_repository,NULL ))return -1 ; state.cached = cached;
@@ -873,7 +873,7 @@ static int get_untracked_files(const char **argv, const char *prefix, max_len =fill_directory (&dir, the_repository->index, &pathspec);for (i =0 ; i < dir.nr; i++) {struct dir_entry *ent = dir.entries[i]; -if (!dir_path_match (ent, &pathspec, max_len, seen)) { +if (!dir_path_match (the_repository->index, ent, &pathspec, max_len, seen)) {free (ent);continue ; }
@@ -1299,7 +1299,7 @@ static int do_push_stash(int argc, const char **argv, const char *prefix,for (i =0 ; i < active_nr; ++i) {const struct cache_entry *ce = active_cache[i]; -if (!ce_path_match (ce, &ps, ps_matched)) +if (!ce_path_match (&the_index, ce, &ps, ps_matched))continue ; }
Nguyễn Thái Ngọc Duy (24):
diff.c: move read_index() code back to the caller
cache-tree: wrap the_index based wrappers with #ifdef
attr: remove an implicit dependency on the_index convert.c: remove an implicit dependency on the_index dir.c: remove an implicit dependency on the_index in pathspec code preload-index.c: use the right index instead of the_index
ls-files: correct index argument to get_convert_attr_ascii()
unpack-trees: remove 'extern' on function declaration
unpack-trees: add a note about path invalidation
unpack-trees: don't shadow global var the_index
unpack-trees: convert clear_ce_flags* to avoid the_index
unpack-trees: avoid the_index in verify_absent() pathspec.c: use the right index instead of the_index submodule.c: use the right index instead of the_index entry.c: use the right index instead of the_index
attr: remove index from git_attr_set_direction()
grep: use the right index instead of the_index archive.c: avoid access to the_index
archive-*.c: use the right repository
resolve-undo.c: use the right index instead of the_index apply.c: pass struct apply_state to more functions apply.c: make init_apply_state() take a struct repository apply.c: remove implicit dependency on the_index blame.c: remove implicit dependency on the_index
apply.c | 66 +++++++++++++++++++++---------------- apply.h | 4 +++ archive-tar.c | 2 +- archive-zip.c | 2 +- archive.c | 47 ++++++++++++++++---------- archive.h | 16 +++++++-- attr.c | 52 +++++++++++++++++------------ attr.h | 11 ++++--- blame.c | 52 +++++++++++++++++------------ blame.h | 1 + builtin/add.c | 6 ++-- builtin/am.c | 2 +- builtin/apply.c | 2 +- builtin/archive.c | 2 +- builtin/blame.c | 1 + builtin/cat-file.c | 2 +- builtin/check-attr.c | 6 ++-- builtin/checkout-index.c | 1 + builtin/checkout.c | 2 +- builtin/clean.c | 2 +- builtin/commit.c | 2 +- builtin/diff-tree.c | 8 +++-- builtin/grep.c | 6 ++-- builtin/ls-files.c | 17 +++++----- builtin/pack-objects.c | 2 +- builtin/rm.c | 2 +- builtin/submodule--helper.c | 2 +- builtin/update-index.c | 2 +- builtin/upload-archive.c | 3 +- cache-tree.c | 12 ------- cache-tree.h | 17 ++++++++-- convert.c | 41 +++++++++++++---------- convert.h | 15 ++++++--- diff-lib.c | 4 +-- diff.c | 12 +------ diff.h | 1 - dir.c | 27 ++++++++------- dir.h | 16 +++++---- entry.c | 9 ++--- ll-merge.c | 4 +-- merge-recursive.c | 2 +- pathspec.c | 2 +- preload-index.c | 2 +- read-cache.c | 2 +- rerere.c | 2 +- resolve-undo.c | 2 +- revision.c | 2 +- sequencer.c | 4 +-- sha1-file.c | 4 +-- submodule.c | 8 ++--- unpack-trees.c | 57 ++++++++++++++++++++------------ unpack-trees.h | 4 +-- userdiff.c | 2 +- ws.c | 2 +- wt-status.c | 6 ++-- 55 files changed, 337 insertions(+), 245 deletions(-)
Signature · 1 line
2.18.0.1004.g6639190530
PATCH 1/24 diff.c: move read_index() code back to the caller
This code is only needed for diff-tree (since f0c6b2a2fd ([PATCH] Optimize diff-tree -[CM] --stdin - 2005-05-27)). Let the caller do the preparation instead and avoid read_index() in diff.c code.
read_index() should be avoided (in addition to the_index) because it uses get_index_file() underneath to get the path $GIT_DIR/index. This effectively pulls the_repository in and may become the only reason to pull a 'struct repository *' in diff.c. Let's keep the dependencies as few as possible and kick it back to diff-tree.c
--- builtin/diff-tree.c | 8 +++++--- diff.c | 10 ---------- diff.h | 1 - 3 files changed, 5 insertions(+), 14 deletions(-)
builtin/diff-tree.c +5 -3
@@ -163,9 +163,11 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)int saved_nrl =0 ;int saved_dcctc =0 ; -if (opt->diffopt.detect_rename) - opt->diffopt.setup |= (DIFF_SETUP_USE_SIZE_CACHE | -DIFF_SETUP_USE_CACHE ); +if (opt->diffopt.detect_rename) { +if (!the_index.cache) +read_index (&the_index); + opt->diffopt.setup |=DIFF_SETUP_USE_SIZE_CACHE ; + }while (fgets (line,sizeof (line), stdin)) {struct object_id oid;
diff.c +0 -10
@@ -4414,16 +4414,6 @@ void diff_setup_done(struct diff_options *options)if (options->detect_rename && options->rename_limit <0 ) options->rename_limit = diff_rename_limit_default; -if (options->setup &DIFF_SETUP_USE_CACHE ) { -if (!active_cache) -/* read-cache does not die even when it fails -* so it is safe for us to do this here. Also -* it does not smudge active_cache or active_nr -* when it fails, so we do not have to worry about -* cleaning it up ourselves either. -*/ -read_cache (); - }if (hexsz < options->abbrev) options->abbrev = hexsz;/* full */
diff.h +0 -1
@@ -312,7 +312,6 @@ void diff_change(struct diff_options *,struct diff_filepair *diff_unmerge (struct diff_options *,const char *path);#define DIFF_SETUP_REVERSE 1 -#define DIFF_SETUP_USE_CACHE 2#define DIFF_SETUP_USE_SIZE_CACHE 4/*
Signature · 1 line
2.18.0.1004.g6639190530
PATCH 2/24 cache-tree: wrap the_index based wrappers with #ifdef
In reply to Nguyễn Thái Ngọc DuyThis puts update_main_cache_tree() and write_cache_as_tree() in the same group of "index compat" functions that assume the_index implicitly, which should only be used within builtin/ or t/helper.
sequencer.c is also updated to not use these functions. As of now, no files outside builtin/ use these functions anymore.
--- cache-tree.c | 12 ------------ cache-tree.h | 17 ++++++++++++++--- sequencer.c | 4 ++-- 3 files changed, 16 insertions(+), 17 deletions(-)
cache-tree.c +0 -12
@@ -652,11 +652,6 @@ int write_index_as_tree(struct object_id *oid, struct index_state *index_state,return ret; } -int write_cache_as_tree (struct object_id *oid,int flags,const char *prefix) -{ -return write_index_as_tree (oid, &the_index,get_index_file (), flags, prefix); -} -static void prime_cache_tree_rec (struct cache_tree *it,struct tree *tree) {struct tree_desc desc;
@@ -723,10 +718,3 @@ int cache_tree_matches_traversal(struct cache_tree *root,return it->entry_count;return 0 ; } - -int update_main_cache_tree (int flags) -{ -if (!the_index.cache_tree) - the_index.cache_tree =cache_tree (); -return cache_tree_update (&the_index, flags); -}
cache-tree.h +14 -3
@@ -33,8 +33,6 @@ struct cache_tree *cache_tree_read(const char *buffer, unsigned long size);int cache_tree_fully_valid (struct cache_tree *);int cache_tree_update (struct index_state *,int ); -int update_main_cache_tree (int ); -/* bitmasks to write_cache_as_tree flags */ #define WRITE_TREE_MISSING_OK 1#define WRITE_TREE_IGNORE_CACHE_TREE 2
@@ -48,9 +46,22 @@ int update_main_cache_tree(int);#define WRITE_TREE_PREFIX_ERROR (-3)int write_index_as_tree (struct object_id *oid,struct index_state *index_state,const char *index_path,int flags,const char *prefix); -int write_cache_as_tree (struct object_id *oid,int flags,const char *prefix);void prime_cache_tree (struct index_state *,struct tree *);int cache_tree_matches_traversal (struct cache_tree *,struct name_entry *ent,struct traverse_info *info); +#ifndef NO_THE_INDEX_COMPATIBILITY_MACROS +static inline int write_cache_as_tree (struct object_id *oid,int flags,const char *prefix) +{ +return write_index_as_tree (oid, &the_index,get_index_file (), flags, prefix); +} + +static inline int update_main_cache_tree (int flags) +{ +if (!the_index.cache_tree) + the_index.cache_tree =cache_tree (); +return cache_tree_update (&the_index, flags); +} +#endif +#endif
sequencer.c +2 -2
@@ -1244,7 +1244,7 @@ static int try_to_commit(struct strbuf *msg, const char *author,commit_list_insert (current_head, &parents); } -if (write_cache_as_tree (&tree,0 ,NULL )) { +if (write_index_as_tree (&tree, &the_index,get_index_file (),0 ,NULL )) { res =error (_ ("git write-tree failed to write a tree" )); goto out; }
@@ -1630,7 +1630,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,* that represents the "current" state for merge-recursive * to work on. */ -if (write_cache_as_tree (&head,0 ,NULL )) +if (write_index_as_tree (&head, &the_index,get_index_file (),0 ,NULL ))return error (_ ("your index file is unmerged." )); }else { unborn =get_oid ("HEAD" , &head);
Signature · 1 line
2.18.0.1004.g6639190530
Re: [PATCH 02/24] cache-tree: wrap the_index based wrappers with #ifdef
Quoted 6 lines · Nguyễn Thái Ngọc Duy
This puts
update_main_cache_tree()andwrite_cache_as_tree()in the same group of "index compat" functions that assumethe_indeximplicitly, which should only be used within builtin/ or t/helper.
sequencer.cis also updated to not use these functions. As of now, no files outside builtin/ use these functions anymore.
Makes sense.
PATCH 3/24 attr: remove an implicit dependency on the_index
In reply to Nguyễn Thái Ngọc DuyMake the attr API take an index_state instead of assuming the_index in attr code. All call sites are converted blindly to keep the patch simple and retain current behavior. Individual call sites may receive further updates to use the right index instead of the_index.
There is one ugly temporary workaround added in attr.c that needs some more explanation.
Commit c24f3abace (apply: file commited with CRLF should roundtrip diff and apply - 2017-08-19) forces one convert_to_git() call to NOT read the index at all. But what do you know, we read it anyway by falling back to the_index. When "istate" from convert_to_git is now propagated down to read_attr_from_array() we will hit segfault somewhere inside read_blob_data_from_index.
The right way of dealing with this is to kill "use_index" variable and only follow "istate" but at this stage we are not ready for that:
while most git_attr_set_direction() calls just passes the_index to be assigned to use_index, unpack-trees passes a different one which is used by entry.c code, which has no way to know what index to use if we delete use_index. So this has to be done later.
--- archive.c | 2 +- attr.c | 57 ++++++++++++++++++++++++++++-------------- attr.h | 10 +++++--- builtin/check-attr.c | 4 +-- builtin/pack-objects.c | 2 +- convert.c | 2 +- dir.c | 2 +- ll-merge.c | 4 +-- userdiff.c | 2 +- ws.c | 2 +- 10 files changed, 55 insertions(+), 32 deletions(-)
archive.c +1 -1
@@ -109,7 +109,7 @@ static const struct attr_check *get_archive_attrs(const char *path)static struct attr_check *check;if (!check) check =attr_check_initl ("export-ignore" ,"export-subst" ,NULL ); -return git_check_attr (path, check) ?NULL : check; +return git_check_attr (&the_index, path, check) ?NULL : check; }static int check_attr_export_ignore (const struct attr_check *check)
attr.c +38 -19
155 lines not on this page · raw message
attr.h +7 -3
@@ -1,6 +1,8 @@#ifndef ATTR_H #define ATTR_H +struct index_state ; +/* An attribute is a pointer to this opaque structure */ struct git_attr ;
@@ -60,13 +62,15 @@ void attr_check_free(struct attr_check *check);*/ const char *git_attr_name (const struct git_attr *); -int git_check_attr (const char *path,struct attr_check *check); +int git_check_attr (const struct index_state *istate, +const char *path,struct attr_check *check);/* * Retrieve all attributes that apply to the specified path. * check holds the attributes and their values. */ -void git_all_attrs (const char *path,struct attr_check *check); +void git_all_attrs (const struct index_state *istate, +const char *path,struct attr_check *check);enum git_attr_direction {GIT_ATTR_CHECKIN ,
@@ -74,7 +78,7 @@ enum git_attr_direction { GIT_ATTR_INDEX };void git_attr_set_direction (enum git_attr_direction new_direction, -struct index_state *istate); +const struct index_state *istate);void attr_start (void );
builtin/check-attr.c +2 -2
@@ -63,9 +63,9 @@ static void check_attr(const char *prefix,prefix_path (prefix, prefix ?strlen (prefix) :0 , file);if (collect_all) { -git_all_attrs (full_path, check); +git_all_attrs (&the_index, full_path, check); }else { -if (git_check_attr (full_path, check)) +if (git_check_attr (&the_index, full_path, check))die ("git_check_attr died" ); }output_attr (check, file);
builtin/pack-objects.c +1 -1
@@ -945,7 +945,7 @@ static int no_try_delta(const char *path)if (!check) check =attr_check_initl ("delta" ,NULL ); -if (git_check_attr (path, check)) +if (git_check_attr (&the_index, path, check))return 0 ;if (ATTR_FALSE (check->items[0 ].value))return 1 ;
convert.c +1 -1
@@ -1303,7 +1303,7 @@ static void convert_attrs(struct conv_attrs *ca, const char *path)git_config (read_convert_config,NULL ); } -if (!git_check_attr (path, check)) { +if (!git_check_attr (&the_index, path, check)) {struct attr_check_item *ccheck = check->items; ca->crlf_action =git_path_check_crlf (ccheck +4 );if (ca->crlf_action ==CRLF_UNDEFINED )
dir.c +1 -1
@@ -281,7 +281,7 @@ static int match_attrs(const char *name, int namelen, {int i; -git_check_attr (name, item->attr_check); +git_check_attr (&the_index, name, item->attr_check);for (i =0 ; i < item->attr_match_nr; i++) {const char *value;int matched;
ll-merge.c +2 -2
@@ -371,7 +371,7 @@ int ll_merge(mmbuffer_t *result_buf,if (!check) check =attr_check_initl ("merge" ,"conflict-marker-size" ,NULL ); -if (!git_check_attr (path, check)) { +if (!git_check_attr (&the_index, path, check)) { ll_driver_name = check->items[0 ].value;if (check->items[1 ].value) { marker_size =atoi (check->items[1 ].value);
@@ -398,7 +398,7 @@ int ll_merge_marker_size(const char *path)if (!check) check =attr_check_initl ("conflict-marker-size" ,NULL ); -if (!git_check_attr (path, check) && check->items[0 ].value) { +if (!git_check_attr (&the_index, path, check) && check->items[0 ].value) { marker_size =atoi (check->items[0 ].value);if (marker_size <=0 ) marker_size =DEFAULT_CONFLICT_MARKER_SIZE ;
userdiff.c +1 -1
@@ -278,7 +278,7 @@ struct userdiff_driver *userdiff_find_by_path(const char *path) check =attr_check_initl ("diff" ,NULL );if (!path)return NULL ; -if (git_check_attr (path, check)) +if (git_check_attr (&the_index, path, check))return NULL ;if (ATTR_TRUE (check->items[0 ].value))
ws.c +1 -1
@@ -78,7 +78,7 @@ unsigned whitespace_rule(const char *pathname)if (!attr_whitespace_rule) attr_whitespace_rule =attr_check_initl ("whitespace" ,NULL ); -if (!git_check_attr (pathname, attr_whitespace_rule)) { +if (!git_check_attr (&the_index, pathname, attr_whitespace_rule)) {const char *value; value = attr_whitespace_rule->items[0 ].value;
Signature · 1 line
2.18.0.1004.g6639190530
Re: [PATCH 03/24] attr: remove an implicit dependency on the_index
Quoted 15 lines · Nguyễn Thái Ngọc Duy
Make the attr API take an
index_stateinstead of assumingthe_indexin attr code. All call sites are converted blindly to keep the patch simple and retain current behavior. Individual call sites may receive further updates to use the right index instead ofthe_index.There is one ugly temporary workaround added in
attr.cthat needs some more explanation.Commit
c24f3abace(apply: file commited with CRLF should roundtrip diff and apply - 2017-08-19) forces oneconvert_to_git()call to NOT read the index at all. But what do you know, we read it anyway by falling back tothe_index. When "istate" fromconvert_to_gitis now propagated down toread_attr_from_array()we will hit segfault somewhere insideread_blob_data_from_index.
Quoted 6 lines
The right way of dealing with this is to kill "
use_index" variable and only follow "istate" but at this stage we are not ready for that:
while mostgit_attr_set_direction()calls just passesthe_indexto be assigned touse_index, unpack-trees passes a different one which is used byentry.ccode, which has no way to know what index to use if we deleteuse_index. So this has to be done later.
Yep, I remember back when I was doing some refactorings on the attr system trying to get rid of the whole "use_index" thing. At that point in time it wasn't feasible to do so, so i'm excited that it should be done soon! :D
Quoted 346 lines
Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>--- archive.c | 2 +- attr.c | 57 ++++++++++++++++++++++++++++-------------- attr.h | 10 +++++--- builtin/check-attr.c | 4 +-- builtin/pack-objects.c | 2 +- convert.c | 2 +- dir.c | 2 +- ll-merge.c | 4 +-- userdiff.c | 2 +- ws.c | 2 +- 10 files changed, 55 insertions(+), 32 deletions(-)archive.c +1
-1@@ -109,7 +109,7 @@ static const struct attr_check *get_archive_attrs(const char *path)static struct attr_check *check;if (!check) check =attr_check_initl ("export-ignore" ,"export-subst" ,NULL ); -return git_check_attr (path, check) ?NULL : check; +return git_check_attr (&the_index, path, check) ?NULL : check; }static int check_attr_export_ignore (const struct attr_check *check)attr.c +38
-19@@ -708,10 +708,10 @@ 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 struct index_state *use_index; +static const struct index_state *use_index;void git_attr_set_direction (enum git_attr_direction new_direction, -struct index_state *istate) +const struct index_state *istate) {if (is_bare_repository () && new_direction !=GIT_ATTR_INDEX )BUG ("non-INDEX attr direction in a bare repo" );@@ -743,13 +743,24 @@ static struct attr_stack *read_attr_from_file(const char *path, int macro_ok)return res; } -static struct attr_stack *read_attr_from_index (const char *path,int macro_ok) +static struct attr_stack *read_attr_from_index (const struct index_state *istate, +const char *path, +int macro_ok) {struct attr_stack *res;char *buf, *sp;int lineno =0 ; +const struct index_state *to_read_from; - buf =read_blob_data_from_index (use_index ? use_index : &the_index, path,NULL ); +/* +* 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) +return NULL ; + + buf =read_blob_data_from_index (to_read_from, path,NULL );if (!buf)return NULL ;@@ -768,15 +779,16 @@ static struct attr_stack *read_attr_from_index(const char *path, int macro_ok)return res; } -static struct attr_stack *read_attr (const char *path,int macro_ok) +static struct attr_stack *read_attr (const struct index_state *istate, +const char *path,int macro_ok) {struct attr_stack *res =NULL ;if (direction ==GIT_ATTR_INDEX ) { - res =read_attr_from_index (path, macro_ok); + res =read_attr_from_index (istate, path, macro_ok); }else if (!is_bare_repository ()) {if (direction ==GIT_ATTR_CHECKOUT ) { - res =read_attr_from_index (path, macro_ok); + res =read_attr_from_index (istate, path, macro_ok);if (!res) res =read_attr_from_file (path, macro_ok); }else if (direction ==GIT_ATTR_CHECKIN ) {@@ -788,7 +800,7 @@ static struct attr_stack *read_attr(const char *path, int macro_ok)* We allow operation in a sparsely checked out * work tree, so read from it. */ - res =read_attr_from_index (path, macro_ok); + res =read_attr_from_index (istate, path, macro_ok); } }@@ -859,7 +871,8 @@ static void push_stack(struct attr_stack **attr_stack_p, } } -static void bootstrap_attr_stack (struct attr_stack **stack) +static void bootstrap_attr_stack (const struct index_state *istate, +struct attr_stack **stack) {struct attr_stack *e;@@ -883,7 +896,7 @@ static void bootstrap_attr_stack(struct attr_stack **stack) }/* root directory */ - e =read_attr (GITATTRIBUTES_FILE ,1 ); + e =read_attr (istate,GITATTRIBUTES_FILE ,1 );push_stack (stack, e,xstrdup ("" ),0 );/* info frame */ @@ -896,7 +909,8 @@ static void bootstrap_attr_stack(struct attr_stack **stack)push_stack (stack, e,NULL ,0 ); } -static void prepare_attr_stack (const char *path,int dirlen, +static void prepare_attr_stack (const struct index_state *istate, +const char *path,int dirlen,struct attr_stack **stack) {struct attr_stack *info;@@ -917,7 +931,7 @@ static void prepare_attr_stack(const char *path, int dirlen,* .gitattributes in deeper directories to shallower ones, * and finally use the built-in set as the default. */ -bootstrap_attr_stack (stack); +bootstrap_attr_stack (istate, stack);/* * Pop the "info" one that is always at the top of the stack. @@ -973,7 +987,7 @@ static void prepare_attr_stack(const char *path, int dirlen,strbuf_add (&pathbuf, path + pathbuf.len, (len - pathbuf.len));strbuf_addf (&pathbuf,"/%s" ,GITATTRIBUTES_FILE ); - next =read_attr (pathbuf.buf,0 ); + next =read_attr (istate, pathbuf.buf,0 );/* reset the pathbuf to not include "/.gitattributes" */ strbuf_setlen (&pathbuf, len);@@ -1095,7 +1109,9 @@ static void determine_macros(struct all_attrs_item *all_attrs,* If check->check_nr is non-zero, only attributes in check[] are collected. * Otherwise all attributes are collected. */ -static void collect_some_attrs (const char *path,struct attr_check *check) +static void collect_some_attrs (const struct index_state *istate, +const char *path, +struct attr_check *check) {int i, pathlen, rem, dirlen;const char *cp, *last_slash =NULL ;@@ -1114,7 +1130,7 @@ static void collect_some_attrs(const char *path, struct attr_check *check) dirlen =0 ; } -prepare_attr_stack (path, dirlen, &check->stack); +prepare_attr_stack (istate, path, dirlen, &check->stack);all_attrs_init (&g_attr_hashmap, check);determine_macros (check->all_attrs, check->stack);@@ -1136,11 +1152,13 @@ static void collect_some_attrs(const char *path, struct attr_check *check)fill (path, pathlen, basename_offset, check->stack, check->all_attrs, rem); } -int git_check_attr (const char *path,struct attr_check *check) +int git_check_attr (const struct index_state *istate, +const char *path, +struct attr_check *check) {int i; -collect_some_attrs (path, check); +collect_some_attrs (istate, path, check);for (i =0 ; i < check->nr; i++) {size_t n = check->items[i].attr->attr_nr;@@ -1153,12 +1171,13 @@ int git_check_attr(const char *path, struct attr_check *check)return 0 ; } -void git_all_attrs (const char *path,struct attr_check *check) +void git_all_attrs (const struct index_state *istate, +const char *path,struct attr_check *check) {int i;attr_check_reset (check); -collect_some_attrs (path, check); +collect_some_attrs (istate, path, check);for (i =0 ; i < check->all_attrs_nr; i++) {const char *name = check->all_attrs[i].attr->name;attr.h +7
-3@@ -1,6 +1,8 @@#ifndef ATTR_H #define ATTR_H +struct index_state ; +/* An attribute is a pointer to this opaque structure */ struct git_attr ;@@ -60,13 +62,15 @@ void attr_check_free(struct attr_check *check);*/ const char *git_attr_name (const struct git_attr *); -int git_check_attr (const char *path,struct attr_check *check); +int git_check_attr (const struct index_state *istate, +const char *path,struct attr_check *check);/* * Retrieve all attributes that apply to the specified path. * check holds the attributes and their values. */ -void git_all_attrs (const char *path,struct attr_check *check); +void git_all_attrs (const struct index_state *istate, +const char *path,struct attr_check *check);enum git_attr_direction {GIT_ATTR_CHECKIN ,@@ -74,7 +78,7 @@ enum git_attr_direction { GIT_ATTR_INDEX };void git_attr_set_direction (enum git_attr_direction new_direction, -struct index_state *istate); +const struct index_state *istate);void attr_start (void );builtin/check-attr.c +2
-2@@ -63,9 +63,9 @@ static void check_attr(const char *prefix,prefix_path (prefix, prefix ?strlen (prefix) :0 , file);if (collect_all) { -git_all_attrs (full_path, check); +git_all_attrs (&the_index, full_path, check); }else { -if (git_check_attr (full_path, check)) +if (git_check_attr (&the_index, full_path, check))die ("git_check_attr died" ); }output_attr (check, file);builtin/pack-objects.c +1
-1@@ -945,7 +945,7 @@ static int no_try_delta(const char *path)if (!check) check =attr_check_initl ("delta" ,NULL ); -if (git_check_attr (path, check)) +if (git_check_attr (&the_index, path, check))return 0 ;if (ATTR_FALSE (check->items[0 ].value))return 1 ;convert.c +1
-1@@ -1303,7 +1303,7 @@ static void convert_attrs(struct conv_attrs *ca, const char *path)git_config (read_convert_config,NULL ); } -if (!git_check_attr (path, check)) { +if (!git_check_attr (&the_index, path, check)) {struct attr_check_item *ccheck = check->items; ca->crlf_action =git_path_check_crlf (ccheck +4 );if (ca->crlf_action ==CRLF_UNDEFINED )dir.c +1
-1@@ -281,7 +281,7 @@ static int match_attrs(const char *name, int namelen, {int i; -git_check_attr (name, item->attr_check); +git_check_attr (&the_index, name, item->attr_check);for (i =0 ; i < item->attr_match_nr; i++) {const char *value;int matched;ll-merge.c +2
-2@@ -371,7 +371,7 @@ int ll_merge(mmbuffer_t *result_buf,if (!check) check =attr_check_initl ("merge" ,"conflict-marker-size" ,NULL ); -if (!git_check_attr (path, check)) { +if (!git_check_attr (&the_index, path, check)) { ll_driver_name = check->items[0 ].value;if (check->items[1 ].value) { marker_size =atoi (check->items[1 ].value);@@ -398,7 +398,7 @@ int ll_merge_marker_size(const char *path)if (!check) check =attr_check_initl ("conflict-marker-size" ,NULL ); -if (!git_check_attr (path, check) && check->items[0 ].value) { +if (!git_check_attr (&the_index, path, check) && check->items[0 ].value) { marker_size =atoi (check->items[0 ].value);if (marker_size <=0 ) marker_size =DEFAULT_CONFLICT_MARKER_SIZE ;userdiff.c +1
-1@@ -278,7 +278,7 @@ struct userdiff_driver *userdiff_find_by_path(const char *path) check =attr_check_initl ("diff" ,NULL );if (!path)return NULL ; -if (git_check_attr (path, check)) +if (git_check_attr (&the_index, path, check))return NULL ;if (ATTR_TRUE (check->items[0 ].value))ws.c +1
-1@@ -78,7 +78,7 @@ unsigned whitespace_rule(const char *pathname)if (!attr_whitespace_rule) attr_whitespace_rule =attr_check_initl ("whitespace" ,NULL ); -if (!git_check_attr (pathname, attr_whitespace_rule)) { +if (!git_check_attr (&the_index, pathname, attr_whitespace_rule)) {const char *value; value = attr_whitespace_rule->items[0 ].value;Signature · 1 line
2.18.0.1004.g6639190530
Signature · 1 line
Brandon Williams
PATCH 4/24 convert.c: remove an implicit dependency on the_index
In reply to Nguyễn Thái Ngọc DuyMake the convert API take an index_state instead of assuming the_index in convert.c. All external call sites are converted blindly to keep the patch simple and retain current behavior. Individual call sites may receive further updates to use the right index instead of the_index.
--- apply.c | 2 +- archive.c | 2 +- builtin/cat-file.c | 2 +- builtin/ls-files.c | 2 +- convert.c | 41 ++++++++++++++++++++++++----------------- convert.h | 15 ++++++++++----- diff.c | 2 +- entry.c | 6 +++--- merge-recursive.c | 2 +- sha1-file.c | 4 ++-- 10 files changed, 45 insertions(+), 33 deletions(-)
apply.c +1 -1
@@ -4335,7 +4335,7 @@ static int try_create_file(const char *path, unsigned int mode, const char *buf,if (fd <0 )return 1 ; -if (convert_to_working_tree (path, buf, size, &nbuf)) { +if (convert_to_working_tree (&the_index, path, buf, size, &nbuf)) { size = nbuf.len; buf = nbuf.buf; }
archive.c +1 -1
@@ -79,7 +79,7 @@ void *object_file_to_archive(const struct archiver_args *args,size_t size =0 ;strbuf_attach (&buf, buffer, *sizep, *sizep +1 ); -convert_to_working_tree (path, buf.buf, buf.len, &buf); +convert_to_working_tree (&the_index, path, buf.buf, buf.len, &buf);if (commit)format_subst (commit, buf.buf, buf.len, &buf); buffer =strbuf_detach (&buf, &size);
builtin/cat-file.c +1 -1
@@ -39,7 +39,7 @@ static int filter_object(const char *path, unsigned mode, oid_to_hex(oid), path);if ((type ==OBJ_BLOB ) &&S_ISREG (mode)) {struct strbuf strbuf =STRBUF_INIT ; -if (convert_to_working_tree (path, *buf, *size, &strbuf)) { +if (convert_to_working_tree (&the_index, path, *buf, *size, &strbuf)) {free (*buf); *size = strbuf.len; *buf =strbuf_detach (&strbuf,NULL );
builtin/ls-files.c +1 -1
@@ -63,7 +63,7 @@ static void write_eolinfo(const struct index_state *istate,struct stat st;const char *i_txt ="" ;const char *w_txt ="" ; -const char *a_txt =get_convert_attr_ascii (path); +const char *a_txt =get_convert_attr_ascii (&the_index, path);if (ce &&S_ISREG (ce->ce_mode)) i_txt =get_cached_convert_stats_ascii (istate, ce->name);
convert.c +24 -17
122 lines not on this page · raw message
convert.h +10 -5
@@ -60,15 +60,18 @@ extern char *check_roundtrip_encoding;const char *get_cached_convert_stats_ascii (const struct index_state *istate,const char *path);const char *get_wt_convert_stats_ascii (const char *path); -const char *get_convert_attr_ascii (const char *path); +const char *get_convert_attr_ascii (const struct index_state *istate, +const char *path);/* returns 1 if *dst was used */ int convert_to_git (const struct index_state *istate,const char *path,const char *src,size_t len,struct strbuf *dst,int conv_flags); -int convert_to_working_tree (const char *path,const char *src, +int convert_to_working_tree (const struct index_state *istate, +const char *path,const char *src,size_t len,struct strbuf *dst); -int async_convert_to_working_tree (const char *path,const char *src, +int async_convert_to_working_tree (const struct index_state *istate, +const char *path,const char *src,size_t len,struct strbuf *dst,void *dco); int async_query_available_blobs(const char *cmd,
@@ -86,7 +89,8 @@ void convert_to_git_filter_fd(const struct index_state *istate, const char *path, int fd, struct strbuf *dst, int conv_flags); -int would_convert_to_git_filter_fd (const char *path); +int would_convert_to_git_filter_fd (const struct index_state *istate, +const char *path);/***************************************************************** *
@@ -96,7 +100,8 @@ int would_convert_to_git_filter_fd(const char *path);struct stream_filter ;/* opaque */ -struct stream_filter *get_stream_filter (const char *path, +struct stream_filter *get_stream_filter (const struct index_state *istate, +const char *path,const struct object_id *);void free_stream_filter (struct stream_filter *);int is_null_stream_filter (struct stream_filter *);
diff.c +1 -1
@@ -3893,7 +3893,7 @@ static void prep_temp_blob(const char *path, struct diff_tempfile *temp, temp->tempfile =mks_tempfile_ts (tempfile.buf,strlen (base) +1 );if (!temp->tempfile)die_errno ("unable to create temp-file" ); -if (convert_to_working_tree (path, +if (convert_to_working_tree (&the_index, path, (const char *)blob, (size_t )size, &buf)) { blob = buf.buf; size = buf.len;
entry.c +3 -3
@@ -266,7 +266,7 @@ static int write_entry(struct cache_entry *ce,const struct submodule *sub; if (ce_mode_s_ifmt == S_IFREG) { - struct stream_filter *filter = get_stream_filter(ce->name, + struct stream_filter *filter = get_stream_filter(&the_index, ce->name, &ce->oid); if (filter && !streaming_write_entry(ce, path, filter,
@@ -314,14 +314,14 @@ static int write_entry(struct cache_entry *ce,* Convert from git internal format to working tree format */ if (dco && dco->state !=CE_NO_DELAY ) { - ret =async_convert_to_working_tree (ce->name, new_blob, + ret =async_convert_to_working_tree (&the_index, ce->name, new_blob, size, &buf, dco);if (ret &&string_list_has_string (&dco->paths, ce->name)) {free (new_blob); goto delayed; } }else - ret =convert_to_working_tree (ce->name, new_blob, size, &buf); + ret =convert_to_working_tree (&the_index, ce->name, new_blob, size, &buf);if (ret) {free (new_blob);
merge-recursive.c +1 -1
@@ -966,7 +966,7 @@ static int update_file_flags(struct merge_options *o, }if (S_ISREG (mode)) {struct strbuf strbuf =STRBUF_INIT ; -if (convert_to_working_tree (path, buf, size, &strbuf)) { +if (convert_to_working_tree (&the_index, path, buf, size, &strbuf)) {free (buf); size = strbuf.len; buf =strbuf_detach (&strbuf,NULL );
sha1-file.c +2 -2
@@ -1860,7 +1860,7 @@ static int index_stream_convert_blob(struct object_id *oid, int fd,struct strbuf sbuf =STRBUF_INIT ;assert (path); -assert (would_convert_to_git_filter_fd (path)); +assert (would_convert_to_git_filter_fd (&the_index, path));convert_to_git_filter_fd (&the_index, path, fd, &sbuf,get_conv_flags (flags));
@@ -1950,7 +1950,7 @@ int index_fd(struct object_id *oid, int fd, struct stat *st,* Call xsize_t() only when needed to avoid potentially unnecessary * die() for large files. */ -if (type ==OBJ_BLOB && path &&would_convert_to_git_filter_fd (path)) +if (type ==OBJ_BLOB && path &&would_convert_to_git_filter_fd (&the_index, path)) ret =index_stream_convert_blob (oid, fd, path, flags);else if (!S_ISREG (st->st_mode)) ret =index_pipe (oid, fd, type, path, flags);
Signature · 1 line
2.18.0.1004.g6639190530
Re: [PATCH 04/24] convert.c: remove an implicit dependency on the_index
Quoted 8 lines · Nguyễn Thái Ngọc Duy
Make the convert API take an
index_stateinstead of assumingthe_indexinconvert.c. All external call sites are converted blindly to keep the patch simple and retain current behavior. Individual call sites may receive further updates to use the right index instead ofthe_index.Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>---
Nice progression from inside to outside, a bit by bit. Quite a pleasant read so far ;-)
PATCH 5/24 dir.c: remove an implicit dependency on the_index in pathspec code
In reply to Nguyễn Thái Ngọc DuyMake the match_patchspec API and friends take an index_state instead of assuming the_index in dir.c. All external call sites are converted blindly to keep the patch simple and retain current behavior. Individual call sites may receive further updates to use the right index instead of the_index.
--- archive.c | 2 +- builtin/add.c | 6 +++--- builtin/checkout.c | 2 +- builtin/clean.c | 2 +- builtin/commit.c | 2 +- builtin/grep.c | 6 +++--- builtin/ls-files.c | 6 +++--- builtin/rm.c | 2 +- builtin/submodule--helper.c | 2 +- builtin/update-index.c | 2 +- diff-lib.c | 4 ++-- dir.c | 27 ++++++++++++++++----------- dir.h | 16 ++++++++++------ pathspec.c | 2 +- preload-index.c | 2 +- read-cache.c | 2 +- rerere.c | 2 +- resolve-undo.c | 2 +- revision.c | 2 +- submodule.c | 2 +- wt-status.c | 6 +++--- 21 files changed, 54 insertions(+), 45 deletions(-)
archive.c +1 -1
@@ -313,7 +313,7 @@ static int reject_entry(const struct object_id *oid, struct strbuf *base,struct strbuf sb =STRBUF_INIT ;strbuf_addbuf (&sb, base);strbuf_addstr (&sb, filename); -if (!match_pathspec (context, sb.buf, sb.len,0 ,NULL ,1 )) +if (!match_pathspec (&the_index, context, sb.buf, sb.len,0 ,NULL ,1 )) ret =READ_TREE_RECURSIVE ;strbuf_release (&sb); }
builtin/add.c +3 -3
@@ -40,7 +40,7 @@ static void chmod_pathspec(struct pathspec *pathspec, char flip)for (i =0 ; i < active_nr; i++) {struct cache_entry *ce = active_cache[i]; -if (pathspec && !ce_path_match (ce, pathspec,NULL )) +if (pathspec && !ce_path_match (&the_index, ce, pathspec,NULL ))continue ;if (chmod_cache_entry (ce, flip) <0 )
@@ -135,7 +135,7 @@ static int renormalize_tracked_files(const struct pathspec *pathspec, int flags)continue ;/* do not touch unmerged paths */ if (!S_ISREG (ce->ce_mode) && !S_ISLNK (ce->ce_mode))continue ;/* do not touch non blobs */ -if (pathspec && !ce_path_match (ce, pathspec,NULL )) +if (pathspec && !ce_path_match (&the_index, ce, pathspec,NULL ))continue ; retval |=add_file_to_cache (ce->name, flags |HASH_RENORMALIZE ); }
@@ -155,7 +155,7 @@ static char *prune_directory(struct dir_struct *dir, struct pathspec *pathspec, i = dir->nr;while (--i >=0 ) {struct dir_entry *entry = *src++; -if (dir_path_match (entry, pathspec, prefix, seen)) +if (dir_path_match (&the_index, entry, pathspec, prefix, seen)) *dst++ = entry; } dir->nr = dst - dir->entries;
builtin/checkout.c +1 -1
@@ -318,7 +318,7 @@ static int checkout_paths(const struct checkout_opts *opts,* match_pathspec() for _all_ entries when * opts->source_tree != NULL. */ -if (ce_path_match (ce, &opts->pathspec, ps_matched)) +if (ce_path_match (&the_index, ce, &opts->pathspec, ps_matched)) ce->ce_flags |=CE_MATCHED ; }
builtin/clean.c +1 -1
@@ -976,7 +976,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)continue ;if (pathspec.nr) - matches =dir_path_match (ent, &pathspec,0 ,NULL ); + matches =dir_path_match (&the_index, ent, &pathspec,0 ,NULL );if (pathspec.nr && !matches)continue ;
builtin/commit.c +1 -1
@@ -251,7 +251,7 @@ static int list_paths(struct string_list *list, const char *with_tree,if (ce->ce_flags &CE_UPDATE )continue ; -if (!ce_path_match (ce, pattern, m)) +if (!ce_path_match (&the_index, ce, pattern, m))continue ; item =string_list_insert (list, ce->name);if (ce_skip_worktree (ce))
builtin/grep.c +3 -3
@@ -497,7 +497,7 @@ static int grep_cache(struct grep_opt *opt, struct repository *repo,strbuf_addstr (&name, ce->name);if (S_ISREG (ce->ce_mode) && -match_pathspec (pathspec, name.buf, name.len,0 ,NULL , +match_pathspec (&the_index, pathspec, name.buf, name.len,0 ,NULL ,S_ISDIR (ce->ce_mode) ||S_ISGITLINK (ce->ce_mode))) {/*
@@ -515,7 +515,7 @@ static int grep_cache(struct grep_opt *opt, struct repository *repo, hit |=grep_file (opt, name.buf); } }else if (recurse_submodules &&S_ISGITLINK (ce->ce_mode) && -submodule_path_match (pathspec, name.buf,NULL )) { +submodule_path_match (&the_index, pathspec, name.buf,NULL )) { hit |=grep_submodule (opt, repo, pathspec,NULL , ce->name, ce->name); }else {continue ;
@@ -679,7 +679,7 @@ static int grep_directory(struct grep_opt *opt, const struct pathspec *pathspec,fill_directory (&dir, &the_index, pathspec);for (i =0 ; i < dir.nr; i++) { -if (!dir_path_match (dir.entries[i], pathspec,0 ,NULL )) +if (!dir_path_match (&the_index, dir.entries[i], pathspec,0 ,NULL ))continue ; hit |=grep_file (opt, dir.entries[i]->name);if (hit && opt->status_only)
builtin/ls-files.c +3 -3
@@ -128,7 +128,7 @@ static void show_dir_entry(const char *tag, struct dir_entry *ent)if (len > ent->len)die ("git ls-files: internal error - directory entry not superset of prefix" ); -if (!dir_path_match (ent, &pathspec, len, ps_matched)) +if (!dir_path_match (&the_index, ent, &pathspec, len, ps_matched))return ;fputs (tag, stdout);
@@ -228,7 +228,7 @@ static void show_ce(struct repository *repo, struct dir_struct *dir,if (recurse_submodules &&S_ISGITLINK (ce->ce_mode) &&is_submodule_active (repo, ce->name)) {show_submodule (repo, dir, ce->name); - }else if (match_pathspec (&pathspec, fullname,strlen (fullname), + }else if (match_pathspec (&the_index, &pathspec, fullname,strlen (fullname), max_prefix_len, ps_matched,S_ISDIR (ce->ce_mode) ||S_ISGITLINK (ce->ce_mode))) {
@@ -264,7 +264,7 @@ static void show_ru_info(const struct index_state *istate) len =strlen (path);if (len < max_prefix_len)continue ;/* outside of the prefix */ -if (!match_pathspec (&pathspec, path, len, +if (!match_pathspec (&the_index, &pathspec, path, len, max_prefix_len, ps_matched,0 ))continue ;/* uninterested */ for (i =0 ; i <3 ; i++) {
builtin/rm.c +1 -1
@@ -278,7 +278,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)for (i =0 ; i < active_nr; i++) {const struct cache_entry *ce = active_cache[i]; -if (!ce_path_match (ce, &pathspec, seen)) +if (!ce_path_match (&the_index, ce, &pathspec, seen))continue ;ALLOC_GROW (list.entry, list.nr +1 , list.alloc); list.entry[list.nr].name =xstrdup (ce->name);
builtin/submodule--helper.c +1 -1
@@ -331,7 +331,7 @@ static int module_list_compute(int argc, const char **argv,for (i =0 ; i < active_nr; i++) {const struct cache_entry *ce = active_cache[i]; -if (!match_pathspec (pathspec, ce->name,ce_namelen (ce), +if (!match_pathspec (&the_index, pathspec, ce->name,ce_namelen (ce),0 , ps_matched,1 ) || !S_ISGITLINK (ce->ce_mode))continue ;
builtin/update-index.c +1 -1
@@ -748,7 +748,7 @@ static int do_reupdate(int ac, const char **av,int save_nr;char *path; -if (ce_stage (ce) || !ce_path_match (ce, &pathspec,NULL )) +if (ce_stage (ce) || !ce_path_match (&the_index, ce, &pathspec,NULL ))continue ;if (has_head) old = read_one_ent(NULL, &head_oid,
diff-lib.c +2 -2
@@ -109,7 +109,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)if (diff_can_quit_early (&revs->diffopt))break ; -if (!ce_path_match (ce, &revs->prune_data,NULL )) +if (!ce_path_match (&the_index, ce, &revs->prune_data,NULL ))continue ;if (ce_stage (ce)) {
@@ -474,7 +474,7 @@ static int oneway_diff(const struct cache_entry * const *src,if (tree == o->df_conflict_entry) tree =NULL ; -if (ce_path_match (idx ? idx : tree, &revs->prune_data,NULL )) { +if (ce_path_match (&the_index, idx ? idx : tree, &revs->prune_data,NULL )) {do_oneway_diff (o, idx, tree);if (diff_can_quit_early (&revs->diffopt)) { o->exiting_early =1 ;
dir.c +16 -11
@@ -276,12 +276,13 @@ static int do_read_blob(const struct object_id *oid, struct oid_stat *oid_stat,#define DO_MATCH_DIRECTORY (1<<1)#define DO_MATCH_SUBMODULE (1<<2) -static int match_attrs (const char *name,int namelen, +static int match_attrs (const struct index_state *istate, +const char *name,int namelen,const struct pathspec_item *item) {int i; -git_check_attr (&the_index, name, item->attr_check); +git_check_attr (istate, name, item->attr_check);for (i =0 ; i < item->attr_match_nr; i++) {const char *value;int matched;
@@ -318,7 +319,8 @@ static int match_attrs(const char *name, int namelen,* * It returns 0 when there is no match. */ -static int match_pathspec_item (const struct pathspec_item *item,int prefix, +static int match_pathspec_item (const struct index_state *istate, +const struct pathspec_item *item,int prefix,const char *name,int namelen,unsigned flags) {/* name/namelen has prefix cut off by caller */
@@ -358,7 +360,7 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix, strncmp(item->match, name - prefix, item->prefix))return 0 ; -if (item->attr_match_nr && !match_attrs (name, namelen, item)) +if (item->attr_match_nr && !match_attrs (istate, name, namelen, item))return 0 ;/* If the match was just the prefix, we matched */
@@ -426,7 +428,8 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix,* pathspec did not match any names, which could indicate that the * user mistyped the nth pathspec. */ -static int do_match_pathspec (const struct pathspec *ps, +static int do_match_pathspec (const struct index_state *istate, +const struct pathspec *ps,const char *name,int namelen,int prefix,char *seen,unsigned flags)
@@ -472,7 +475,7 @@ static int do_match_pathspec(const struct pathspec *ps,*/ if (seen && ps->items[i].magic &PATHSPEC_EXCLUDE ) seen[i] =MATCHED_FNMATCH ; - how =match_pathspec_item (ps->items+i, prefix, name, + how =match_pathspec_item (istate, ps->items+i, prefix, name, namelen, flags); if (ps->recursive && (ps->magic & PATHSPEC_MAXDEPTH) &&
@@ -496,17 +499,18 @@ static int do_match_pathspec(const struct pathspec *ps,return retval; } -int match_pathspec (const struct pathspec *ps, +int match_pathspec (const struct index_state *istate, +const struct pathspec *ps,const char *name,int namelen,int prefix,char *seen,int is_dir) {int positive, negative;unsigned flags = is_dir ?DO_MATCH_DIRECTORY :0 ; - positive =do_match_pathspec (ps, name, namelen, + positive =do_match_pathspec (istate, ps, name, namelen, prefix, seen, flags);if (!(ps->magic &PATHSPEC_EXCLUDE ) || !positive)return positive; - negative =do_match_pathspec (ps, name, namelen, + negative =do_match_pathspec (istate, ps, name, namelen, prefix, seen, flags |DO_MATCH_EXCLUDE );return negative ?0 : positive;
@@ -515,11 +519,12 @@ int match_pathspec(const struct pathspec *ps,/** * Check if a submodule is a superset of the pathspec */ -int submodule_path_match (const struct pathspec *ps, +int submodule_path_match (const struct index_state *istate, +const struct pathspec *ps,const char *submodule_name,char *seen) { - int matched = do_match_pathspec(ps, submodule_name, + int matched = do_match_pathspec(istate, ps, submodule_name, strlen(submodule_name), 0, seen, DO_MATCH_DIRECTORY |
dir.h +10 -6
@@ -216,7 +216,8 @@ extern int count_slashes(const char *s);extern int simple_length (const char *match);extern int no_wildcard (const char *string);extern char *common_prefix (const struct pathspec *pathspec); -extern int match_pathspec (const struct pathspec *pathspec, +extern int match_pathspec (const struct index_state *istate, +const struct pathspec *pathspec,const char *name,int namelen,int prefix,char *seen,int is_dir);extern int report_path_error (const char *ps_matched,const struct pathspec *pathspec,const char *prefix);
@@ -326,25 +327,28 @@ extern int git_fnmatch(const struct pathspec_item *item, const char *pattern, const char *string, int prefix); -extern int submodule_path_match (const struct pathspec *ps, +extern int submodule_path_match (const struct index_state *istate, +const struct pathspec *ps,const char *submodule_name,char *seen); -static inline int ce_path_match (const struct cache_entry *ce, +static inline int ce_path_match (const struct index_state *istate, +const struct cache_entry *ce,const struct pathspec *pathspec,char *seen) { -return match_pathspec (pathspec, ce->name,ce_namelen (ce),0 , seen, +return match_pathspec (istate, pathspec, ce->name,ce_namelen (ce),0 , seen,S_ISDIR (ce->ce_mode) ||S_ISGITLINK (ce->ce_mode)); } -static inline int dir_path_match (const struct dir_entry *ent, +static inline int dir_path_match (const struct index_state *istate, +const struct dir_entry *ent,const struct pathspec *pathspec,int prefix,char *seen) {int has_trailing_dir = ent->len && ent->name[ent->len -1 ] =='/' ;int len = has_trailing_dir ? ent->len -1 : ent->len; -return match_pathspec (pathspec, ent->name, len, prefix, seen, +return match_pathspec (istate, pathspec, ent->name, len, prefix, seen, has_trailing_dir); }
pathspec.c +1 -1
@@ -37,7 +37,7 @@ void add_pathspec_matches_against_index(const struct pathspec *pathspec,return ;for (i =0 ; i < istate->cache_nr; i++) {const struct cache_entry *ce = istate->cache[i]; -ce_path_match (ce, pathspec, seen); +ce_path_match (&the_index, ce, pathspec, seen); } }
preload-index.c +1 -1
@@ -58,7 +58,7 @@ static void *preload_thread(void *_data)continue ;if (ce->ce_flags &CE_FSMONITOR_VALID )continue ; -if (!ce_path_match (ce, &p->pathspec,NULL )) +if (!ce_path_match (&the_index, ce, &p->pathspec,NULL ))continue ;if (threaded_has_symlink_leading_path (&cache, ce->name,ce_namelen (ce)))continue ;
read-cache.c +1 -1
@@ -1493,7 +1493,7 @@ int refresh_index(struct index_state *istate, unsigned int flags,if (ignore_submodules &&S_ISGITLINK (ce->ce_mode))continue ; -if (pathspec && !ce_path_match (ce, pathspec, seen)) +if (pathspec && !ce_path_match (&the_index, ce, pathspec, seen)) filtered =1 ;if (ce_stage (ce)) {
rerere.c +1 -1
@@ -1120,7 +1120,7 @@ int rerere_forget(struct pathspec *pathspec)find_conflict (&conflict);for (i =0 ; i < conflict.nr; i++) {struct string_list_item *it = &conflict.items[i]; -if (!match_pathspec (pathspec, it->string, +if (!match_pathspec (&the_index, pathspec, it->string,strlen (it->string),0 ,NULL ,0 ))continue ;rerere_forget_one_path (it->string, &merge_rr);
resolve-undo.c +1 -1
@@ -188,7 +188,7 @@ void unmerge_index(struct index_state *istate, const struct pathspec *pathspec)for (i =0 ; i < istate->cache_nr; i++) {const struct cache_entry *ce = istate->cache[i]; -if (!ce_path_match (ce, pathspec,NULL )) +if (!ce_path_match (&the_index, ce, pathspec,NULL ))continue ; i =unmerge_index_entry_at (istate, i); }
revision.c +1 -1
@@ -1517,7 +1517,7 @@ static void prepare_show_merge(struct rev_info *revs)const struct cache_entry *ce = active_cache[i];if (!ce_stage (ce))continue ; -if (ce_path_match (ce, &revs->prune_data,NULL )) { +if (ce_path_match (&the_index, ce, &revs->prune_data,NULL )) { prune_num++;REALLOC_ARRAY (prune, prune_num); prune[prune_num-2 ] = ce->name;
submodule.c +1 -1
@@ -258,7 +258,7 @@ int is_submodule_active(struct repository *repo, const char *path) }parse_pathspec (&ps,0 ,0 ,NULL , args.argv); - ret =match_pathspec (&ps, path,strlen (path),0 ,NULL ,1 ); + ret =match_pathspec (&the_index, &ps, path,strlen (path),0 ,NULL ,1 );argv_array_clear (&args);clear_pathspec (&ps);
wt-status.c +3 -3
@@ -647,7 +647,7 @@ static void wt_status_collect_changes_initial(struct wt_status *s)struct wt_status_change_data *d;const struct cache_entry *ce = active_cache[i]; -if (!ce_path_match (ce, &s->pathspec,NULL )) +if (!ce_path_match (&the_index, ce, &s->pathspec,NULL ))continue ;if (ce_intent_to_add (ce))continue ;
@@ -703,7 +703,7 @@ static void wt_status_collect_untracked(struct wt_status *s)for (i =0 ; i < dir.nr; i++) {struct dir_entry *ent = dir.entries[i];if (cache_name_is_other (ent->name, ent->len) && -dir_path_match (ent, &s->pathspec,0 ,NULL )) +dir_path_match (&the_index, ent, &s->pathspec,0 ,NULL ))string_list_insert (&s->untracked, ent->name);free (ent); }
@@ -711,7 +711,7 @@ static void wt_status_collect_untracked(struct wt_status *s)for (i =0 ; i < dir.ignored_nr; i++) {struct dir_entry *ent = dir.ignored[i];if (cache_name_is_other (ent->name, ent->len) && -dir_path_match (ent, &s->pathspec,0 ,NULL )) +dir_path_match (&the_index, ent, &s->pathspec,0 ,NULL ))string_list_insert (&s->ignored, ent->name);free (ent); }
Signature · 1 line
2.18.0.1004.g6639190530
Re: [PATCH 05/24] dir.c: remove an implicit dependency on the_index in pathspec code
Quoted 31 lines · Nguyễn Thái Ngọc Duy
dir.c +5
-3@@ -276,12 +276,13 @@ static int do_read_blob(const struct object_id *oid, struct oid_stat *oid_stat,#define DO_MATCH_DIRECTORY (1<<1)#define DO_MATCH_SUBMODULE (1<<2) -static int match_attrs (const char *name,int namelen, +static int match_attrs (const struct index_state *istate, +const char *name,int namelen,const struct pathspec_item *item) {int i; -git_check_attr (&the_index, name, item->attr_check); +git_check_attr (istate, name, item->attr_check);for (i =0 ; i < item->attr_match_nr; i++) {const char *value;int matched;@@ -318,7 +319,8 @@ static int match_attrs(const char *name, int namelen,* * It returns 0 when there is no match. */ -static int match_pathspec_item (const struct pathspec_item *item,int prefix, +static int match_pathspec_item (const struct index_state *istate, +const struct pathspec_item *item,int prefix,const char *name,int namelen,unsigned flags) {/* name/namelen has prefix cut off by caller */
dir.c static int match_pathspec_item(const struct pathspec_item *item, int prefix, · line 364
Quoted 6 lines
dir.c +1
-1@@ -358,7 +360,7 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix, strncmp(item->match, name - prefix, item->prefix))return 0 ; -if (item->attr_match_nr && !match_attrs (name, namelen, item)) +if (item->attr_match_nr && !match_attrs (istate, name, namelen, item))return 0 ;
Yuck, all of this just because I added the ability to match against attrs with pathspecs. Part of me wonders if it would be better to put a pointer to the needed istate in the pathspec struct...but then I can think of a ton of reasons why that wouldn't be good either.
So yes I think this is probably the right approach, I'm just sorry I made it this messy :/
Signature · 1 line
Brandon Williams
Re: [PATCH 05/24] dir.c: remove an implicit dependency on the_index in pathspec code
Quoted 1 line · Brandon Williams
I'm just sorry I made it this messy :/
Take it with pride! I think all core devs can say "oh yes I've made big contributions. See there? I made that mess!" :D
Signature · 1 line
Duy
PATCH 6/24 preload-index.c: use the right index instead of the_index
In reply to Nguyễn Thái Ngọc Duy--- preload-index.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
preload-index.c +1 -1
@@ -58,7 +58,7 @@ static void *preload_thread(void *_data)continue ;if (ce->ce_flags &CE_FSMONITOR_VALID )continue ; -if (!ce_path_match (&the_index, ce, &p->pathspec,NULL )) +if (!ce_path_match (index, ce, &p->pathspec,NULL ))continue ;if (threaded_has_symlink_leading_path (&cache, ce->name,ce_namelen (ce)))continue ;
Signature · 1 line
2.18.0.1004.g6639190530
PATCH 7/24 ls-files: correct index argument to get_convert_attr_ascii()
In reply to Nguyễn Thái Ngọc Duywrite_eolinfo() does take an istate as function argument and it should be used instead of the_index.
--- builtin/ls-files.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)
builtin/ls-files.c +9 -8
@@ -63,7 +63,7 @@ static void write_eolinfo(const struct index_state *istate,struct stat st;const char *i_txt ="" ;const char *w_txt ="" ; -const char *a_txt =get_convert_attr_ascii (&the_index, path); +const char *a_txt =get_convert_attr_ascii (istate, path);if (ce &&S_ISREG (ce->ce_mode)) i_txt =get_cached_convert_stats_ascii (istate, ce->name);
@@ -121,18 +121,19 @@ static void print_debug(const struct cache_entry *ce) } } -static void show_dir_entry (const char *tag,struct dir_entry *ent) +static void show_dir_entry (const struct index_state *istate, +const char *tag,struct dir_entry *ent) {int len = max_prefix_len;if (len > ent->len)die ("git ls-files: internal error - directory entry not superset of prefix" ); -if (!dir_path_match (&the_index, ent, &pathspec, len, ps_matched)) +if (!dir_path_match (istate, ent, &pathspec, len, ps_matched))return ;fputs (tag, stdout); -write_eolinfo (NULL ,NULL , ent->name); +write_eolinfo (istate,NULL , ent->name);write_name (ent->name); }
@@ -145,7 +146,7 @@ static void show_other_files(const struct index_state *istate,struct dir_entry *ent = dir->entries[i];if (!index_name_is_other (istate, ent->name, ent->len))continue ; -show_dir_entry (tag_other, ent); +show_dir_entry (istate, tag_other, ent); } }
@@ -196,7 +197,7 @@ static void show_killed_files(const struct index_state *istate, } }if (killed) -show_dir_entry (tag_killed, dir->entries[i]); +show_dir_entry (istate, tag_killed, dir->entries[i]); } }
@@ -228,7 +229,7 @@ static void show_ce(struct repository *repo, struct dir_struct *dir,if (recurse_submodules &&S_ISGITLINK (ce->ce_mode) &&is_submodule_active (repo, ce->name)) {show_submodule (repo, dir, ce->name); - }else if (match_pathspec (&the_index, &pathspec, fullname,strlen (fullname), + }else if (match_pathspec (repo->index, &pathspec, fullname,strlen (fullname), max_prefix_len, ps_matched,S_ISDIR (ce->ce_mode) ||S_ISGITLINK (ce->ce_mode))) {
@@ -264,7 +265,7 @@ static void show_ru_info(const struct index_state *istate) len =strlen (path);if (len < max_prefix_len)continue ;/* outside of the prefix */ -if (!match_pathspec (&the_index, &pathspec, path, len, +if (!match_pathspec (istate, &pathspec, path, len, max_prefix_len, ps_matched,0 ))continue ;/* uninterested */ for (i =0 ; i <3 ; i++) {
Signature · 1 line
2.18.0.1004.g6639190530
Re: [PATCH 07/24] ls-files: correct index argument to get_convert_attr_ascii()
Quoted 14 lines · Nguyễn Thái Ngọc Duy
write_eolinfo()does take an istate as function argument and it should be used instead ofthe_index.Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>--- builtin/ls-files.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)builtin/ls-files.c +0
-0
builtin/ls-files.c static void write_eolinfo(const struct index_state *istate, · line 66
Quoted 5 lines
builtin/ls-files.c +1
-1@@ -63,7 +63,7 @@ static void write_eolinfo(const struct index_state *istate,struct stat st;const char *i_txt ="" ;const char *w_txt ="" ; -const char *a_txt =get_convert_attr_ascii (&the_index, path); +const char *a_txt =get_convert_attr_ascii (istate, path);
Going by the commit message this patch should end here?
Quoted 3 lines
builtin/ls-files.c +2
-1-static void show_dir_entry (const char *tag,struct dir_entry *ent) +static void show_dir_entry (const struct index_state *istate, +const char *tag,struct dir_entry *ent)
[...]
Quoted 2 lines
builtin/ls-files.c +1
-1-if (!dir_path_match (&the_index, ent, &pathspec, len, ps_matched)) +if (!dir_path_match (istate, ent, &pathspec, len, ps_matched))
[...]
builtin/ls-files.c static void print_debug(const struct cache_entry *ce) · line 136
Quoted 2 lines
builtin/ls-files.c +1
-1-write_eolinfo (NULL ,NULL , ent->name); +write_eolinfo (istate,NULL , ent->name);
but here we need to pass through the istate, which is why we adjust the dir_path_match while we're here
Quoted 2 lines
builtin/ls-files.c +1
-1-show_dir_entry (tag_other, ent); +show_dir_entry (istate, tag_other, ent);
[...]
builtin/ls-files.c static void show_killed_files(const struct index_state *istate, · line 200
Quoted 2 lines
builtin/ls-files.c +1
-1-show_dir_entry (tag_killed, dir->entries[i]); +show_dir_entry (istate, tag_killed, dir->entries[i]);
and having to adjust more callers here
builtin/ls-files.c static void show_ru_info(const struct index_state *istate) · line 266
Quoted 9 lines
builtin/ls-files.c +2
-2@@ -228,7 +229,7 @@ static void show_ce(struct repository *repo, struct dir_struct *dir, - } else if (match_pathspec(&the_index, &pathspec, fullname, strlen(fullname), + } else if (match_pathspec(repo->index, &pathspec, fullname, strlen(fullname),@@ -264,7 +265,7 @@ static void show_ru_info(const struct index_state *istate) - if (!match_pathspec(&the_index, &pathspec, path, len, + if (!match_pathspec(istate, &pathspec, path, len,
These seem more or less unrelated to the commit message or the code changes above. Maybe mention these as a "while at it" or separate them out in their own commit?
thanks,
Stefan
PATCH 8/24 unpack-trees: remove 'extern' on function declaration
In reply to Nguyễn Thái Ngọc Duy--- unpack-trees.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
unpack-trees.h +2 -2
@@ -82,8 +82,8 @@ struct unpack_trees_options {struct exclude_list *el;/* for internal use */ }; -extern int unpack_trees (unsigned n,struct tree_desc *t, -struct unpack_trees_options *options); +int unpack_trees (unsigned n,struct tree_desc *t, +struct unpack_trees_options *options);int verify_uptodate (const struct cache_entry *ce,struct unpack_trees_options *o);
Signature · 1 line
2.18.0.1004.g6639190530
Re: [PATCH 08/24] unpack-trees: remove 'extern' on function declaration
Quoted 2 lines · Nguyễn Thái Ngọc Duy
Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>
This removes the only existing extern keyword, which was added by Linus in 933bf40a5c6 (Start moving unpack-trees to "struct tree_desc", 2007-08-09). All other callers do not have this noise word as it was simply never
present there despite the old age of unpack-trees.h. Interesting history.
Thanks!
Stefan
Re: [PATCH 08/24] unpack-trees: remove 'extern' on function declaration
Quoted 10 lines · Stefan Beller
Quoted 2 lines · Nguyễn Thái Ngọc Duy
Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>This removes the only existing extern keyword, which was added by Linus in
933bf40a5c6(Start moving unpack-trees to "structtree_desc", 2007-08-09). All other callers do not have this noise word as it was simply never
present there despite the old age ofunpack-trees.h. Interesting history.
Linus did not add 'extern' though. It was Johannes a year ago in 16da134b1f (read-trees: refactor the unpack_trees() part - 2006-07-30). Man this function is _old_.
Signature · 1 line
Duy
Re: [PATCH 08/24] unpack-trees: remove 'extern' on function declaration
Quoted 13 lines · Duy Nguyen
Quoted 10 lines · Stefan Beller
Quoted 2 lines · Nguyễn Thái Ngọc Duy
Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>This removes the only existing extern keyword, which was added by Linus in
933bf40a5c6(Start moving unpack-trees to "structtree_desc", 2007-08-09). All other callers do not have this noise word as it was simply never
present there despite the old age ofunpack-trees.h. Interesting history.
Quoted 3 lines
Linus did not add 'extern' though. It was Johannes a year ago in
16da134b1f(read-trees: refactor theunpack_trees()part - 2006-07-30). Man this function is _old_.
Ah, yes. I stopped at the first blame here but dug down on other functions as I expected some of the recent "remove externs here" and magically overlook this function.
PATCH 9/24 unpack-trees: add a note about path invalidation
In reply to Nguyễn Thái Ngọc Duy--- unpack-trees.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
unpack-trees.c +11 -0
@@ -1552,6 +1552,17 @@ static int verify_uptodate_sparse(const struct cache_entry *ce,return verify_uptodate_1 (ce, o,ERROR_SPARSE_NOT_UPTODATE_FILE ); } +/* +* TODO: We should actually invalidate o->result, not src_index [1]. +* But since cache tree and untracked cache both are not copied to +* o->result until unpacking is complete, we invalidate them on +* src_index instead with the assumption that they will be copied to +* dst_index at the end. +* +* [1] src_index->cache_tree is also used in unpack_callback() so if +* we invalidate o->result, we need to update it to use +* o->result.cache_tree as well. +*/ static void invalidate_ce_path (const struct cache_entry *ce,struct unpack_trees_options *o) {
Signature · 1 line
2.18.0.1004.g6639190530
PATCH 10/24 unpack-trees: don't shadow global var the_index
In reply to Nguyễn Thái Ngọc DuyThis function mark_new_skip_worktree() has an argument named the_index which is also the name of a global variable. While they have different types (the global the_index is not a pointer) mistakes can easily happen and it's also confusing for readers. Rename the function argument to something other than the_index.
--- unpack-trees.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
unpack-trees.c +4 -5
@@ -1238,7 +1238,7 @@ static int clear_ce_flags(struct cache_entry **cache, int nr,* Set/Clear CE_NEW_SKIP_WORKTREE according to $GIT_DIR/info/sparse-checkout */ static void mark_new_skip_worktree (struct exclude_list *el, -struct index_state *the_index, +struct index_state *istate,int select_flag,int skip_wt_flag) {int i;
@@ -1247,8 +1247,8 @@ static void mark_new_skip_worktree(struct exclude_list *el,* 1. Pretend the narrowest worktree: only unmerged entries * are checked out */ -for (i =0 ; i < the_index->cache_nr; i++) { -struct cache_entry *ce = the_index->cache[i]; +for (i =0 ; i < istate->cache_nr; i++) { +struct cache_entry *ce = istate->cache[i];if (select_flag && !(ce->ce_flags & select_flag))continue ;
@@ -1263,8 +1263,7 @@ static void mark_new_skip_worktree(struct exclude_list *el,* 2. Widen worktree according to sparse-checkout file. * Matched entries will have skip_wt_flag cleared (i.e. "in") */ -clear_ce_flags (the_index->cache, the_index->cache_nr, - select_flag, skip_wt_flag, el); +clear_ce_flags (istate->cache, istate->cache_nr, select_flag, skip_wt_flag, el); } static int verify_absent(const struct cache_entry *,
Signature · 1 line
2.18.0.1004.g6639190530
PATCH 11/24 unpack-trees: convert clear_ce_flags* to avoid the_index
In reply to Nguyễn Thái Ngọc DuyPrior to fba92be8f7, this code implicitly (and incorrectly) assumes the_index when running the exclude machinery. fba92be8f7 helps show this problem clearer because unpack-trees operation is supposed to work on whatever index the caller specifies... not specifically the_index.
Update the code to use "istate" argument that's originally from mark_new_skip_worktree(). From the call sites, both in unpack_trees(), you can see that this function works on two separate indexes:o->src_index and o->result. The second mark_new_skip_worktree() so far has incorecctly applied exclude rules on o->src_index instead of o->result. It's unclear what is the consequences of this, but it's definitely wrong.
[1] fba92be8f7 (dir: convert is_excluded_from_list to take an index -
2017-05-05)
--- unpack-trees.c | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-)
unpack-trees.c +18 -13
@@ -1092,13 +1092,15 @@ static int unpack_callback(int n, unsigned long mask, unsigned long dirmask, strreturn mask; } -static int clear_ce_flags_1 (struct cache_entry **cache,int nr, +static int clear_ce_flags_1 (struct index_state *istate, +struct cache_entry **cache,int nr,struct strbuf *prefix,int select_mask,int clear_mask,struct exclude_list *el,int defval);/* Whole directory matching */ -static int clear_ce_flags_dir(struct cache_entry **cache, int nr, +static int clear_ce_flags_dir(struct index_state *istate, + struct cache_entry **cache, int nr, struct strbuf *prefix, char *basename, int select_mask, int clear_mask,
@@ -1107,7 +1109,7 @@ static int clear_ce_flags_dir(struct cache_entry **cache, int nr,struct cache_entry **cache_end;int dtype =DT_DIR ;int ret =is_excluded_from_list (prefix->buf, prefix->len, - basename, &dtype, el, &the_index); + basename, &dtype, el, istate);int rc;strbuf_addch (prefix,'/' );
@@ -1129,7 +1131,7 @@ static int clear_ce_flags_dir(struct cache_entry **cache, int nr,* calling clear_ce_flags_1(). That function will call * the expensive is_excluded_from_list() on every entry. */ - rc =clear_ce_flags_1 (cache, cache_end - cache, + rc =clear_ce_flags_1 (istate, cache, cache_end - cache, prefix, select_mask, clear_mask, el, ret);
@@ -1152,7 +1154,8 @@ static int clear_ce_flags_dir(struct cache_entry **cache, int nr,* cache[0]->name[0..(prefix_len-1)] * Top level path has prefix_len zero. */ -static int clear_ce_flags_1 (struct cache_entry **cache,int nr, +static int clear_ce_flags_1 (struct index_state *istate, +struct cache_entry **cache,int nr,struct strbuf *prefix,int select_mask,int clear_mask,struct exclude_list *el,int defval)
@@ -1186,7 +1189,7 @@ static int clear_ce_flags_1(struct cache_entry **cache, int nr, len = slash - name;strbuf_add (prefix, name, len); - processed = clear_ce_flags_dir(cache, cache_end - cache, + processed = clear_ce_flags_dir(istate, cache, cache_end - cache, prefix, prefix->buf + prefix->len - len, select_mask, clear_mask,
@@ -1200,7 +1203,7 @@ static int clear_ce_flags_1(struct cache_entry **cache, int nr, }strbuf_addch (prefix,'/' ); - cache +=clear_ce_flags_1 (cache, cache_end - cache, + cache +=clear_ce_flags_1 (istate, cache, cache_end - cache, prefix, select_mask, clear_mask, el, defval);strbuf_setlen (prefix, prefix->len - len -1 );
@@ -1210,7 +1213,7 @@ static int clear_ce_flags_1(struct cache_entry **cache, int nr,/* Non-directory */ dtype =ce_to_dtype (ce); ret =is_excluded_from_list (ce->name,ce_namelen (ce), - name, &dtype, el, &the_index); + name, &dtype, el, istate);if (ret <0 ) ret = defval;if (ret >0 )
@@ -1220,15 +1223,17 @@ static int clear_ce_flags_1(struct cache_entry **cache, int nr,return nr - (cache_end - cache); } -static int clear_ce_flags (struct cache_entry **cache,int nr, -int select_mask,int clear_mask, -struct exclude_list *el) +static int clear_ce_flags (struct index_state *istate, +int select_mask,int clear_mask, +struct exclude_list *el) {static struct strbuf prefix =STRBUF_INIT ;strbuf_reset (&prefix); -return clear_ce_flags_1 (cache, nr, +return clear_ce_flags_1 (istate, + istate->cache, + istate->cache_nr, &prefix, select_mask, clear_mask, el,0 );
@@ -1263,7 +1268,7 @@ static void mark_new_skip_worktree(struct exclude_list *el,* 2. Widen worktree according to sparse-checkout file. * Matched entries will have skip_wt_flag cleared (i.e. "in") */ -clear_ce_flags (istate->cache, istate->cache_nr, select_flag, skip_wt_flag, el); +clear_ce_flags (istate, select_flag, skip_wt_flag, el); } static int verify_absent(const struct cache_entry *,
Signature · 1 line
2.18.0.1004.g6639190530
PATCH 12/24 unpack-trees: avoid the_index in verify_absent()
In reply to Nguyễn Thái Ngọc DuyBoth functions that are updated in this commit are called by verify_absent(), which is part of the "unpack-trees" operation that is supposed to work on any index file specified by the caller. Thanks to Brandon [1] [2], an implicit dependency on the_index is exposed. This commit fixes it.
In both functions, it makes sense to use src_index to check for exclusion because it's almost unchanged and should give us the same outcome as if running the exclude check before the unpack.
It's "almost unchanged" because we do invalidate cache-tree and untracked cache in the source index. But this should not affect how exclude machinery uses the index: to see if a file is tracked, and to read a blob from the index instead of worktree if it's marked skip-worktree (i.e. it's not available in worktree)
[1] a0bba65b10 (dir: convert is_excluded to take an index - 2017-05-05
[2] 2c1eb10454 (dir: convert read_directory to take an index - 2017-05-05)
--- unpack-trees.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
unpack-trees.c +2 -2
@@ -1658,7 +1658,7 @@ static int verify_clean_subdirectory(const struct cache_entry *ce,memset (&d,0 ,sizeof (d));if (o->dir) d.exclude_per_dir = o->dir->exclude_per_dir; - i =read_directory (&d, &the_index, pathbuf, namelen+1 ,NULL ); + i =read_directory (&d, o->src_index, pathbuf, namelen+1 ,NULL );if (i)return o->gently ?-1 :add_rejected_path (o,ERROR_NOT_UPTODATE_DIR , ce->name);
@@ -1700,7 +1700,7 @@ static int check_ok_to_remove(const char *name, int len, int dtype,return 0 ;if (o->dir && -is_excluded (o->dir, &the_index, name, &dtype)) +is_excluded (o->dir, o->src_index, name, &dtype))/* * ce->name is explicitly excluded, so it is Ok to * overwrite it.
Signature · 1 line
2.18.0.1004.g6639190530
PATCH 13/24 pathspec.c: use the right index instead of the_index
In reply to Nguyễn Thái Ngọc Duy--- pathspec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
pathspec.c +1 -1
@@ -37,7 +37,7 @@ void add_pathspec_matches_against_index(const struct pathspec *pathspec,return ;for (i =0 ; i < istate->cache_nr; i++) {const struct cache_entry *ce = istate->cache[i]; -ce_path_match (&the_index, ce, pathspec, seen); +ce_path_match (istate, ce, pathspec, seen); } }
Signature · 1 line
2.18.0.1004.g6639190530
PATCH 14/24 submodule.c: use the right index instead of the_index
In reply to Nguyễn Thái Ngọc Duy--- submodule.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
submodule.c +4 -4
@@ -93,7 +93,7 @@ int update_path_in_gitmodules(const char *oldpath, const char *newpath)if (!file_exists (GITMODULES_FILE ))/* Do nothing without .gitmodules */ return -1 ; -if (is_gitmodules_unmerged (&the_index)) +if (is_gitmodules_unmerged (the_repository->index))die (_ ("Cannot change unmerged .gitmodules, resolve merge conflicts first" )); submodule =submodule_from_path (the_repository, &null_oid, oldpath);
@@ -127,7 +127,7 @@ int remove_path_from_gitmodules(const char *path)if (!file_exists (GITMODULES_FILE ))/* Do nothing without .gitmodules */ return -1 ; -if (is_gitmodules_unmerged (&the_index)) +if (is_gitmodules_unmerged (the_repository->index))die (_ ("Cannot change unmerged .gitmodules, resolve merge conflicts first" )); submodule =submodule_from_path (the_repository, &null_oid, path);
@@ -188,7 +188,7 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,if (ignore)handle_ignore_submodules_arg (diffopt, ignore); -else if (is_gitmodules_unmerged (&the_index)) +else if (is_gitmodules_unmerged (the_repository->index)) diffopt->flags.ignore_submodules =1 ; } }
@@ -258,7 +258,7 @@ int is_submodule_active(struct repository *repo, const char *path) }parse_pathspec (&ps,0 ,0 ,NULL , args.argv); - ret =match_pathspec (&the_index, &ps, path,strlen (path),0 ,NULL ,1 ); + ret =match_pathspec (repo->index, &ps, path,strlen (path),0 ,NULL ,1 );argv_array_clear (&args);clear_pathspec (&ps);
Signature · 1 line
2.18.0.1004.g6639190530
PATCH 15/24 entry.c: use the right index instead of the_index
In reply to Nguyễn Thái Ngọc Duycheckout-index.c needs update because if checkout->istate is NULL, ie_match_stat() will crash. Previously this is ie_match_stat(&the_index, ..) so it will not crash, but it is not technically correct either.
--- builtin/checkout-index.c | 1 + entry.c | 9 +++++---- 2 files changed, 6 insertions(+), 4 deletions(-)
builtin/checkout-index.c +1 -0
@@ -190,6 +190,7 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix) argc =parse_options (argc, argv, prefix, builtin_checkout_index_options, builtin_checkout_index_usage,0 ); + state.istate = &the_index; state.force = force; state.quiet = quiet; state.not_new = not_new;
entry.c +5 -4
@@ -266,7 +266,7 @@ static int write_entry(struct cache_entry *ce,const struct submodule *sub; if (ce_mode_s_ifmt == S_IFREG) { - struct stream_filter *filter = get_stream_filter(&the_index, ce->name, + struct stream_filter *filter = get_stream_filter(state->istate, ce->name, &ce->oid); if (filter && !streaming_write_entry(ce, path, filter,
@@ -314,14 +314,14 @@ static int write_entry(struct cache_entry *ce,* Convert from git internal format to working tree format */ if (dco && dco->state !=CE_NO_DELAY ) { - ret =async_convert_to_working_tree (&the_index, ce->name, new_blob, + ret =async_convert_to_working_tree (state->istate, ce->name, new_blob, size, &buf, dco);if (ret &&string_list_has_string (&dco->paths, ce->name)) {free (new_blob); goto delayed; } }else - ret =convert_to_working_tree (&the_index, ce->name, new_blob, size, &buf); + ret =convert_to_working_tree (state->istate, ce->name, new_blob, size, &buf);if (ret) {free (new_blob);
@@ -422,7 +422,8 @@ int checkout_entry(struct cache_entry *ce,if (!check_path (path.buf, path.len, &st, state->base_dir_len)) {const struct submodule *sub; -unsigned changed =ce_match_stat (ce, &st,CE_MATCH_IGNORE_VALID |CE_MATCH_IGNORE_SKIP_WORKTREE ); +unsigned changed =ie_match_stat (state->istate, ce, &st, +CE_MATCH_IGNORE_VALID |CE_MATCH_IGNORE_SKIP_WORKTREE );/* * Needs to be checked before !changed returns early, * as the possibly empty directory was not changed
Signature · 1 line
2.18.0.1004.g6639190530
PATCH 16/24 attr: remove index from git_attr_set_direction()
In reply to Nguyễn Thái Ngọc DuySince 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.
--- 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(-)
archive.c +1 -1
@@ -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,
attr.c +3 -12
@@ -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 ;
attr.h +1 -2
@@ -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 );
builtin/check-attr.c +1 -1
@@ -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++) {
unpack-trees.c +2 -2
@@ -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 ; }
Signature · 1 line
2.18.0.1004.g6639190530
Re: [PATCH 16/24] attr: remove index from git_attr_set_direction()
Quoted 11 lines · Nguyễn Thái Ngọc Duy
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_indexor NULL (which defaults back tothe_indexpreviously). There's only one suspicious call site inunpack-trees.cwhere 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.ccode to do crlf conversion if needed.entry.cnow respects struct checkout's istate field, and this field is correctly set inunpack-trees.c, there should be no regression from this change.
Thanks for fixing this! The API is now that much cleaner :)
Quoted 119 lines
Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>--- 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(-)archive.c +1
-1@@ -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,attr.c +3
-12@@ -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 ;attr.h +1
-2@@ -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 );builtin/check-attr.c +1
-1@@ -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++) {unpack-trees.c +2
-2@@ -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 ; }Signature · 1 line
2.18.0.1004.g6639190530
Signature · 1 line
Brandon Williams
PATCH 17/24 grep: use the right index instead of the_index
In reply to Nguyễn Thái Ngọc Duy--- builtin/grep.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
builtin/grep.c +2 -2
@@ -497,7 +497,7 @@ static int grep_cache(struct grep_opt *opt, struct repository *repo,strbuf_addstr (&name, ce->name);if (S_ISREG (ce->ce_mode) && -match_pathspec (&the_index, pathspec, name.buf, name.len,0 ,NULL , +match_pathspec (repo->index, pathspec, name.buf, name.len,0 ,NULL ,S_ISDIR (ce->ce_mode) ||S_ISGITLINK (ce->ce_mode))) {/*
@@ -515,7 +515,7 @@ static int grep_cache(struct grep_opt *opt, struct repository *repo, hit |=grep_file (opt, name.buf); } }else if (recurse_submodules &&S_ISGITLINK (ce->ce_mode) && -submodule_path_match (&the_index, pathspec, name.buf,NULL )) { +submodule_path_match (repo->index, pathspec, name.buf,NULL )) { hit |=grep_submodule (opt, repo, pathspec,NULL , ce->name, ce->name); }else {continue ;
Signature · 1 line
2.18.0.1004.g6639190530
PATCH 18/24 archive.c: avoid access to the_index
In reply to Nguyễn Thái Ngọc Duy--- archive.c | 45 ++++++++++++++++++++++++++-------------- archive.h | 16 +++++++++++--- builtin/archive.c | 2 +- builtin/upload-archive.c | 3 ++- 4 files changed, 45 insertions(+), 21 deletions(-)
archive.c +29 -16
@@ -79,7 +79,7 @@ void *object_file_to_archive(const struct archiver_args *args,size_t size =0 ;strbuf_attach (&buf, buffer, *sizep, *sizep +1 ); -convert_to_working_tree (&the_index, path, buf.buf, buf.len, &buf); +convert_to_working_tree (args->repo->index, path, buf.buf, buf.len, &buf);if (commit)format_subst (commit, buf.buf, buf.len, &buf); buffer =strbuf_detach (&buf, &size);
@@ -104,12 +104,13 @@ struct archiver_context {struct directory *bottom; }; -static const struct attr_check *get_archive_attrs (const char *path) +static const struct attr_check *get_archive_attrs (struct index_state *istate, +const char *path) {static struct attr_check *check;if (!check) check =attr_check_initl ("export-ignore" ,"export-subst" ,NULL ); -return git_check_attr (&the_index, path, check) ?NULL : check; +return git_check_attr (istate, path, check) ?NULL : check; }static int check_attr_export_ignore (const struct attr_check *check)
@@ -145,7 +146,7 @@ static int write_archive_entry(const struct object_id *oid, const char *base,if (!S_ISDIR (mode)) {const struct attr_check *check; - check =get_archive_attrs (path_without_prefix); + check =get_archive_attrs (args->repo->index, path_without_prefix);if (check_attr_export_ignore (check))return 0 ; args->convert =check_attr_export_subst (check);
@@ -220,7 +221,7 @@ static int queue_or_write_archive_entry(const struct object_id *oid,/* Borrow base, but restore its original value when done. */ strbuf_addstr (base, filename);strbuf_addch (base,'/' ); - check =get_archive_attrs (base->buf); + check =get_archive_attrs (c->args->repo->index, base->buf);strbuf_setlen (base, baselen);if (check_attr_export_ignore (check))
@@ -268,8 +269,8 @@ int write_archive_entries(struct archiver_args *args,memset (&opts,0 ,sizeof (opts)); opts.index_only =1 ; opts.head_idx =-1 ; - opts.src_index = &the_index; - opts.dst_index = &the_index; + opts.src_index = args->repo->index; + opts.dst_index = args->repo->index; opts.fn = oneway_merge;init_tree_desc (&t, args->tree->buffer, args->tree->size);if (unpack_trees (1 , &t, &opts))
@@ -304,33 +305,43 @@ static const struct archiver *lookup_archiver(const char *name)return NULL ; } +struct path_exists_context { +struct pathspec pathspec; +struct archiver_args *args; +}; +static int reject_entry (const struct object_id *oid,struct strbuf *base,const char *filename,unsigned mode,int stage,void *context) {int ret =-1 ; +struct path_exists_context *ctx = context; +if (S_ISDIR (mode)) {struct strbuf sb =STRBUF_INIT ;strbuf_addbuf (&sb, base);strbuf_addstr (&sb, filename); -if (!match_pathspec (&the_index, context, sb.buf, sb.len,0 ,NULL ,1 )) +if (!match_pathspec (ctx->args->repo->index, + &ctx->pathspec, + sb.buf, sb.len,0 ,NULL ,1 )) ret =READ_TREE_RECURSIVE ;strbuf_release (&sb); }return ret; } -static int path_exists (struct tree *tree,const char *path) +static int path_exists (struct archiver_args *args,const char *path) {const char *paths[] = { path,NULL }; -struct pathspec pathspec; +struct path_exists_context ctx;int ret; -parse_pathspec (&pathspec,0 ,0 ,"" , paths); - pathspec.recursive =1 ; - ret =read_tree_recursive (tree,"" ,0 ,0 , &pathspec, - reject_entry, &pathspec); -clear_pathspec (&pathspec); + ctx.args = args; +parse_pathspec (&ctx.pathspec,0 ,0 ,"" , paths); + ctx.pathspec.recursive =1 ; + ret =read_tree_recursive (args->tree,"" ,0 ,0 , &ctx.pathspec, + reject_entry, &ctx); +clear_pathspec (&ctx.pathspec);return ret !=0 ; }
@@ -348,7 +359,7 @@ static void parse_pathspec_arg(const char **pathspec, ar_args->pathspec.recursive =1 ;if (pathspec) {while (*pathspec) { -if (**pathspec && !path_exists (ar_args->tree, *pathspec)) +if (**pathspec && !path_exists (ar_args, *pathspec))die (_ ("pathspec '%s' did not match any files" ), *pathspec); pathspec++; }
@@ -510,6 +521,7 @@ static int parse_archive_args(int argc, const char **argv, }int write_archive (int argc,const char **argv,const char *prefix, +struct repository *repo,const char *name_hint,int remote) {const struct archiver *ar =NULL ;
@@ -521,6 +533,7 @@ int write_archive(int argc, const char **argv, const char *prefix,init_tar_archiver ();init_zip_archiver (); + args.repo = repo; argc =parse_archive_args (argc, argv, &ar, &args, name_hint, remote);if (!startup_info->have_repository) {/*
archive.h +13 -3
@@ -3,7 +3,10 @@#include "pathspec.h" +struct repository ; +struct archiver_args { +struct repository *repo;const char *base;size_t baselen;struct tree *tree;
@@ -17,6 +20,16 @@ struct archiver_args {int compression_level; }; +/* main api */ + +extern int write_archive (int argc,const char **argv,const char *prefix, +struct repository *repo, +const char *name_hint,int remote); + +const char *archive_format_from_filename (const char *filename); + +/* archive backend stuff */ +#define ARCHIVER_WANT_COMPRESSION_LEVELS 1#define ARCHIVER_REMOTE 2struct archiver {
@@ -36,9 +49,6 @@ typedef int (*write_archive_entry_fn_t)(struct archiver_args *args, unsigned int mode);extern int write_archive_entries (struct archiver_args *args,write_archive_entry_fn_t write_entry); -extern int write_archive (int argc,const char **argv,const char *prefix,const char *name_hint,int remote); - -const char *archive_format_from_filename (const char *filename); extern void *object_file_to_archive(const struct archiver_args *args, const char *path, const struct object_id *oid, unsigned int mode, enum object_type *type,
builtin/archive.c +1 -1
@@ -105,5 +105,5 @@ int cmd_archive(int argc, const char **argv, const char *prefix)setvbuf (stderr,NULL , _IOLBF,BUFSIZ ); -return write_archive (argc, argv, prefix, output,0 ); +return write_archive (argc, argv, prefix, the_repository, output,0 ); }
builtin/upload-archive.c +2 -1
@@ -43,7 +43,8 @@ int cmd_upload_archive_writer(int argc, const char **argv, const char *prefix) }/* parse all options sent by the client */ -return write_archive (sent_argv.argc, sent_argv.argv, prefix,NULL ,1 ); +return write_archive (sent_argv.argc, sent_argv.argv, prefix, + the_repository,NULL ,1 ); } __attribute__((format (printf, 1, 2)))
Signature · 1 line
2.18.0.1004.g6639190530
PATCH 19/24 archive-*.c: use the right repository
In reply to Nguyễn Thái Ngọc DuyWith 'struct archive_args' gaining new repository pointer, we don't have to assume the_repository in the archive backends anymore.
--- archive-tar.c | 2 +- archive-zip.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
archive-tar.c +1 -1
@@ -277,7 +277,7 @@ static int write_tar_entry(struct archiver_args *args,memcpy (header.name, path, pathlen);if (S_ISREG (mode) && !args->convert && -oid_object_info (the_repository, oid, &size) ==OBJ_BLOB && +oid_object_info (args->repo, oid, &size) ==OBJ_BLOB && size > big_file_threshold) buffer =NULL ;else if (S_ISLNK (mode) ||S_ISREG (mode)) {
archive-zip.c +1 -1
@@ -326,7 +326,7 @@ static int write_zip_entry(struct archiver_args *args, compressed_size =0 ; buffer =NULL ; }else if (S_ISREG (mode) ||S_ISLNK (mode)) { -enum object_type type =oid_object_info (the_repository, oid, +enum object_type type =oid_object_info (args->repo, oid, &size); method =0 ;
Signature · 1 line
2.18.0.1004.g6639190530
PATCH 20/24 resolve-undo.c: use the right index instead of the_index
In reply to Nguyễn Thái Ngọc Duy--- resolve-undo.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
resolve-undo.c +1 -1
@@ -188,7 +188,7 @@ void unmerge_index(struct index_state *istate, const struct pathspec *pathspec)for (i =0 ; i < istate->cache_nr; i++) {const struct cache_entry *ce = istate->cache[i]; -if (!ce_path_match (&the_index, ce, pathspec,NULL )) +if (!ce_path_match (istate, ce, pathspec,NULL ))continue ; i =unmerge_index_entry_at (istate, i); }
Signature · 1 line
2.18.0.1004.g6639190530
PATCH 21/24 apply.c: pass struct apply_state to more functions
In reply to Nguyễn Thái Ngọc Duywe're going to remove the dependency on the_index by moving 'struct index_state *' to somewhere inside struct apply_state. Let's make sure relevant functions have access to this struct now and reduce the diff noise when the actual conversion happens.
--- apply.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-)
apply.c +11 -7
@@ -3374,7 +3374,9 @@ static struct patch *previous_patch(struct apply_state *state,return previous; } -static int verify_index_match (const struct cache_entry *ce,struct stat *st) +static int verify_index_match (struct apply_state *state, +const struct cache_entry *ce, +struct stat *st) {if (S_ISGITLINK (ce->ce_mode)) {if (!S_ISDIR (st->st_mode))
@@ -3524,7 +3526,7 @@ static int load_current(struct apply_state *state,if (checkout_target (&the_index, ce, &st))return -1 ; } -if (verify_index_match (ce, &st)) +if (verify_index_match (state, ce, &st))return error (_ ("%s: does not match index" ), name); status =load_patch_target (state, &buf, ce, &st, patch, name, mode);
@@ -3694,7 +3696,7 @@ static int check_preimage(struct apply_state *state,if (checkout_target (&the_index, *ce, st))return -1 ; } -if (!state->cached &&verify_index_match (*ce, st)) +if (!state->cached &&verify_index_match (state, *ce, st))return error (_ ("%s: does not match index" ), old_name);if (state->cached) st_mode = (*ce)->ce_mode;
@@ -4313,7 +4315,9 @@ static int add_index_file(struct apply_state *state,* 0 if everything went well * 1 if a recoverable error happened */ -static int try_create_file (const char *path,unsigned int mode,const char *buf,unsigned long size) +static int try_create_file (struct apply_state *state,const char *path, +unsigned int mode,const char *buf, +unsigned long size) {int fd, res;struct strbuf nbuf =STRBUF_INIT ;
@@ -4371,7 +4375,7 @@ static int create_one_file(struct apply_state *state,if (state->cached)return 0 ; - res =try_create_file (path, mode, buf, size); + res =try_create_file (state, path, mode, buf, size);if (res <0 )return -1 ;if (!res)
@@ -4380,7 +4384,7 @@ static int create_one_file(struct apply_state *state,if (errno ==ENOENT ) {if (safe_create_leading_directories (path))return 0 ; - res =try_create_file (path, mode, buf, size); + res =try_create_file (state, path, mode, buf, size);if (res <0 )return -1 ;if (!res)
@@ -4402,7 +4406,7 @@ static int create_one_file(struct apply_state *state,for (;;) {char newpath[PATH_MAX ];mksnpath (newpath,sizeof (newpath),"%s~%u" , path, nr); - res =try_create_file (newpath, mode, buf, size); + res =try_create_file (state, newpath, mode, buf, size);if (res <0 )return -1 ;if (!res) {
Signature · 1 line
2.18.0.1004.g6639190530
PATCH 22/24 apply.c: make init_apply_state() take a struct repository
In reply to Nguyễn Thái Ngọc DuyWe're moving away from the_index in this code. "struct index_state *" could be added to struct apply_state. But let's aim long term and put struct repository here instead so that we could even avoid more global states in the future. The index will be available via apply_state->repo->index.
--- apply.c | 2 ++ apply.h | 4 ++++ builtin/am.c | 2 +- builtin/apply.c | 2 +- 4 files changed, 8 insertions(+), 2 deletions(-)
apply.c +2 -0
@@ -76,10 +76,12 @@ static int parse_ignorewhitespace_option(struct apply_state *state, }int init_apply_state (struct apply_state *state, +struct repository *repo,const char *prefix) {memset (state,0 ,sizeof (*state)); state->prefix = prefix; + state->repo = repo; state->apply =1 ; state->line_termination ='\n' ; state->p_value =1 ;
apply.h +4 -0
@@ -1,6 +1,8 @@#ifndef APPLY_H #define APPLY_H +struct repository ; +enum apply_ws_error_action { nowarn_ws_error, warn_on_ws_error,
@@ -62,6 +64,7 @@ struct apply_state {int unsafe_paths;/* Other non boolean parameters */ +struct repository *repo;const char *index_file;enum apply_verbosity apply_verbosity;const char *fake_ancestor;
@@ -116,6 +119,7 @@ int apply_parse_options(int argc, const char **argv, int *force_apply, int *options, const char * const *apply_usage);int init_apply_state (struct apply_state *state, +struct repository *repo,const char *prefix);void clear_apply_state (struct apply_state *state);int check_apply_state (struct apply_state *state,int force_apply);
builtin/am.c +1 -1
@@ -1464,7 +1464,7 @@ static int run_apply(const struct am_state *state, const char *index_file)int force_apply =0 ;int options =0 ; -if (init_apply_state (&apply_state,NULL )) +if (init_apply_state (&apply_state, the_repository,NULL ))BUG ("init_apply_state() failed" );argv_array_push (&apply_opts,"apply" );
builtin/apply.c +1 -1
@@ -16,7 +16,7 @@ int cmd_apply(int argc, const char **argv, const char *prefix)int ret;struct apply_state state; -if (init_apply_state (&state, prefix)) +if (init_apply_state (&state, the_repository, prefix))exit (128 ); argc = apply_parse_options(argc, argv,
Signature · 1 line
2.18.0.1004.g6639190530
PATCH 23/24 apply.c: remove implicit dependency on the_index
In reply to Nguyễn Thái Ngọc DuyUse apply_state->repo->index instead of the_index (in most cases, unless we need to use a temporary index in some functions). Let the callers (am and apply) tell us what to use, instead of always assuming to operate on the_index.
--- apply.c | 46 +++++++++++++++++++++++++--------------------- 1 file changed, 25 insertions(+), 21 deletions(-)
apply.c +25 -21
@@ -3385,7 +3385,8 @@ static int verify_index_match(struct apply_state *state,return -1 ;return 0 ; } -return ce_match_stat (ce, st,CE_MATCH_IGNORE_VALID |CE_MATCH_IGNORE_SKIP_WORKTREE ); +return ie_match_stat (state->repo->index, ce, st, +CE_MATCH_IGNORE_VALID |CE_MATCH_IGNORE_SKIP_WORKTREE ); }#define SUBMODULE_PATCH_WITHOUT_INDEX 1
@@ -3518,14 +3519,14 @@ static int load_current(struct apply_state *state,if (!patch->is_new)BUG ("patch to %s is not a creation" , patch->old_name); - pos =cache_name_pos (name,strlen (name)); + pos =index_name_pos (state->repo->index, name,strlen (name));if (pos <0 )return error (_ ("%s: does not exist in index" ), name); - ce = active_cache[pos]; + ce = state->repo->index->cache[pos];if (lstat (name, &st)) {if (errno !=ENOENT )return error_errno ("%s" , name); -if (checkout_target (&the_index, ce, &st)) +if (checkout_target (state->repo->index, ce, &st))return -1 ; }if (verify_index_match (state, ce, &st))
@@ -3687,15 +3688,16 @@ static int check_preimage(struct apply_state *state, }if (state->check_index && !previous) { -int pos =cache_name_pos (old_name,strlen (old_name)); +int pos =index_name_pos (state->repo->index, old_name, +strlen (old_name));if (pos <0 ) {if (patch->is_new <0 ) goto is_new;return error (_ ("%s: does not exist in index" ), old_name); } - *ce = active_cache[pos]; + *ce = state->repo->index->cache[pos];if (stat_ret <0 ) { -if (checkout_target (&the_index, *ce, st)) +if (checkout_target (state->repo->index, *ce, st))return -1 ; }if (!state->cached &&verify_index_match (state, *ce, st))
@@ -3742,7 +3744,7 @@ static int check_to_create(struct apply_state *state,struct stat nst;if (state->check_index && -cache_name_pos (new_name,strlen (new_name)) >=0 && +index_name_pos (state->repo->index, new_name,strlen (new_name)) >=0 && !ok_if_exists)return EXISTS_IN_INDEX ;if (state->cached)
@@ -3831,7 +3833,8 @@ static int path_is_beyond_symlink_1(struct apply_state *state, struct strbuf *naif (state->check_index) {struct cache_entry *ce; - ce =cache_file_exists (name->buf, name->len, ignore_case); + ce =index_file_exists (state->repo->index, name->buf, + name->len, ignore_case);if (ce &&S_ISLNK (ce->ce_mode))return 1 ; }else {
@@ -4006,9 +4009,10 @@ static int check_patch_list(struct apply_state *state, struct patch *patch)static int read_apply_cache (struct apply_state *state) {if (state->index_file) -return read_cache_from (state->index_file); +return read_index_from (state->repo->index, state->index_file, +get_git_dir ());else -return read_cache (); +return read_index (state->repo->index); }/* This function tries to read the object name from the current index */
@@ -4019,10 +4023,10 @@ static int get_current_oid(struct apply_state *state, const char *path,if (read_apply_cache (state) <0 )return -1 ; - pos =cache_name_pos (path,strlen (path)); + pos =index_name_pos (state->repo->index, path,strlen (path));if (pos <0 )return -1 ; -oidcpy (oid, &active_cache[pos]->oid); +oidcpy (oid, &state->repo->index->cache[pos]->oid);return 0 ; }
@@ -4250,7 +4254,7 @@ static void patch_stats(struct apply_state *state, struct patch *patch)static int remove_file (struct apply_state *state,struct patch *patch,int rmdir_empty) {if (state->update_index && !state->ita_only) { -if (remove_file_from_cache (patch->old_name) <0 ) +if (remove_file_from_index (state->repo->index, patch->old_name) <0 )return error (_ ("unable to remove %s from index" ), patch->old_name); }if (!state->cached) {
@@ -4271,7 +4275,7 @@ static int add_index_file(struct apply_state *state,struct cache_entry *ce;int namelen =strlen (path); - ce =make_empty_cache_entry (&the_index, namelen); + ce =make_empty_cache_entry (state->repo->index, namelen);memcpy (ce->name, path, namelen); ce->ce_mode =create_ce_mode (mode); ce->ce_flags =create_ce_flags (0 );
@@ -4303,7 +4307,7 @@ static int add_index_file(struct apply_state *state, "for newly created file %s"), path); } } -if (add_cache_entry (ce,ADD_CACHE_OK_TO_ADD ) <0 ) { +if (add_index_entry (state->repo->index, ce,ADD_CACHE_OK_TO_ADD ) <0 ) {discard_cache_entry (ce);return error (_ ("unable to add cache entry for %s" ), path); }
@@ -4341,7 +4345,7 @@ static int try_create_file(struct apply_state *state, const char *path,if (fd <0 )return 1 ; -if (convert_to_working_tree (&the_index, path, buf, size, &nbuf)) { +if (convert_to_working_tree (state->repo->index, path, buf, size, &nbuf)) { size = nbuf.len; buf = nbuf.buf; }
@@ -4438,17 +4442,17 @@ static int add_conflicted_stages_file(struct apply_state *state, namelen =strlen (patch->new_name); mode = patch->new_mode ? patch->new_mode : (S_IFREG |0644 ); -remove_file_from_cache (patch->new_name); +remove_file_from_index (state->repo->index, patch->new_name);for (stage =1 ; stage <4 ; stage++) {if (is_null_oid (&patch->threeway_stage[stage -1 ]))continue ; - ce =make_empty_cache_entry (&the_index, namelen); + ce =make_empty_cache_entry (state->repo->index, namelen);memcpy (ce->name, patch->new_name, namelen); ce->ce_mode =create_ce_mode (mode); ce->ce_flags =create_ce_flags (stage); ce->ce_namelen = namelen;oidcpy (&ce->oid, &patch->threeway_stage[stage -1 ]); -if (add_cache_entry (ce,ADD_CACHE_OK_TO_ADD ) <0 ) { +if (add_index_entry (state->repo->index, ce,ADD_CACHE_OK_TO_ADD ) <0 ) {discard_cache_entry (ce);return error (_ ("unable to add cache entry for %s" ), patch->new_name);
@@ -4897,7 +4901,7 @@ int apply_all_patches(struct apply_state *state, }if (state->update_index) { - res =write_locked_index (&the_index, &state->lock_file,COMMIT_LOCK ); + res =write_locked_index (state->repo->index, &state->lock_file,COMMIT_LOCK );if (res) {error (_ ("Unable to write new index file" )); res =-128 ;
Signature · 1 line
2.18.0.1004.g6639190530
PATCH 24/24 blame.c: remove implicit dependency on the_index
In reply to Nguyễn Thái Ngọc DuySide note, since we gain access to the right repository, we can stop rely on the_repository in this code as well.
--- blame.c | 52 +++++++++++++++++++++++++++++-------------------- blame.h | 1 + builtin/blame.c | 1 + 3 files changed, 33 insertions(+), 21 deletions(-)
blame.c +31 -21
@@ -90,7 +90,8 @@ static struct blame_origin *get_origin(struct commit *commit, const char *path) -static void verify_working_tree_path (struct commit *work_tree,const char *path) +static void verify_working_tree_path (struct repository *repo, +struct commit *work_tree,const char *path) {struct commit_list *parents;int pos;
@@ -101,15 +102,15 @@ static void verify_working_tree_path(struct commit *work_tree, const char *path)unsigned mode;if (!get_tree_entry (commit_oid, path, &blob_oid, &mode) && -oid_object_info (the_repository, &blob_oid,NULL ) ==OBJ_BLOB ) +oid_object_info (repo, &blob_oid,NULL ) ==OBJ_BLOB )return ; } - pos =cache_name_pos (path,strlen (path)); + pos =index_name_pos (repo->index, path,strlen (path));if (pos >=0 ) ;/* path is in the index */ -else if (-1 - pos < active_nr && - !strcmp (active_cache[-1 - pos]->name, path)) +else if (-1 - pos < repo->index->cache_nr && + !strcmp (repo->index->cache[-1 - pos]->name, path)) ;/* path is in the index, unmerged */ else die ("no such path '%s' in HEAD" , path);
@@ -165,7 +166,8 @@ static void set_commit_buffer_from_strbuf(struct commit *c, struct strbuf *sb)* Prepare a dummy commit that represents the work tree (or staged) item. * Note that annotating work tree item never works in the reverse. */ -static struct commit *fake_working_tree_commit (struct diff_options *opt, +static struct commit *fake_working_tree_commit (struct repository *repo, +struct diff_options *opt,const char *path,const char *contents_from) {
@@ -181,7 +183,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,unsigned mode;struct strbuf msg =STRBUF_INIT ; -read_cache (); +read_index (repo->index);time (&now); commit =alloc_commit_node (the_repository); commit->object.parsed =1 ;
@@ -193,7 +195,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt, parent_tail =append_parent (parent_tail, &head_oid);append_merge_parents (parent_tail); -verify_working_tree_path (commit, path); +verify_working_tree_path (repo, commit, path); origin =make_origin (commit, path);
@@ -251,7 +253,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,if (strbuf_read (&buf,0 ,0 ) <0 )die_errno ("failed to read from stdin" ); } -convert_to_git (&the_index, path, buf.buf, buf.len, &buf,0 ); +convert_to_git (repo->index, path, buf.buf, buf.len, &buf,0 ); origin->file.ptr = buf.buf; origin->file.size = buf.len;pretend_object_file (buf.buf, buf.len,OBJ_BLOB , &origin->blob_oid);
@@ -262,27 +264,28 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,* bits; we are not going to write this index out -- we just * want to run "diff-index --cached". */ -discard_cache (); -read_cache (); +discard_index (repo->index); +read_index (repo->index); len =strlen (path);if (!mode) { -int pos =cache_name_pos (path, len); +int pos =index_name_pos (repo->index, path, len);if (0 <= pos) - mode = active_cache[pos]->ce_mode; + mode = repo->index->cache[pos]->ce_mode;else /* Let's not bother reading from HEAD tree */ mode =S_IFREG |0644 ; } - ce =make_empty_cache_entry (&the_index, len); + ce =make_empty_cache_entry (repo->index, len);oidcpy (&ce->oid, &origin->blob_oid);memcpy (ce->name, path, len); ce->ce_flags =create_ce_flags (0 ); ce->ce_namelen = len; ce->ce_mode =create_ce_mode (mode); -add_cache_entry (ce,ADD_CACHE_OK_TO_ADD |ADD_CACHE_OK_TO_REPLACE ); +add_index_entry (repo->index, ce, +ADD_CACHE_OK_TO_ADD |ADD_CACHE_OK_TO_REPLACE ); -cache_tree_invalidate_path (&the_index, path); +cache_tree_invalidate_path (repo->index, path);return commit; }
@@ -519,13 +522,14 @@ static void queue_blames(struct blame_scoreboard *sb, struct blame_origin *porig* * This also fills origin->mode for corresponding tree path. */ -static int fill_blob_sha1_and_mode (struct blame_origin *origin) +static int fill_blob_sha1_and_mode (struct repository *repo, +struct blame_origin *origin) {if (!is_null_oid (&origin->blob_oid))return 0 ;if (get_tree_entry (&origin->commit->object.oid, origin->path, &origin->blob_oid, &origin->mode)) goto error_out; -if (oid_object_info (the_repository, &origin->blob_oid,NULL ) !=OBJ_BLOB ) +if (oid_object_info (repo, &origin->blob_oid,NULL ) !=OBJ_BLOB ) goto error_out;return 0 ; error_out:
@@ -1767,7 +1771,9 @@ void init_scoreboard(struct blame_scoreboard *sb) sb->copy_score =BLAME_DEFAULT_COPY_SCORE ; } -void setup_scoreboard (struct blame_scoreboard *sb,const char *path,struct blame_origin **orig) +void setup_scoreboard (struct blame_scoreboard *sb, +const char *path, +struct blame_origin **orig) {const char *final_commit_name =NULL ;struct blame_origin *o;
@@ -1779,6 +1785,9 @@ void setup_scoreboard(struct blame_scoreboard *sb, const char *path, struct blamif (sb->reverse && sb->contents_from)die (_ ("--contents and --reverse do not blend well." )); +if (!sb->repo) +BUG ("repo is NULL" ); +if (!sb->reverse) { sb->final =find_single_final (sb->revs, &final_commit_name); sb->commits.compare = compare_commits_by_commit_date;
@@ -1800,7 +1809,8 @@ void setup_scoreboard(struct blame_scoreboard *sb, const char *path, struct blam* or "--contents". */ setup_work_tree (); - sb->final =fake_working_tree_commit (&sb->revs->diffopt, + sb->final =fake_working_tree_commit (sb->repo, + &sb->revs->diffopt, path, sb->contents_from);add_pending_object (sb->revs, &(sb->final->object),":" ); }
@@ -1845,7 +1855,7 @@ void setup_scoreboard(struct blame_scoreboard *sb, const char *path, struct blam }else { o =get_origin (sb->final, path); -if (fill_blob_sha1_and_mode (o)) +if (fill_blob_sha1_and_mode (sb->repo, o))die (_ ("no such path %s in %s" ), path, final_commit_name); if (sb->revs->diffopt.flags.allow_textconv &&
blame.h +1 -0
@@ -102,6 +102,7 @@ struct blame_scoreboard {struct commit *final;/* Priority queue for commits with unassigned blame records */ struct prio_queue commits; +struct repository *repo;struct rev_info *revs;const char *path;
builtin/blame.c +1 -0
@@ -988,6 +988,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix) sb.revs = &revs; sb.contents_from = contents_from; sb.reverse = reverse; + sb.repo = the_repository;setup_scoreboard (&sb, path, &o); lno = sb.num_lines;
Signature · 1 line
2.18.0.1004.g6639190530
Quoted 8 lines · Nguyễn Thái Ngọc Duy
This is the third part of killing
the_index(at least outside builtin/). Part 1 [1] is dropped. Part 2 is nd/no-extern on 'pu'. This part is built on top of nd/no-extern.This series would actually break 'pu' because
builtin/stash.cuses three functions that are updated here. So we would need something like the following patch to make it build again.
Quoted 6 lines
I don't know if that adds too much work on Junio. If it does, I guess I'll hold this off for a while until
builtin/stash.cgets merged because reordering these patches, pushing the patches that breakstash.caway, really takes a lot of work.
I went through and found this to be a pleasant read and hopefully others agree with the approach this series took vs what your part 1 did so that we can get this change in.
Signature · 1 line
Brandon Williams
Quoted 14 lines · Brandon Williams
Quoted 13 lines · Nguyễn Thái Ngọc Duy
This is the third part of killing
the_index(at least outside builtin/). Part 1 [1] is dropped. Part 2 is nd/no-extern on 'pu'. This part is built on top of nd/no-extern.This series would actually break 'pu' because
builtin/stash.cuses three functions that are updated here. So we would need something like the following patch to make it build again.I don't know if that adds too much work on Junio. If it does, I guess I'll hold this off for a while until
builtin/stash.cgets merged because reordering these patches, pushing the patches that breakstash.caway, really takes a lot of work.
Quoted 5 lines
Quoted 1 line
I went through and found this to be a pleasant read and hopefully others agree with the approach this series took vs what your part 1 did so that we can get this change in.
Yeah, I've only finished my first pass (read: I didn't go through the patches with fine toothed comb, nor thought about interactions with other topics), but this round was quite a pleasnt read so far.
Quoted 19 lines · Junio C Hamano
Quoted 16 lines · Brandon Williams
Quoted 14 lines · Nguyễn Thái Ngọc Duy
This is the third part of killing
the_index(at least outside builtin/). Part 1 [1] is dropped. Part 2 is nd/no-extern on 'pu'. This part is built on top of nd/no-extern.This series would actually break 'pu' because
builtin/stash.cuses three functions that are updated here. So we would need something like the following patch to make it build again.I don't know if that adds too much work on Junio. If it does, I guess I'll hold this off for a while until
builtin/stash.cgets merged because reordering these patches, pushing the patches that breakstash.caway, really takes a lot of work.
Quoted 8 lines
Quoted 3 lines
I went through and found this to be a pleasant read and hopefully others agree with the approach this series took vs what your part 1 did so that we can get this change in.
Yeah, I've only finished my first pass (read: I didn't go through the patches with fine toothed comb, nor thought about interactions with other topics), but this round was quite a pleasnt read so far.
I went through this topic with a finer comb and just found nits, none of which I would deem a complete show stopper.