repository object
cover repository object
When I first started working on the git project I found it very difficult to understand parts of the code base because of the inherently global nature of our code. It also made working on submodules very difficult. Since we can only open up a single repository per process, you need to launch a child process in order to process a submodule. But you also need to be able to communicate other stateful information to the children processes so that the submodules know how best to format their output or match against a pathspec...it ends up feeling like layering on hack after hack. What I would really like to do, is to have the ability to have a repository object so that I can open a submodule in-process.
Before this becomes a reality for all commands, much of the library code would need to be refactored in order to work purely on handles instead of global state. As it turned out, ls-files is a pretty simple command and doesn't have *too* many dependencies. The biggest thing that needed to be changed was piping through an index into a couple library routines so that they don't inherently rely on 'the_index'. A few of these changes I've sent out and can be found at 'origin/bw/pathspec-sans-the-index' and 'origin/bw/dir-c-stops-relying-on-the-index' which this series is based on.
Patches 1-16 are refactorings to prepare either library code or ls-files itself to be ready to handle passing around an index struct. Patches 17-22 introduce a repository struct and change a couple of things about how submodule caches work (getting submodule information from .gitmodules). And Patch 23 converts ls-files to use a repository struct.
The most interesting part of the series is from 17-23. And 1-16 could be taken as is without the rest of the series.
This is still very much in a WIP state, though it does pass all tests. What I'm hoping for here is to get a discussion started about the feasibility of a change like this and hopefully to get the ball rolling. Is this a direction we want to move in? Is it worth the pain?
Thanks for taking the time to look at this and entertain my insane ideas :)
Brandon Williams (23):
convert: convert get_cached_convert_stats_ascii to take an index
convert: convert crlf_to_git to take an index
convert: convert convert_to_git_filter_fd to take an index
convert: convert convert_to_git to take an index
convert: convert renormalize_buffer to take an index
tree: convert read_tree to take an index parameter
ls-files: convert overlay_tree_on_cache to take an index
ls-files: convert write_eolinfo to take an index
ls-files: convert show_killed_files to take an index
ls-files: convert show_other_files to take an index
ls-files: convert show_ru_info to take an index
ls-files: convert ce_excluded to take an index
ls-files: convert prune_cache to take an index
ls-files: convert show_files to take an index
ls-files: factor out debug info into a function
ls-files: factor out tag calculation
repo: introduce new repository object
repo: add index_state to struct repo
repo: add per repo config
submodule-config: refactor to allow for multiple submodule_cache's
repo: add repo_read_gitmodules
submodule: add is_submodule_active
ls-files: use repository object
Makefile | 1 + apply.c | 2 +- builtin/blame.c | 2 +- builtin/commit.c | 3 +- builtin/ls-files.c | 348 ++++++++++++++++----------------- cache.h | 4 +- combine-diff.c | 2 +- config.c | 2 +- convert.c | 31 +-- convert.h | 19 +- diff.c | 6 +- dir.c | 2 +- git.c | 2 +- ll-merge.c | 2 +- merge-recursive.c | 4 +- repo.c | 112 +++++++++++ repo.h | 22 +++ sha1_file.c | 6 +- submodule-config.c | 40 +++- submodule-config.h | 10 + submodule.c | 51 +++++ submodule.h | 2 + t/t3007-ls-files-recurse-submodules.sh | 39 ++++ tree.c | 28 ++- tree.h | 3 +- 25 files changed, 513 insertions(+), 230 deletions(-) create mode 100644 repo.c create mode 100644 repo.h
Signature · 1 line
2.13.0.303.g4ebf302169-goog
PATCH 1/23 convert: convert get_cached_convert_stats_ascii to take an index
--- builtin/ls-files.c | 3 ++- convert.c | 5 +++-- convert.h | 5 ++++- 3 files changed, 9 insertions(+), 4 deletions(-)
builtin/ls-files.c +2 -1
@@ -63,7 +63,8 @@ static void write_eolinfo(const struct cache_entry *ce, const char *path)const char *w_txt ="" ;const char *a_txt =get_convert_attr_ascii (path);if (ce &&S_ISREG (ce->ce_mode)) - i_txt =get_cached_convert_stats_ascii (ce->name); + i_txt =get_cached_convert_stats_ascii (&the_index, + ce->name);if (!lstat (path, &st) &&S_ISREG (st.st_mode)) w_txt =get_wt_convert_stats_ascii (path);printf ("i/%-5s w/%-5s attr/%-17s\t" , i_txt, w_txt, a_txt);
convert.c +3 -2
@@ -133,11 +133,12 @@ static const char *gather_convert_stats_ascii(const char *data, unsigned long si } } -const char *get_cached_convert_stats_ascii (const char *path) +const char *get_cached_convert_stats_ascii (const struct index_state *istate, +const char *path) {const char *ret;unsigned long sz; -void *data =read_blob_data_from_cache (path, &sz); +void *data =read_blob_data_from_index (istate, path, &sz); ret =gather_convert_stats_ascii (data, sz);free (data);return ret;
convert.h +4 -1
@@ -4,6 +4,8 @@#ifndef CONVERT_H #define CONVERT_H +struct index_state ; +enum safe_crlf {SAFE_CRLF_FALSE =0 , SAFE_CRLF_FAIL = 1,
@@ -33,7 +35,8 @@ enum eol { };extern enum eol core_eol; -extern const char *get_cached_convert_stats_ascii (const char *path); +extern const char *get_cached_convert_stats_ascii (const struct index_state *istate, +const char *path);extern const char *get_wt_convert_stats_ascii (const char *path);extern const char *get_convert_attr_ascii (const char *path);
Signature · 1 line
2.13.0.303.g4ebf302169-goog
PATCH 2/23 convert: convert crlf_to_git to take an index
In reply to Brandon Williams--- convert.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
convert.c +8 -6
@@ -217,13 +217,13 @@ static void check_safe_crlf(const char *path, enum crlf_action crlf_action, } } -static int has_cr_in_index (const char *path) +static int has_cr_in_index (const struct index_state *istate,const char *path) {unsigned long sz;void *data;int has_cr; - data =read_blob_data_from_cache (path, &sz); + data =read_blob_data_from_index (istate, path, &sz);if (!data)return 0 ; has_cr =memchr (data,'\r' , sz) !=NULL ;
@@ -253,7 +253,8 @@ static int will_convert_lf_to_crlf(size_t len, struct text_stat *stats, } -static int crlf_to_git (const char *path,const char *src,size_t len, +static int crlf_to_git (const struct index_state *istate, +const char *path,const char *src,size_t len,struct strbuf *buf,enum crlf_action crlf_action,enum safe_crlf checksafe) {
@@ -285,7 +286,8 @@ static int crlf_to_git(const char *path, const char *src, size_t len,* unless we want to renormalize in a merge or * cherry-pick. */ -if ((checksafe !=SAFE_CRLF_RENORMALIZE ) &&has_cr_in_index (path)) +if ((checksafe !=SAFE_CRLF_RENORMALIZE ) && +has_cr_in_index (istate, path)) convert_crlf_into_lf =0 ; } if ((checksafe == SAFE_CRLF_WARN ||
@@ -1193,7 +1195,7 @@ int convert_to_git(const char *path, const char *src, size_t len, src = dst->buf; len = dst->len; } - ret |=crlf_to_git (path, src, len, dst, ca.crlf_action, checksafe); + ret |=crlf_to_git (&the_index, path, src, len, dst, ca.crlf_action, checksafe);if (ret && dst) { src = dst->buf; len = dst->len;
@@ -1213,7 +1215,7 @@ void convert_to_git_filter_fd(const char *path, int fd, struct strbuf *dst,if (!apply_filter (path,NULL ,0 , fd, dst, ca.drv,CAP_CLEAN ))die ("%s: clean filter '%s' failed" , path, ca.drv->name); -crlf_to_git (path, dst->buf, dst->len, dst, ca.crlf_action, checksafe); +crlf_to_git (&the_index, path, dst->buf, dst->len, dst, ca.crlf_action, checksafe);ident_to_git (path, dst->buf, dst->len, dst, ca.ident); }
Signature · 1 line
2.13.0.303.g4ebf302169-goog
PATCH 3/23 convert: convert convert_to_git_filter_fd to take an index
In reply to Brandon Williams--- convert.c | 5 +++-- convert.h | 3 ++- sha1_file.c | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-)
convert.c +3 -2
@@ -1203,7 +1203,8 @@ int convert_to_git(const char *path, const char *src, size_t len,return ret |ident_to_git (path, src, len, dst, ca.ident); } -void convert_to_git_filter_fd (const char *path,int fd,struct strbuf *dst, +void convert_to_git_filter_fd (const struct index_state *istate, +const char *path,int fd,struct strbuf *dst,enum safe_crlf checksafe) {struct conv_attrs ca;
@@ -1215,7 +1216,7 @@ void convert_to_git_filter_fd(const char *path, int fd, struct strbuf *dst,if (!apply_filter (path,NULL ,0 , fd, dst, ca.drv,CAP_CLEAN ))die ("%s: clean filter '%s' failed" , path, ca.drv->name); -crlf_to_git (&the_index, path, dst->buf, dst->len, dst, ca.crlf_action, checksafe); +crlf_to_git (istate, path, dst->buf, dst->len, dst, ca.crlf_action, checksafe);ident_to_git (path, dst->buf, dst->len, dst, ca.ident); }
convert.h +2 -1
@@ -52,7 +52,8 @@ static inline int would_convert_to_git(const char *path)return convert_to_git (path,NULL ,0 ,NULL ,0 ); }/* Precondition: would_convert_to_git_filter_fd(path) == true */ -extern void convert_to_git_filter_fd (const char *path,int fd, +extern void convert_to_git_filter_fd (const struct index_state *istate, +const char *path,int fd,struct strbuf *dst,enum safe_crlf checksafe);extern int would_convert_to_git_filter_fd (const char *path);
sha1_file.c +1 -1
@@ -3580,7 +3580,7 @@ static int index_stream_convert_blob(unsigned char *sha1, int fd,assert (path);assert (would_convert_to_git_filter_fd (path)); -convert_to_git_filter_fd (path, fd, &sbuf, +convert_to_git_filter_fd (&the_index, path, fd, &sbuf, write_object ? safe_crlf :SAFE_CRLF_FALSE );if (write_object)
Signature · 1 line
2.13.0.303.g4ebf302169-goog
PATCH 4/23 convert: convert convert_to_git to take an index
In reply to Brandon Williams--- apply.c | 2 +- builtin/blame.c | 2 +- combine-diff.c | 2 +- convert.c | 7 ++++--- convert.h | 8 +++++--- diff.c | 6 +++--- dir.c | 2 +- sha1_file.c | 4 ++-- 8 files changed, 18 insertions(+), 15 deletions(-)
apply.c +1 -1
@@ -2267,7 +2267,7 @@ static int read_old_data(struct stat *st, const char *path, struct strbuf *buf)case S_IFREG :if (strbuf_read_file (buf, path, st->st_size) != st->st_size)return error (_ ("unable to open or read %s" ), path); -convert_to_git (path, buf->buf, buf->len, buf,0 ); +convert_to_git (&the_index, path, buf->buf, buf->len, buf,0 );return 0 ;default :return -1 ;
builtin/blame.c +1 -1
@@ -2383,7 +2383,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 (path, buf.buf, buf.len, &buf,0 ); +convert_to_git (&the_index, path, buf.buf, buf.len, &buf,0 ); origin->file.ptr = buf.buf; origin->file.size = buf.len;pretend_sha1_file (buf.buf, buf.len,OBJ_BLOB , origin->blob_oid.hash);
combine-diff.c +1 -1
@@ -1053,7 +1053,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,if (is_file) {struct strbuf buf =STRBUF_INIT ; -if (convert_to_git (elem->path, result, len, &buf, safe_crlf)) { +if (convert_to_git (&the_index, elem->path, result, len, &buf, safe_crlf)) {free (result); result =strbuf_detach (&buf, &len); result_size = len;
convert.c +4 -3
@@ -1179,7 +1179,8 @@ const char *get_convert_attr_ascii(const char *path)return "" ; } -int convert_to_git (const char *path,const char *src,size_t len, +int convert_to_git (const struct index_state *istate, +const char *path,const char *src,size_t len,struct strbuf *dst,enum safe_crlf checksafe) {int ret =0 ;
@@ -1195,7 +1196,7 @@ int convert_to_git(const char *path, const char *src, size_t len, src = dst->buf; len = dst->len; } - ret |=crlf_to_git (&the_index, path, src, len, dst, ca.crlf_action, checksafe); + ret |=crlf_to_git (istate, path, src, len, dst, ca.crlf_action, checksafe);if (ret && dst) { src = dst->buf; len = dst->len;
@@ -1266,7 +1267,7 @@ int renormalize_buffer(const char *path, const char *src, size_t len, struct str src = dst->buf; len = dst->len; } -return ret |convert_to_git (path, src, len, dst,SAFE_CRLF_RENORMALIZE ); +return ret |convert_to_git (&the_index, path, src, len, dst,SAFE_CRLF_RENORMALIZE ); }/*****************************************************************
convert.h +5 -3
@@ -41,15 +41,17 @@ extern const char *get_wt_convert_stats_ascii(const char *path);extern const char *get_convert_attr_ascii (const char *path);/* returns 1 if *dst was used */ -extern int convert_to_git (const char *path,const char *src,size_t len, +extern int convert_to_git (const struct index_state *istate, +const char *path,const char *src,size_t len,struct strbuf *dst,enum safe_crlf checksafe);extern int convert_to_working_tree (const char *path,const char *src,size_t len,struct strbuf *dst);extern int renormalize_buffer (const char *path,const char *src,size_t len,struct strbuf *dst); -static inline int would_convert_to_git (const char *path) +static inline int would_convert_to_git (const struct index_state *istate, +const char *path) { -return convert_to_git (path,NULL ,0 ,NULL ,0 ); +return convert_to_git (istate, path,NULL ,0 ,NULL ,0 ); }/* Precondition: would_convert_to_git_filter_fd(path) == true */ extern void convert_to_git_filter_fd(const struct index_state *istate,
diff.c +3 -3
@@ -2755,7 +2755,7 @@ static int reuse_worktree_file(const char *name, const unsigned char *sha1, int* Similarly, if we'd have to convert the file contents anyway, that * makes the optimization not worthwhile. */ -if (!want_file &&would_convert_to_git (name)) +if (!want_file &&would_convert_to_git (&the_index, name))return 0 ; len =strlen (name);
@@ -2877,7 +2877,7 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)* point if the path requires us to run the content * conversion. */ -if (size_only && !would_convert_to_git (s->path)) +if (size_only && !would_convert_to_git (&the_index, s->path))return 0 ;/*
@@ -2904,7 +2904,7 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)/* * Convert from working tree format to canonical git format */ -if (convert_to_git (s->path, s->data, s->size, &buf, crlf_warn)) { +if (convert_to_git (&the_index, s->path, s->data, s->size, &buf, crlf_warn)) {size_t size =0 ;munmap (s->data, s->size); s->should_munmap =0 ;
dir.c +1 -1
@@ -795,7 +795,7 @@ static int add_excludes(const char *fname, const char *base, int baselen, (pos = index_name_pos(istate, fname, strlen(fname))) >= 0 && !ce_stage(istate->cache[pos]) && ce_uptodate(istate->cache[pos]) && - !would_convert_to_git(fname)) + !would_convert_to_git(istate, fname))hashcpy (sha1_stat->sha1, istate->cache[pos]->oid.hash); else
sha1_file.c +2 -2
@@ -3546,7 +3546,7 @@ static int index_mem(unsigned char *sha1, void *buf, size_t size,*/ if ((type ==OBJ_BLOB ) && path) {struct strbuf nbuf =STRBUF_INIT ; -if (convert_to_git (path, buf, size, &nbuf, +if (convert_to_git (&the_index, path, buf, size, &nbuf, write_object ? safe_crlf :SAFE_CRLF_FALSE )) { buf =strbuf_detach (&nbuf, &size); re_allocated =1 ;
@@ -3668,7 +3668,7 @@ int index_fd(unsigned char *sha1, int fd, struct stat *st,else if (!S_ISREG (st->st_mode)) ret =index_pipe (sha1, fd, type, path, flags);else if (st->st_size <= big_file_threshold || type !=OBJ_BLOB || - (path &&would_convert_to_git (path))) + (path &&would_convert_to_git (&the_index, path))) ret =index_core (sha1, fd,xsize_t (st->st_size), type, path, flags);else
Signature · 1 line
2.13.0.303.g4ebf302169-goog
PATCH 5/23 convert: convert renormalize_buffer to take an index
In reply to Brandon Williams--- convert.c | 6 ++++-- convert.h | 3 ++- ll-merge.c | 2 +- merge-recursive.c | 4 ++-- 4 files changed, 9 insertions(+), 6 deletions(-)
convert.c +4 -2
@@ -1,3 +1,4 @@ +#define NO_THE_INDEX_COMPATIBILITY_MACROS #include "cache.h" #include "attr.h" #include "run-command.h"
@@ -1260,14 +1261,15 @@ int convert_to_working_tree(const char *path, const char *src, size_t len, strucreturn convert_to_working_tree_internal (path, src, len, dst,0 ); } -int renormalize_buffer (const char *path,const char *src,size_t len,struct strbuf *dst) +int renormalize_buffer (const struct index_state *istate,const char *path, +const char *src,size_t len,struct strbuf *dst) {int ret =convert_to_working_tree_internal (path, src, len, dst,1 );if (ret) { src = dst->buf; len = dst->len; } -return ret |convert_to_git (&the_index, path, src, len, dst,SAFE_CRLF_RENORMALIZE ); +return ret |convert_to_git (istate, path, src, len, dst,SAFE_CRLF_RENORMALIZE ); }/*****************************************************************
convert.h +2 -1
@@ -46,7 +46,8 @@ extern int convert_to_git(const struct index_state *istate, struct strbuf *dst, enum safe_crlf checksafe);extern int convert_to_working_tree (const char *path,const char *src,size_t len,struct strbuf *dst); -extern int renormalize_buffer (const char *path,const char *src,size_t len, +extern int renormalize_buffer (const struct index_state *istate, +const char *path,const char *src,size_t len,struct strbuf *dst);static inline int would_convert_to_git (const struct index_state *istate,const char *path)
ll-merge.c +1 -1
@@ -339,7 +339,7 @@ static const struct ll_merge_driver *find_ll_merge_driver(const char *merge_attrstatic void normalize_file (mmfile_t *mm,const char *path) {struct strbuf strbuf =STRBUF_INIT ; -if (renormalize_buffer (path, mm->ptr, mm->size, &strbuf)) { +if (renormalize_buffer (&the_index, path, mm->ptr, mm->size, &strbuf)) {free (mm->ptr); mm->size = strbuf.len; mm->ptr =strbuf_detach (&strbuf,NULL );
merge-recursive.c +2 -2
@@ -1639,8 +1639,8 @@ static int blob_unchanged(struct merge_options *opt,* performed. Comparison can be skipped if both files are * unchanged since their sha1s have already been compared. */ -if (renormalize_buffer (path, o.buf, o.len, &o) | -renormalize_buffer (path, a.buf, a.len, &a)) +if (renormalize_buffer (&the_index, path, o.buf, o.len, &o) | +renormalize_buffer (&the_index, path, a.buf, a.len, &a)) ret = (o.len == a.len && !memcmp (o.buf, a.buf, o.len)); error_return:
Signature · 1 line
2.13.0.303.g4ebf302169-goog
PATCH 6/23 tree: convert read_tree to take an index parameter
In reply to Brandon Williams--- builtin/ls-files.c | 2 +- tree.c | 28 ++++++++++++++++++---------- tree.h | 3 ++- 3 files changed, 21 insertions(+), 12 deletions(-)
builtin/ls-files.c +1 -1
@@ -460,7 +460,7 @@ void overlay_tree_on_cache(const char *tree_name, const char *prefix) PATHSPEC_PREFER_CWD, prefix, matchbuf); }else memset (&pathspec,0 ,sizeof (pathspec)); -if (read_tree (tree,1 , &pathspec)) +if (read_tree (tree,1 , &pathspec, &the_index))die ("unable to read tree entries %s" , tree_name);for (i =0 ; i < active_nr; i++) {
tree.c +18 -10
@@ -1,3 +1,4 @@ +#define NO_THE_INDEX_COMPATIBILITY_MACROS #include "cache.h" #include "cache-tree.h" #include "tree.h"
@@ -8,7 +9,11 @@const char *tree_type ="tree" ; -static int read_one_entry_opt (const unsigned char *sha1,const char *base,int baselen,const char *pathname,unsigned mode,int stage,int opt) +static int read_one_entry_opt (struct index_state *istate, +const unsigned char *sha1, +const char *base,int baselen, +const char *pathname, +unsigned mode,int stage,int opt) {int len;unsigned int size;
@@ -27,14 +32,15 @@ static int read_one_entry_opt(const unsigned char *sha1, const char *base, int bmemcpy (ce->name, base, baselen);memcpy (ce->name + baselen, pathname, len+1 );hashcpy (ce->oid.hash, sha1); -return add_cache_entry (ce, opt); +return add_index_entry (istate, ce, opt); }static int read_one_entry (const unsigned char *sha1,struct strbuf *base,const char *pathname,unsigned mode,int stage,void *context) { -return read_one_entry_opt (sha1, base->buf, base->len, pathname, +struct index_state *istate = context; +return read_one_entry_opt (istate, sha1, base->buf, base->len, pathname, mode, stage,ADD_CACHE_OK_TO_ADD |ADD_CACHE_SKIP_DFCHECK ); }
@@ -47,7 +53,8 @@ static int read_one_entry_quick(const unsigned char *sha1, struct strbuf *base, const char *pathname, unsigned mode, int stage, void *context) { - return read_one_entry_opt(sha1, base->buf, base->len, pathname, + struct index_state *istate = context; +return read_one_entry_opt (istate, sha1, base->buf, base->len, pathname, mode, stage,ADD_CACHE_JUST_APPEND ); }
@@ -144,7 +151,8 @@ static int cmp_cache_name_compare(const void *a_, const void *b_) ce2->name, ce2->ce_namelen, ce_stage(ce2)); } -int read_tree (struct tree *tree,int stage,struct pathspec *match) +int read_tree (struct tree *tree,int stage,struct pathspec *match, +struct index_state *istate) {read_tree_fn_t fn =NULL ;int i, err;
@@ -164,23 +172,23 @@ int read_tree(struct tree *tree, int stage, struct pathspec *match)* do it the original slow way, otherwise, append and then * sort at the end. */ -for (i =0 ; !fn && i < active_nr; i++) { -const struct cache_entry *ce = active_cache[i]; +for (i =0 ; !fn && i < istate->cache_nr; i++) { +const struct cache_entry *ce = istate->cache[i];if (ce_stage (ce) == stage) fn = read_one_entry; }if (!fn) fn = read_one_entry_quick; - err =read_tree_recursive (tree,"" ,0 , stage, match, fn,NULL ); + err =read_tree_recursive (tree,"" ,0 , stage, match, fn, istate);if (fn == read_one_entry || err)return err;/* * Sort the cache entry -- we need to nuke the cache tree, though. */ -cache_tree_free (&active_cache_tree); -QSORT (active_cache, active_nr, cmp_cache_name_compare); +cache_tree_free (&istate->cache_tree); +QSORT (istate->cache, istate->cache_nr, cmp_cache_name_compare);return 0 ; }
tree.h +2 -1
@@ -34,6 +34,7 @@ extern int read_tree_recursive(struct tree *tree, int stage, const struct pathspec *pathspec, read_tree_fn_t fn, void *context); -extern int read_tree (struct tree *tree,int stage,struct pathspec *pathspec); +extern int read_tree (struct tree *tree,int stage,struct pathspec *pathspec, +struct index_state *istate);#endif /* TREE_H */
Signature · 1 line
2.13.0.303.g4ebf302169-goog
PATCH 7/23 ls-files: convert overlay_tree_on_cache to take an index
In reply to Brandon Williams--- builtin/commit.c | 3 ++- builtin/ls-files.c | 15 ++++++++------- cache.h | 3 ++- 3 files changed, 12 insertions(+), 9 deletions(-)
builtin/commit.c +2 -1
@@ -253,7 +253,8 @@ static int list_paths(struct string_list *list, const char *with_tree,if (with_tree) {char *max_prefix =common_prefix (pattern); -overlay_tree_on_cache (with_tree, max_prefix ? max_prefix : prefix); +overlay_tree_on_index (&the_index, with_tree, + max_prefix ? max_prefix : prefix);free (max_prefix); }
builtin/ls-files.c +8 -7
@@ -431,7 +431,8 @@ static int get_common_prefix_len(const char *common_prefix)* that were given from the command line. We are not * going to write this index out. */ -void overlay_tree_on_cache (const char *tree_name,const char *prefix) +void overlay_tree_on_index (struct index_state *istate, +const char *tree_name,const char *prefix) {struct tree *tree;unsigned char sha1[20 ];
@@ -446,8 +447,8 @@ void overlay_tree_on_cache(const char *tree_name, const char *prefix)die ("bad tree-ish %s" , tree_name);/* Hoist the unmerged entries up to stage #3 to make room */ -for (i =0 ; i < active_nr; i++) { -struct cache_entry *ce = active_cache[i]; +for (i =0 ; i < istate->cache_nr; i++) { +struct cache_entry *ce = istate->cache[i];if (!ce_stage (ce))continue ; ce->ce_flags |=CE_STAGEMASK ;
@@ -460,11 +461,11 @@ void overlay_tree_on_cache(const char *tree_name, const char *prefix) PATHSPEC_PREFER_CWD, prefix, matchbuf); }else memset (&pathspec,0 ,sizeof (pathspec)); -if (read_tree (tree,1 , &pathspec, &the_index)) +if (read_tree (tree,1 , &pathspec, istate))die ("unable to read tree entries %s" , tree_name); -for (i =0 ; i < active_nr; i++) { -struct cache_entry *ce = active_cache[i]; +for (i =0 ; i < istate->cache_nr; i++) { +struct cache_entry *ce = istate->cache[i];switch (ce_stage (ce)) {case 0 : last_stage0 = ce;
@@ -679,7 +680,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)*/ if (show_stage || show_unmerged)die ("ls-files --with-tree is incompatible with -s or -u" ); -overlay_tree_on_cache (with_tree, max_prefix); +overlay_tree_on_index (&the_index, with_tree, max_prefix); }show_files (&dir);if (show_resolve_undo)
cache.h +2 -1
@@ -2179,7 +2179,8 @@ extern int ws_blank_line(const char *line, int len, unsigned ws_rule);#define ws_tab_width (rule) ((rule) & WS_TAB_WIDTH_MASK)/* ls-files */ -void overlay_tree_on_cache (const char *tree_name,const char *prefix); +void overlay_tree_on_index (struct index_state *istate, +const char *tree_name,const char *prefix);char *alias_lookup (const char *alias);int split_cmdline (char *cmdline,const char ***argv);
Signature · 1 line
2.13.0.303.g4ebf302169-goog
PATCH 8/23 ls-files: convert write_eolinfo to take an index
In reply to Brandon Williams--- builtin/ls-files.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)
builtin/ls-files.c +6 -7
@@ -53,17 +53,16 @@ static const char *tag_modified = "";static const char *tag_skip_worktree ="" ;static const char *tag_resolve_undo ="" ; -static void write_eolinfo (const struct cache_entry *ce,const char *path) +static void write_eolinfo (const struct index_state *istate, +const struct cache_entry *ce,const char *path) { -if (!show_eol) -return ; -else { +if (show_eol) {struct stat st;const char *i_txt ="" ;const char *w_txt ="" ;const char *a_txt =get_convert_attr_ascii (path);if (ce &&S_ISREG (ce->ce_mode)) - i_txt =get_cached_convert_stats_ascii (&the_index, + i_txt =get_cached_convert_stats_ascii (istate, ce->name);if (!lstat (path, &st) &&S_ISREG (st.st_mode)) w_txt =get_wt_convert_stats_ascii (path);
@@ -105,7 +104,7 @@ static void show_dir_entry(const char *tag, struct dir_entry *ent)return ;fputs (tag, stdout); -write_eolinfo (NULL , ent->name); +write_eolinfo (NULL ,NULL , ent->name);write_name (ent->name); }
@@ -275,7 +274,7 @@ static void show_ce_entry(const char *tag, const struct cache_entry *ce)find_unique_abbrev (ce->oid.hash, abbrev), ce_stage(ce)); } -write_eolinfo (ce, ce->name); +write_eolinfo (&the_index, ce, ce->name);write_name (ce->name);if (debug_mode) {const struct stat_data *sd = &ce->ce_stat_data;
Signature · 1 line
2.13.0.303.g4ebf302169-goog
PATCH 9/23 ls-files: convert show_killed_files to take an index
In reply to Brandon Williams--- builtin/ls-files.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-)
builtin/ls-files.c +11 -10
@@ -120,7 +120,8 @@ static void show_other_files(struct dir_struct *dir) } } -static void show_killed_files (struct dir_struct *dir) +static void show_killed_files (const struct index_state *istate, +const struct dir_struct *dir) {int i;for (i =0 ; i < dir->nr; i++) {
@@ -134,29 +135,29 @@ static void show_killed_files(struct dir_struct *dir)/* If ent->name is prefix of an entry in the * cache, it will be killed. */ - pos =cache_name_pos (ent->name, ent->len); + pos =index_name_pos (istate, ent->name, ent->len);if (0 <= pos)die ("BUG: killed-file %.*s not found" , ent->len, ent->name); pos = -pos -1 ; -while (pos < active_nr && -ce_stage (active_cache[pos])) +while (pos < istate->cache_nr && +ce_stage (istate->cache[pos])) pos++;/* skip unmerged */ -if (active_nr <= pos) +if (istate->cache_nr <= pos)break ;/* pos points at a name immediately after * ent->name in the cache. Does it expect * ent->name to be a directory? */ - len =ce_namelen (active_cache[pos]); + len =ce_namelen (istate->cache[pos]);if ((ent->len < len) && - !strncmp (active_cache[pos]->name, + !strncmp (istate->cache[pos]->name, ent->name, ent->len) && - active_cache[pos]->name[ent->len] =='/' ) + istate->cache[pos]->name[ent->len] =='/' ) killed =1 ;break ; } -if (0 <=cache_name_pos (ent->name, sp - ent->name)) { +if (0 <=index_name_pos (istate, ent->name, sp - ent->name)) {/* If any of the leading directories in * ent->name is registered in the cache, * ent->name will be killed.
@@ -337,7 +338,7 @@ static void show_files(struct dir_struct *dir)if (show_others)show_other_files (dir);if (show_killed) -show_killed_files (dir); +show_killed_files (&the_index, dir); }if (show_cached || show_stage) {for (i =0 ; i < active_nr; i++) {
Signature · 1 line
2.13.0.303.g4ebf302169-goog
PATCH 10/23 ls-files: convert show_other_files to take an index
In reply to Brandon Williams--- builtin/ls-files.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
builtin/ls-files.c +4 -3
@@ -108,13 +108,14 @@ static void show_dir_entry(const char *tag, struct dir_entry *ent)write_name (ent->name); } -static void show_other_files (struct dir_struct *dir) +static void show_other_files (const struct index_state *istate, +const struct dir_struct *dir) {int i;for (i =0 ; i < dir->nr; i++) {struct dir_entry *ent = dir->entries[i]; -if (!cache_name_is_other (ent->name, ent->len)) +if (!index_name_is_other (istate, ent->name, ent->len))continue ;show_dir_entry (tag_other, ent); }
@@ -336,7 +337,7 @@ static void show_files(struct dir_struct *dir) dir->flags |=DIR_COLLECT_KILLED_ONLY ;fill_directory (dir, &the_index, &pathspec);if (show_others) -show_other_files (dir); +show_other_files (&the_index, dir);if (show_killed)show_killed_files (&the_index, dir); }
Signature · 1 line
2.13.0.303.g4ebf302169-goog
PATCH 11/23 ls-files: convert show_ru_info to take an index
In reply to Brandon Williams--- builtin/ls-files.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
builtin/ls-files.c +4 -4
@@ -292,14 +292,14 @@ static void show_ce_entry(const char *tag, const struct cache_entry *ce)strbuf_release (&name); } -static void show_ru_info (void ) +static void show_ru_info (const struct index_state *istate) {struct string_list_item *item; -if (!the_index.resolve_undo) +if (!istate->resolve_undo)return ; -for_each_string_list_item (item, the_index.resolve_undo) { +for_each_string_list_item (item, istate->resolve_undo) {const char *path = item->string;struct resolve_undo_info *ui = item->util;int i, len;
@@ -685,7 +685,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix) }show_files (&dir);if (show_resolve_undo) -show_ru_info (); +show_ru_info (&the_index);if (ps_matched) {int bad;
Signature · 1 line
2.13.0.303.g4ebf302169-goog
PATCH 12/23 ls-files: convert ce_excluded to take an index
In reply to Brandon Williams--- builtin/ls-files.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
builtin/ls-files.c +5 -4
@@ -321,10 +321,11 @@ static void show_ru_info(const struct index_state *istate) } } -static int ce_excluded (struct dir_struct *dir,const struct cache_entry *ce) +static int ce_excluded (struct dir_struct *dir,struct index_state *istate, +const struct cache_entry *ce) {int dtype =ce_to_dtype (ce); -return is_excluded (dir, &the_index, ce->name, &dtype); +return is_excluded (dir, istate, ce->name, &dtype); }static void show_files (struct dir_struct *dir)
@@ -345,7 +346,7 @@ static void show_files(struct dir_struct *dir)for (i =0 ; i < active_nr; i++) {const struct cache_entry *ce = active_cache[i];if ((dir->flags &DIR_SHOW_IGNORED ) && - !ce_excluded (dir, ce)) + !ce_excluded (dir, &the_index, ce))continue ;if (show_unmerged && !ce_stage (ce))continue ;
@@ -361,7 +362,7 @@ static void show_files(struct dir_struct *dir)struct stat st;int err;if ((dir->flags &DIR_SHOW_IGNORED ) && - !ce_excluded (dir, ce)) + !ce_excluded (dir, &the_index, ce))continue ;if (ce->ce_flags &CE_UPDATE )continue ;
Signature · 1 line
2.13.0.303.g4ebf302169-goog
PATCH 13/23 ls-files: convert prune_cache to take an index
In reply to Brandon Williams--- builtin/ls-files.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)
builtin/ls-files.c +8 -7
@@ -380,30 +380,31 @@ static void show_files(struct dir_struct *dir)/* * Prune the index to only contain stuff starting with "prefix" */ -static void prune_cache (const char *prefix,size_t prefixlen) +static void prune_index (struct index_state *istate, +const char *prefix,size_t prefixlen) {int pos;unsigned int first, last;if (!prefix)return ; - pos =cache_name_pos (prefix, prefixlen); + pos =index_name_pos (istate, prefix, prefixlen);if (pos <0 ) pos = -pos-1 ; first = pos; - last = active_nr; + last = istate->cache_nr;while (last > first) {int next = (last + first) >>1 ; -const struct cache_entry *ce = active_cache[next]; +const struct cache_entry *ce = istate->cache[next];if (!strncmp (ce->name, prefix, prefixlen)) { first = next+1 ;continue ; } last = next; } -memmove (active_cache, active_cache + pos, +memmove (istate->cache, istate->cache + pos, (last - pos) *sizeof (struct cache_entry *)); - active_nr = last - pos; + istate->cache_nr = last - pos; }static int get_common_prefix_len (const char *common_prefix)
@@ -661,7 +662,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix) max_prefix =common_prefix (&pathspec); max_prefix_len =get_common_prefix_len (max_prefix); -prune_cache (max_prefix, max_prefix_len); +prune_index (&the_index, max_prefix, max_prefix_len);/* Treat unmatching pathspec elements as errors */ if (pathspec.nr && error_unmatch)
Signature · 1 line
2.13.0.303.g4ebf302169-goog
PATCH 14/23 ls-files: convert show_files to take an index
In reply to Brandon Williams--- builtin/ls-files.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)
builtin/ls-files.c +12 -12
@@ -328,7 +328,7 @@ static int ce_excluded(struct dir_struct *dir, struct index_state *istate,return is_excluded (dir, istate, ce->name, &dtype); } -static void show_files (struct dir_struct *dir) +static void show_files (struct index_state *istate,struct dir_struct *dir) {int i;
@@ -336,17 +336,17 @@ static void show_files(struct dir_struct *dir)if (show_others || show_killed) {if (!show_others) dir->flags |=DIR_COLLECT_KILLED_ONLY ; -fill_directory (dir, &the_index, &pathspec); +fill_directory (dir, istate, &pathspec);if (show_others) -show_other_files (&the_index, dir); +show_other_files (istate, dir);if (show_killed) -show_killed_files (&the_index, dir); +show_killed_files (istate, dir); }if (show_cached || show_stage) { -for (i =0 ; i < active_nr; i++) { -const struct cache_entry *ce = active_cache[i]; +for (i =0 ; i < istate->cache_nr; i++) { +const struct cache_entry *ce = istate->cache[i];if ((dir->flags &DIR_SHOW_IGNORED ) && - !ce_excluded (dir, &the_index, ce)) + !ce_excluded (dir, istate, ce))continue ;if (show_unmerged && !ce_stage (ce))continue ;
@@ -357,12 +357,12 @@ static void show_files(struct dir_struct *dir) } }if (show_deleted || show_modified) { -for (i =0 ; i < active_nr; i++) { -const struct cache_entry *ce = active_cache[i]; +for (i =0 ; i < istate->cache_nr; i++) { +const struct cache_entry *ce = istate->cache[i];struct stat st;int err;if ((dir->flags &DIR_SHOW_IGNORED ) && - !ce_excluded (dir, &the_index, ce)) + !ce_excluded (dir, istate, ce))continue ;if (ce->ce_flags &CE_UPDATE )continue ;
@@ -371,7 +371,7 @@ static void show_files(struct dir_struct *dir) err =lstat (ce->name, &st);if (show_deleted && err)show_ce_entry (tag_removed, ce); -if (show_modified &&ce_modified (ce, &st,0 )) +if (show_modified &&ie_modified (istate, ce, &st,0 ))show_ce_entry (tag_modified, ce); } }
@@ -685,7 +685,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)die ("ls-files --with-tree is incompatible with -s or -u" );overlay_tree_on_index (&the_index, with_tree, max_prefix); } -show_files (&dir); +show_files (&the_index, &dir);if (show_resolve_undo)show_ru_info (&the_index);
Signature · 1 line
2.13.0.303.g4ebf302169-goog
PATCH 15/23 ls-files: factor out debug info into a function
In reply to Brandon Williams--- builtin/ls-files.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-)
builtin/ls-files.c +14 -9
@@ -93,6 +93,19 @@ static void write_name(const char *name)strbuf_release (&full_name); } +static void print_debug (const struct cache_entry *ce) +{ +if (debug_mode) { +const struct stat_data *sd = &ce->ce_stat_data; + +printf (" ctime: %d:%d\n" , sd->sd_ctime.sec, sd->sd_ctime.nsec); +printf (" mtime: %d:%d\n" , sd->sd_mtime.sec, sd->sd_mtime.nsec); +printf (" dev: %d\tino: %d\n" , sd->sd_dev, sd->sd_ino); +printf (" uid: %d\tgid: %d\n" , sd->sd_uid, sd->sd_gid); +printf (" size: %d\tflags: %x\n" , sd->sd_size, ce->ce_flags); + } +} +static void show_dir_entry (const char *tag,struct dir_entry *ent) {int len = max_prefix_len;
@@ -278,15 +291,7 @@ static void show_ce_entry(const char *tag, const struct cache_entry *ce) }write_eolinfo (&the_index, ce, ce->name);write_name (ce->name); -if (debug_mode) { -const struct stat_data *sd = &ce->ce_stat_data; - -printf (" ctime: %d:%d\n" , sd->sd_ctime.sec, sd->sd_ctime.nsec); -printf (" mtime: %d:%d\n" , sd->sd_mtime.sec, sd->sd_mtime.nsec); -printf (" dev: %d\tino: %d\n" , sd->sd_dev, sd->sd_ino); -printf (" uid: %d\tgid: %d\n" , sd->sd_uid, sd->sd_gid); -printf (" size: %d\tflags: %x\n" , sd->sd_size, ce->ce_flags); - } +print_debug (ce); }strbuf_release (&name);
Signature · 1 line
2.13.0.303.g4ebf302169-goog
PATCH 16/23 ls-files: factor out tag calculation
In reply to Brandon Williams--- builtin/ls-files.c | 41 +++++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 16 deletions(-)
builtin/ls-files.c +25 -16
@@ -93,6 +93,30 @@ static void write_name(const char *name)strbuf_release (&full_name); } +static const char *get_tag (const struct cache_entry *ce,const char *tag) +{ +static char alttag[4 ]; + +if (tag && *tag && show_valid_bit && (ce->ce_flags &CE_VALID )) { +memcpy (alttag, tag,3 ); + +if (isalpha (tag[0 ])) { + alttag[0 ] =tolower (tag[0 ]); + }else if (tag[0 ] =='?' ) { + alttag[0 ] ='!' ; + }else { + alttag[0 ] ='v' ; + alttag[1 ] = tag[0 ]; + alttag[2 ] =' ' ; + alttag[3 ] =0 ; + } + + tag = alttag; + } + +return tag; +} +static void print_debug (const struct cache_entry *ce) {if (debug_mode) {
@@ -263,22 +287,7 @@ static void show_ce_entry(const char *tag, const struct cache_entry *ce) len, ps_matched, S_ISDIR(ce->ce_mode) || S_ISGITLINK(ce->ce_mode))) { -if (tag && *tag && show_valid_bit && - (ce->ce_flags &CE_VALID )) { -static char alttag[4 ]; -memcpy (alttag, tag,3 ); -if (isalpha (tag[0 ])) - alttag[0 ] =tolower (tag[0 ]); -else if (tag[0 ] =='?' ) - alttag[0 ] ='!' ; -else { - alttag[0 ] ='v' ; - alttag[1 ] = tag[0 ]; - alttag[2 ] =' ' ; - alttag[3 ] =0 ; - } - tag = alttag; - } + tag =get_tag (ce, tag);if (!show_stage) {fputs (tag, stdout);
Signature · 1 line
2.13.0.303.g4ebf302169-goog
PATCH 17/23 repo: introduce new repository object
In reply to Brandon WilliamsIntroduce 'struct repo' an object used to represent a repository.
--- Makefile | 1 + repo.c | 42 ++++++++++++++++++++++++++++++++++++++++++ repo.h | 15 +++++++++++++++ 3 files changed, 58 insertions(+) create mode 100644 repo.c create mode 100644 repo.h
Makefile +1 -0
@@ -821,6 +821,7 @@ LIB_OBJS += refs/ref-cache.o LIB_OBJS += ref-filter.o LIB_OBJS += remote.o LIB_OBJS += replace_object.o +LIB_OBJS += repo.o LIB_OBJS += rerere.o LIB_OBJS += resolve-undo.o LIB_OBJS += revision.o
repo.c new file +42 -0
42 lines not on this page · raw message
repo.h new file +15 -0
@@ -0,0 +1,15 @@ +#ifndef REPO_H +#define REPO_H + +struct index_state ; + +struct repo { +char *gitdir; +char *worktree; +const char *submodule_prefix; +}; + +extern int repo_init (struct repo *repo,const char *gitdir,const char *worktree); +extern void repo_clear (struct repo *repo); + +#endif /* REPO_H */
Signature · 1 line
2.13.0.303.g4ebf302169-goog
Re: [WIP/RFC 17/23] repo: introduce new repository object
Quoted 1 line · Brandon Williams
Introduce 'struct repo' an object used to represent a repository.
Is this the right place to outline what you expect from a repo object? Are you planning to use it everywhere?
Is it lazy-init'd and it takes care of it itself, or would the caller have to take
care of the state of the repo? ("the repo object is just a place to put the current globals")
Quoted 28 lines
Signed-off-by Brandon Williams <[email protected]>--- Makefile | 1 + repo.c | 42 ++++++++++++++++++++++++++++++++++++++++++ repo.h | 15 +++++++++++++++ 3 files changed, 58 insertions(+) create mode 100644 repo.c create mode 100644 repo.hMakefile +1
-0@@ -821,6 +821,7 @@ LIB_OBJS += refs/ref-cache.o LIB_OBJS += ref-filter.o LIB_OBJS += remote.o LIB_OBJS += replace_object.o +LIB_OBJS += repo.o LIB_OBJS += rerere.o LIB_OBJS += resolve-undo.o LIB_OBJS += revision.orepo.c new file +0
-0
repo.c line 5
Quoted 5 lines
repo.c new file +5
-0@@ -0,0 +1,42 @@ +#include "cache.h" +#include "repo.h" + +int +repo_init (struct repo *repo,const char *gitdir,const char *worktree)
style ;)
repo.c line 26
Quoted 1 line
repo.c new file +1
-0+/* Maybe need a check to verify that a worktree is indeed a worktree? */
add NEEDSWORK/FIXME prefix to comment?
repo.c line 36
Quoted 2 lines
repo.c new file +2
-0+void +repo_clear (struct repo *repo)
style ;)
Re: [WIP/RFC 17/23] repo: introduce new repository object
Quoted 9 lines · Stefan Beller
Quoted 1 line · Brandon Williams
Introduce 'struct repo' an object used to represent a repository.
Is this the right place to outline what you expect from a repo object? Are you planning to use it everywhere?
Is it lazy-init'd and it takes care of it itself, or would the caller have to take
care of the state of the repo? ("the repo object is just a place to put the current globals")
Those are all great questions, questions that I don't think I have all the answers for right now. Since this is still in the idea phase I'm hoping to hear what other people think this would look like in an ideal world. I don't think everything would need to be lazy-init'd or can easily be done that way. At least the index stuff isn't set up to be able to do that. I can see the config being lazy-init'd in one way or another, though that would require passing in a repo object instead of a config-set when you want to look-up a config value (which is probably very reasonable) that way the config-set stored in the repo object can be properly initialized.
Quoted 28 lines
Quoted 28 lines
Signed-off-by Brandon Williams <[email protected]>--- Makefile | 1 + repo.c | 42 ++++++++++++++++++++++++++++++++++++++++++ repo.h | 15 +++++++++++++++ 3 files changed, 58 insertions(+) create mode 100644 repo.c create mode 100644 repo.hMakefile +1
-0@@ -821,6 +821,7 @@ LIB_OBJS += refs/ref-cache.o LIB_OBJS += ref-filter.o LIB_OBJS += remote.o LIB_OBJS += replace_object.o +LIB_OBJS += repo.o LIB_OBJS += rerere.o LIB_OBJS += resolve-undo.o LIB_OBJS += revision.orepo.c new file +0
-0
Quoted 8 lines
repo.c line 5
Quoted 6 lines
repo.c new file +5
-0@@ -0,0 +1,42 @@ +#include "cache.h" +#include "repo.h" + +int +repo_init (struct repo *repo,const char *gitdir,const char *worktree)style ;)
Darn, I forgot to change this before sending out. An easy fix, though I still like this style better :P
Quoted 10 lines
repo.c line 26
Quoted 3 lines
repo.c new file +1
-0+/* Maybe need a check to verify that a worktree is indeed a worktree? */ add NEEDSWORK/FIXME prefix to comment?
repo.c line 36
Quoted 2 lines
repo.c new file +2
-0+void +repo_clear (struct repo *repo)style ;)
Signature · 1 line
Brandon Williams
PATCH 18/23 repo: add index_state to struct repo
In reply to Brandon Williams--- repo.c | 19 +++++++++++++++++++ repo.h | 2 ++ 2 files changed, 21 insertions(+)
repo.c +19 -0
@@ -2,6 +2,20 @@#include "repo.h" int +repo_read_index (struct repo *repo,const char *index_file) +{ +char *index_path =xstrfmt ("%s/index" , repo->gitdir); +const char *file = index_file ? index_file : index_path; + + repo->index =xcalloc (1 ,sizeof (struct index_state )); +if (read_index_from (repo->index, file) <0 ) +die ("failure reading index\n" ); + +free (index_path); +return 0 ; +} + +int repo_init (struct repo *repo,const char *gitdir,const char *worktree) {int error =0 ;
@@ -39,4 +53,9 @@ repo_clear(struct repo *repo) repo->gitdir =NULL ;free (repo->worktree); repo->worktree =NULL ; + +if (repo->index) { +discard_index (repo->index); +free (repo->index); + } }
repo.h +2 -0
@@ -7,8 +7,10 @@ struct repo {char *gitdir;char *worktree;const char *submodule_prefix; +struct index_state *index; }; +extern int repo_read_index (struct repo *repo,const char *index_file);extern int repo_init (struct repo *repo,const char *gitdir,const char *worktree);extern void repo_clear (struct repo *repo);
Signature · 1 line
2.13.0.303.g4ebf302169-goog
Re: [WIP/RFC 18/23] repo: add index_state to struct repo
Quoted 2 lines
int
+repo_read_index(struct repo *repo, const char*index_file)
...
repo.c line 19
Quoted 3 lines
repo.c +2
-0+ +int repo_init (struct repo *repo,const char *gitdir,const char *worktree)
The version 2.13.0.303.g4ebf302169-goog doesn't have diff slider heuristics on by default, and you also did not enable it? (I am curious if the heuristics would have helped here)
PATCH 19/23 repo: add per repo config
In reply to Brandon Williams--- cache.h | 1 + config.c | 2 +- repo.c | 27 ++++++++++++++++++++++++++- repo.h | 2 ++ 4 files changed, 30 insertions(+), 2 deletions(-)
cache.h +1 -0
@@ -2015,6 +2015,7 @@ struct config_set { };extern void git_configset_init (struct config_set *cs); +extern int config_set_callback (const char *key,const char *value,void *cb);extern int git_configset_add_file (struct config_set *cs,const char *filename);extern int git_configset_get_value (struct config_set *cs,const char *key,const char **value);extern const struct string_list *git_configset_get_value_multi (struct config_set *cs,const char *key);
config.c +1 -1
@@ -1765,7 +1765,7 @@ void git_configset_clear(struct config_set *cs) cs->list.items =NULL ; } -static int config_set_callback (const char *key,const char *value,void *cb) +int config_set_callback (const char *key,const char *value,void *cb) {struct config_set *cs = cb;configset_add_value (cs, key, value);
repo.c +26 -1
@@ -1,10 +1,16 @@#include "cache.h" #include "repo.h" +char * +repo_git_pathdup (const struct repo *repo,const char *file) +{ +return xstrfmt ("%s/%s" , repo->gitdir, file); +} +int repo_read_index (struct repo *repo,const char *index_file) { -char *index_path =xstrfmt ("%s/index" , repo->gitdir); +char *index_path =repo_git_pathdup (repo,"index" );const char *file = index_file ? index_file : index_path; repo->index =xcalloc (1 ,sizeof (struct index_state ));
@@ -16,6 +22,18 @@ repo_read_index(struct repo *repo, const char *index_file) }int +repo_read_config (struct repo *repo) +{ +struct config_options opts = {1 , repo->gitdir }; + + repo->config =xcalloc (1 ,sizeof (struct config_set )); +git_configset_init (repo->config); + +return git_config_with_options (config_set_callback, repo->config, +NULL , &opts); +} + +int repo_init (struct repo *repo,const char *gitdir,const char *worktree) {int error =0 ;
@@ -40,6 +58,8 @@ repo_init(struct repo *repo, const char *gitdir, const char *worktree)/* Maybe need a check to verify that a worktree is indeed a worktree? */ repo->worktree =xstrdup_or_null (worktree); +repo_read_config (repo); +free (abspath);free (suspect);
@@ -58,4 +78,9 @@ repo_clear(struct repo *repo)discard_index (repo->index);free (repo->index); } + +if (repo->config) { +git_configset_clear (repo->config); +free (repo->config); + } }
repo.h +2 -0
@@ -2,12 +2,14 @@#define REPO_H struct index_state ; +struct config_set ;struct repo {char *gitdir;char *worktree;const char *submodule_prefix;struct index_state *index; +struct config_set *config; };extern int repo_read_index (struct repo *repo,const char *index_file);
Signature · 1 line
2.13.0.303.g4ebf302169-goog
PATCH 20/23 submodule-config: refactor to allow for multiple submodule_cache's
In reply to Brandon WilliamsA repository object will have its own submodule cache so lay the ground work for allowing multiple submodule cache structs.
--- submodule-config.c | 40 ++++++++++++++++++++++++++++++++-------- submodule-config.h | 10 ++++++++++ 2 files changed, 42 insertions(+), 8 deletions(-)
submodule-config.c +32 -8
@@ -30,7 +30,7 @@ enum lookup_type {lookup_path }; -static struct submodule_cache the_submodule_cache; +struct submodule_cache the_submodule_cache;static int is_cache_init; static int config_path_cmp(const struct submodule_entry *a,
@@ -49,7 +49,12 @@ static int config_name_cmp(const struct submodule_entry *a,hashcmp (a->config->gitmodules_sha1, b->config->gitmodules_sha1); } -static void cache_init (struct submodule_cache *cache) +struct submodule_cache *submodule_cache_alloc (void ) +{ +return xcalloc (1 ,sizeof (struct submodule_cache )); +} + +void submodule_cache_init (struct submodule_cache *cache) {hashmap_init (&cache->for_path, (hashmap_cmp_fn ) config_path_cmp,0 );hashmap_init (&cache->for_name, (hashmap_cmp_fn ) config_name_cmp,0 );
@@ -64,7 +69,7 @@ static void free_one_config(struct submodule_entry *entry)free (entry->config); } -static void cache_free (struct submodule_cache *cache) +static void submodule_cache_clear (struct submodule_cache *cache) {struct hashmap_iter iter;struct submodule_entry *entry;
@@ -82,6 +87,12 @@ static void cache_free(struct submodule_cache *cache)hashmap_free (&cache->for_name,1 ); } +void submodule_cache_free (struct submodule_cache *cache) +{ +submodule_cache_clear (cache); +free (cache); +} +static unsigned int hash_sha1_string (const unsigned char *sha1,const char *string) {
@@ -493,27 +504,40 @@ static const struct submodule *config_from(struct submodule_cache *cache,return submodule; } +const struct submodule * +submodule_from_cache (struct submodule_cache *cache, +const unsigned char *treeish_name, +const char *key) +{ +return config_from (cache, treeish_name, key, lookup_path); +} +static void ensure_cache_init (void ) {if (is_cache_init)return ; -cache_init (&the_submodule_cache); +submodule_cache_init (&the_submodule_cache); is_cache_init =1 ; } -int parse_submodule_config_option (const char *var,const char *value) +int parse_submodule_config_option_cache (struct submodule_cache *cache,const char *var,const char *value) {struct parse_config_parameter parameter; - parameter.cache = &the_submodule_cache; + parameter.cache = cache; parameter.treeish_name =NULL ; parameter.gitmodules_sha1 = null_sha1; parameter.overwrite =1 ; -ensure_cache_init ();return parse_config (var, value, ¶meter); } +int parse_submodule_config_option (const char *var,const char *value) +{ +ensure_cache_init (); +return parse_submodule_config_option_cache (&the_submodule_cache, var, value); +} +const struct submodule *submodule_from_name (const unsigned char *treeish_name,const char *name) {
@@ -530,6 +554,6 @@ const struct submodule *submodule_from_path(const unsigned char *treeish_name,void submodule_free (void ) { -cache_free (&the_submodule_cache); +submodule_cache_clear (&the_submodule_cache); is_cache_init =0 ; }
submodule-config.h +10 -0
@@ -22,14 +22,24 @@ struct submodule {int recommend_shallow; }; +struct submodule_cache ; + +extern struct submodule_cache *submodule_cache_alloc (void ); +extern void submodule_cache_init (struct submodule_cache *cache); +extern void submodule_cache_free (struct submodule_cache *cache); +extern int parse_fetch_recurse_submodules_arg (const char *opt,const char *arg);extern int parse_update_recurse_submodules_arg (const char *opt,const char *arg);extern int parse_push_recurse_submodules_arg (const char *opt,const char *arg);extern int parse_submodule_config_option (const char *var,const char *value); +extern int parse_submodule_config_option_cache (struct submodule_cache *cache,const char *var,const char *value);extern const struct submodule *submodule_from_name (const unsigned char *commit_or_tree,const char *name);extern const struct submodule *submodule_from_path (const unsigned char *commit_or_tree,const char *path); +extern const struct submodule *submodule_from_cache (struct submodule_cache *cache, +const unsigned char *treeish_name, +const char *key);extern int gitmodule_sha1_from_commit (const unsigned char *commit_sha1,unsigned char *gitmodules_sha1,struct strbuf *rev);
Signature · 1 line
2.13.0.303.g4ebf302169-goog
PATCH 21/23 repo: add repo_read_gitmodules
In reply to Brandon WilliamsAdd ability for a repository to poulate its own submodule_cache by reading the repository's gitmodules file.
--- repo.c | 26 ++++++++++++++++++++++++++ repo.h | 3 +++ 2 files changed, 29 insertions(+)
repo.c +26 -0
@@ -1,4 +1,6 @@#include "cache.h" +#include "submodule.h" +#include "submodule-config.h" #include "repo.h" char *
@@ -33,6 +35,27 @@ repo_read_config(struct repo *repo) NULL, &opts); } +static int +gitmodules_cb (const char *var,const char *value,void *data) +{ +struct repo *repo = data; +return parse_submodule_config_option_cache (repo->submodule_cache, var, value); +} + +int +repo_read_gitmodules (struct repo *repo) +{ +char *gitmodules_path =xstrfmt ("%s/.gitmodules" , repo->worktree); + +if (!repo->worktree) +die ("BUG: no worktree" ); + + repo->submodule_cache =submodule_cache_alloc (); +submodule_cache_init (repo->submodule_cache); +git_config_from_file (gitmodules_cb, gitmodules_path, repo); +return 0 ; +} +int repo_init (struct repo *repo,const char *gitdir,const char *worktree) {
@@ -83,4 +106,7 @@ repo_clear(struct repo *repo)git_configset_clear (repo->config);free (repo->config); } + +if (repo->submodule_cache) +submodule_cache_free (repo->submodule_cache); }
repo.h +3 -0
@@ -10,10 +10,13 @@ struct repo {const char *submodule_prefix;struct index_state *index;struct config_set *config; + struct submodule_cache *submodule_cache; };extern int repo_read_index (struct repo *repo,const char *index_file);extern int repo_init (struct repo *repo,const char *gitdir,const char *worktree);extern void repo_clear (struct repo *repo); +extern int repo_read_gitmodules (struct repo *repo); +#endif /* REPO_H */
Signature · 1 line
2.13.0.303.g4ebf302169-goog
PATCH 22/23 submodule: add is_submodule_active
In reply to Brandon WilliamsAdd method which checks if a submodule is active given the provided repository object.
NOTE: this is almost a copy-paste of is_submodule_initialized. I tried to convert is_submodule_initialized to take a submodule_cache and config_set parameters but that turned out to break some other code paths which I didn't want to change yet.
--- submodule.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ submodule.h | 2 ++ 2 files changed, 53 insertions(+)
submodule.c +51 -0
@@ -16,6 +16,7 @@#include "quote.h" #include "remote.h" #include "worktree.h" +#include "repo.h" static int config_fetch_recurse_submodules =RECURSE_SUBMODULES_ON_DEMAND ;static int config_update_recurse_submodules =RECURSE_SUBMODULES_DEFAULT ;
@@ -270,6 +271,56 @@ int is_submodule_initialized(const char *path)return ret; } +int is_submodule_active (struct repo *repo,const char *path) +{ +int ret =0 ; +char *key =NULL ; +char *value =NULL ; +const struct string_list *sl; +const struct submodule *module; + + module =submodule_from_cache (repo->submodule_cache, null_sha1, path); + +/* early return if there isn't a path->module mapping */ +if (!module) +return 0 ; + +/* submodule.<name>.active is set */ + key =xstrfmt ("submodule.%s.active" , module->name); +if (!git_configset_get_bool (repo->config, key, &ret)) { +free (key); +return ret; + } +free (key); + +/* submodule.active is set */ + sl =git_configset_get_value_multi (repo->config,"submodule.active" ); +if (sl) { +struct pathspec ps; +struct argv_array args =ARGV_ARRAY_INIT ; +const struct string_list_item *item; + +for_each_string_list_item (item, sl) { +argv_array_push (&args, item->string); + } + +parse_pathspec (&ps,0 ,0 ,NULL , args.argv); + ret =match_pathspec (&ps, path,strlen (path),0 ,NULL ,1 ); + +argv_array_clear (&args); +clear_pathspec (&ps); +return ret; + } + +/* fallback to checking if the URL is set */ + key =xstrfmt ("submodule.%s.url" , module->name); + ret = !git_configset_get_string (repo->config, key, &value); + +free (value); +free (key); +return ret; +} +int is_submodule_populated_gently (const char *path,int *return_error_code) {int ret =0 ;
submodule.h +2 -0
@@ -5,6 +5,7 @@ struct diff_options;struct argv_array ;struct oid_array ;struct remote ; +struct repo ;enum {RECURSE_SUBMODULES_ONLY =-5 ,
@@ -42,6 +43,7 @@ extern int submodule_config(const char *var, const char *value, void *cb);extern void gitmodules_config (void );extern void gitmodules_config_sha1 (const unsigned char *commit_sha1);extern int is_submodule_initialized (const char *path); +extern int is_submodule_active (struct repo *repo,const char *path);/* * Determine if a submodule has been populated at a given 'path' by checking if * the <path>/.git resolves to a valid git repository.
Signature · 1 line
2.13.0.303.g4ebf302169-goog
PATCH 23/23 ls-files: use repository object
In reply to Brandon WilliamsConvert ls-files to use a repository struct and recurse submodules inprocess.
--- builtin/ls-files.c | 217 +++++++++++++++------------------ git.c | 2 +- t/t3007-ls-files-recurse-submodules.sh | 39 ++++++ 3 files changed, 137 insertions(+), 121 deletions(-)
builtin/ls-files.c +97 -120
@@ -5,6 +5,7 @@* * Copyright (C) Linus Torvalds, 2005 */ +#define NO_THE_INDEX_COMPATIBILITY_MACROS #include "cache.h" #include "quote.h" #include "dir.h"
@@ -16,6 +17,7 @@#include "pathspec.h" #include "run-command.h" #include "submodule.h" +#include "repo.h" static int abbrev;static int show_deleted;
@@ -31,10 +33,8 @@ static int line_terminator = '\n';static int debug_mode;static int show_eol;static int recurse_submodules; -static struct argv_array submodule_options =ARGV_ARRAY_INIT ;static const char *prefix; -static const char *super_prefix;static int max_prefix_len;static int prefix_len;static struct pathspec pathspec;
@@ -73,24 +73,11 @@ static void write_eolinfo(const struct index_state *istate,static void write_name (const char *name) {/* -* Prepend the super_prefix to name to construct the full_name to be -* written. -*/ -struct strbuf full_name =STRBUF_INIT ; -if (super_prefix) { -strbuf_addstr (&full_name, super_prefix); -strbuf_addstr (&full_name, name); - name = full_name.buf; - } - -/* * With "--full-name", prefix_len=0; this caller needs to pass * an empty string in that case (a NULL is good for ""). */ write_name_quoted_relative (name, prefix_len ? prefix :NULL , stdout, line_terminator); - -strbuf_release (&full_name); }static const char *get_tag (const struct cache_entry *ce,const char *tag)
@@ -209,101 +196,60 @@ static void show_killed_files(const struct index_state *istate, } } -/* -* Compile an argv_array with all of the options supported by --recurse_submodules -*/ -static void compile_submodule_options (const char **argv, -const struct dir_struct *dir, -int show_tag) -{ -if (line_terminator =='\0' ) -argv_array_push (&submodule_options,"-z" ); -if (show_tag) -argv_array_push (&submodule_options,"-t" ); -if (show_valid_bit) -argv_array_push (&submodule_options,"-v" ); -if (show_cached) -argv_array_push (&submodule_options,"--cached" ); -if (show_eol) -argv_array_push (&submodule_options,"--eol" ); -if (debug_mode) -argv_array_push (&submodule_options,"--debug" ); - -/* Add Pathspecs */ -argv_array_push (&submodule_options,"--" ); -for (; *argv; argv++) -argv_array_push (&submodule_options, *argv); -} +static void show_files (struct repo *repo,struct dir_struct *dir); -/** -* Recursively call ls-files on a submodule -*/ -static void show_gitlink (const struct cache_entry *ce) +static void show_submodule (const struct repo *superproject, +struct dir_struct *dir,const char *path) { -struct child_process cp =CHILD_PROCESS_INIT ; -int status; -char *dir; - -prepare_submodule_repo_env (&cp.env_array); -argv_array_push (&cp.env_array,GIT_DIR_ENVIRONMENT ); - -if (prefix_len) -argv_array_pushf (&cp.env_array,"%s=%s" , -GIT_TOPLEVEL_PREFIX_ENVIRONMENT , - prefix); -argv_array_pushf (&cp.args,"--super-prefix=%s%s/" , - super_prefix ? super_prefix :"" , - ce->name); -argv_array_push (&cp.args,"ls-files" ); -argv_array_push (&cp.args,"--recurse-submodules" ); - -/* add supported options */ -argv_array_pushv (&cp.args, submodule_options.argv); - - cp.git_cmd =1 ; - dir =mkpathdup ("%s/%s" ,get_git_work_tree (), ce->name); - cp.dir = dir; - status =run_command (&cp); -free (dir); -if (status) -exit (status); +struct repo submodule; +char *gitdir =mkpathdup ("%s/%s" , superproject->worktree, path); +repo_init (&submodule, gitdir, gitdir); + +repo_read_index (&submodule,NULL ); +repo_read_gitmodules (&submodule); + +if (superproject->submodule_prefix) + submodule.submodule_prefix =xstrfmt ("%s%s/" , superproject->submodule_prefix, path); +else + submodule.submodule_prefix =xstrfmt ("%s/" , path); +show_files (&submodule, dir); + +free ((char *) submodule.submodule_prefix); +repo_clear (&submodule); +free (gitdir); } -static void show_ce_entry (const char *tag,const struct cache_entry *ce) +static void show_ce (struct repo *repo,struct dir_struct *dir, +const struct cache_entry *ce,const char *fullname, +const char *tag) { -struct strbuf name =STRBUF_INIT ; -int len = max_prefix_len; -if (super_prefix) -strbuf_addstr (&name, super_prefix); -strbuf_addstr (&name, ce->name); - -if (len >ce_namelen (ce)) +if (max_prefix_len >strlen (fullname))die ("git ls-files: internal error - cache entry not superset of prefix" );if (recurse_submodules &&S_ISGITLINK (ce->ce_mode) && -submodule_path_match (&pathspec, name.buf, ps_matched)) { -show_gitlink (ce); - }else if (match_pathspec (&pathspec, name.buf, name.len, - len, ps_matched, +submodule_path_match (&pathspec, fullname, ps_matched) && +is_submodule_active (repo, ce->name)) { +show_submodule (repo, dir, ce->name); + }else if (match_pathspec (&pathspec, fullname,strlen (fullname), + max_prefix_len, ps_matched,S_ISDIR (ce->ce_mode) ||S_ISGITLINK (ce->ce_mode))) { tag =get_tag (ce, tag); -if (!show_stage) { -fputs (tag, stdout); - }else { +if (show_stage) {printf ("%s%06o %s %d\t" , tag, ce->ce_mode,find_unique_abbrev (ce->oid.hash, abbrev),ce_stage (ce)); + }else { +fputs (tag, stdout); } -write_eolinfo (&the_index, ce, ce->name); -write_name (ce->name); + +write_eolinfo (repo->index, ce, fullname); +write_name (fullname);print_debug (ce); } - -strbuf_release (&name); }static void show_ru_info (const struct index_state *istate)
@@ -336,59 +282,80 @@ static void show_ru_info(const struct index_state *istate) }static int ce_excluded (struct dir_struct *dir,struct index_state *istate, -const struct cache_entry *ce) +const char *fullname,const struct cache_entry *ce) {int dtype =ce_to_dtype (ce); -return is_excluded (dir, istate, ce->name, &dtype); +return is_excluded (dir, istate, fullname, &dtype); } -static void show_files (struct index_state *istate,struct dir_struct *dir) +static void construct_fullname (struct strbuf *out,const struct repo *repo, +const struct cache_entry *ce) +{ +strbuf_reset (out); +if (repo->submodule_prefix) +strbuf_addstr (out, repo->submodule_prefix); +strbuf_addstr (out, ce->name); + +} + +static void show_files (struct repo *repo,struct dir_struct *dir) {int i; +struct strbuf fullname =STRBUF_INIT ;/* For cached/deleted files we don't need to even do the readdir */ if (show_others || show_killed) {if (!show_others) dir->flags |=DIR_COLLECT_KILLED_ONLY ; -fill_directory (dir, istate, &pathspec); +fill_directory (dir, repo->index, &pathspec);if (show_others) -show_other_files (istate, dir); +show_other_files (repo->index, dir);if (show_killed) -show_killed_files (istate, dir); +show_killed_files (repo->index, dir); }if (show_cached || show_stage) { -for (i =0 ; i < istate->cache_nr; i++) { -const struct cache_entry *ce = istate->cache[i]; +for (i =0 ; i < repo->index->cache_nr; i++) { +const struct cache_entry *ce = repo->index->cache[i]; + +construct_fullname (&fullname, repo, ce); +if ((dir->flags &DIR_SHOW_IGNORED ) && - !ce_excluded (dir, istate, ce)) + !ce_excluded (dir, repo->index, fullname.buf, ce))continue ;if (show_unmerged && !ce_stage (ce))continue ;if (ce->ce_flags &CE_UPDATE )continue ; -show_ce_entry (ce_stage (ce) ? tag_unmerged : - (ce_skip_worktree (ce) ? tag_skip_worktree : tag_cached), ce); +show_ce (repo, dir, ce, fullname.buf, +ce_stage (ce) ? tag_unmerged : + (ce_skip_worktree (ce) ? tag_skip_worktree : + tag_cached)); } }if (show_deleted || show_modified) { -for (i =0 ; i < istate->cache_nr; i++) { -const struct cache_entry *ce = istate->cache[i]; +for (i =0 ; i < repo->index->cache_nr; i++) { +const struct cache_entry *ce = repo->index->cache[i];struct stat st;int err; + +construct_fullname (&fullname, repo, ce); +if ((dir->flags &DIR_SHOW_IGNORED ) && - !ce_excluded (dir, istate, ce)) + !ce_excluded (dir, repo->index, fullname.buf, ce))continue ;if (ce->ce_flags &CE_UPDATE )continue ;if (ce_skip_worktree (ce))continue ; - err =lstat (ce->name, &st); + err =lstat (fullname.buf, &st);if (show_deleted && err) -show_ce_entry (tag_removed, ce); -if (show_modified &&ie_modified (istate, ce, &st,0 )) -show_ce_entry (tag_modified, ce); +show_ce (repo, dir, ce, fullname.buf, tag_removed); +if (show_modified &&ie_modified (repo->index, ce, &st,0 )) +show_ce (repo, dir, ce, fullname.buf, tag_modified); } } + +strbuf_release (&fullname); }/*
@@ -542,8 +509,9 @@ static int option_parse_exclude_standard(const struct option *opt,int cmd_ls_files (int argc,const char **argv,const char *cmd_prefix) { +struct repo main_repository;int require_work_tree =0 , show_tag =0 , i; -const char *max_prefix; +char *max_prefix;struct dir_struct dir;struct exclude_list *el;struct string_list exclude_list =STRING_LIST_INIT_NODUP ;
@@ -613,11 +581,11 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix) prefix = cmd_prefix;if (prefix) prefix_len =strlen (prefix); - super_prefix =get_super_prefix ();git_config (git_default_config,NULL ); -if (read_cache () <0 ) -die ("index file corrupt" ); +repo_init (&main_repository,get_git_dir (),get_git_work_tree ()); + +repo_read_index (&main_repository,get_index_file ()); argc =parse_options (argc, argv, prefix, builtin_ls_files_options, ls_files_usage,0 );
@@ -649,12 +617,14 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)if (require_work_tree && !is_inside_work_tree ())setup_work_tree (); -if (recurse_submodules) -compile_submodule_options (argv, &dir, show_tag); +if (recurse_submodules) { +repo_read_gitmodules (&main_repository); + }if (recurse_submodules && (show_stage || show_deleted || show_others || show_unmerged || - show_killed || show_modified || show_resolve_undo || with_tree)) + show_killed || show_modified || show_resolve_undo || with_tree || + show_eol))die ("ls-files --recurse-submodules unsupported mode" );if (recurse_submodules && error_unmatch)
@@ -668,7 +638,10 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)/* * Find common prefix for all pathspec's * This is used as a performance optimization which unfortunately cannot -* be done when recursing into submodules +* be done when recursing into submodules because when a pathspec is +* given which spans repository boundaries you can't simply remove the +* submodule entry because the pathspec may match something inside the +* submodule. */ if (recurse_submodules) max_prefix =NULL ;
@@ -676,7 +649,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix) max_prefix =common_prefix (&pathspec); max_prefix_len =get_common_prefix_len (max_prefix); -prune_index (&the_index, max_prefix, max_prefix_len); +prune_index (main_repository.index, max_prefix, max_prefix_len);/* Treat unmatching pathspec elements as errors */ if (pathspec.nr && error_unmatch)
@@ -697,11 +670,15 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)*/ if (show_stage || show_unmerged)die ("ls-files --with-tree is incompatible with -s or -u" ); -overlay_tree_on_index (&the_index, with_tree, max_prefix); +overlay_tree_on_index (main_repository.index, with_tree, max_prefix); } -show_files (&the_index, &dir); + +show_files (&main_repository, &dir); +if (show_resolve_undo) -show_ru_info (&the_index); +show_ru_info (main_repository.index); + +repo_clear (&main_repository);if (ps_matched) {int bad;
git.c +1 -1
@@ -441,7 +441,7 @@ static struct cmd_struct commands[] = { { "init-db", cmd_init_db }, { "interpret-trailers", cmd_interpret_trailers, RUN_SETUP_GENTLY }, { "log", cmd_log, RUN_SETUP }, - { "ls-files", cmd_ls_files, RUN_SETUP | SUPPORT_SUPER_PREFIX }, + { "ls-files", cmd_ls_files, RUN_SETUP }, { "ls-remote", cmd_ls_remote, RUN_SETUP_GENTLY }, { "ls-tree", cmd_ls_tree, RUN_SETUP }, { "mailinfo", cmd_mailinfo, RUN_SETUP_GENTLY },
t/t3007-ls-files-recurse-submodules.sh +39 -0
@@ -135,6 +135,45 @@ test_expect_success '--recurse-submodules and pathspecs setup' ' test_cmp expect actual ' +test_expect_success 'inactive submodule' ' + test_when_finished "git config --bool submodule.submodule.active true" && + test_when_finished "git -C submodule config --bool submodule.subsub.active true" && + git config --bool submodule.submodule.active "false" && + + cat >expect <<-\EOF && + .gitmodules + a + b/b + h.txt + sib/file + sub/file + submodule + EOF + + git ls-files --recurse-submodules >actual && + test_cmp expect actual && + + git config --bool submodule.submodule.active "true" && + git -C submodule config --bool submodule.subsub.active "false" && + + cat >expect <<-\EOF && + .gitmodules + a + b/b + h.txt + sib/file + sub/file + submodule/.gitmodules + submodule/c + submodule/f.TXT + submodule/g.txt + submodule/subsub + EOF + + git ls-files --recurse-submodules >actual && + test_cmp expect actual +' + test_expect_success '--recurse-submodules and pathspecs' ' cat >expect <<-\EOF && h.txt
Signature · 1 line
2.13.0.303.g4ebf302169-goog
Quoted 29 lines · Brandon Williams
When I first started working on the git project I found it very difficult to understand parts of the code base because of the inherently global nature of our code. It also made working on submodules very difficult. Since we can only open up a single repository per process, you need to launch a child process in order to process a submodule. But you also need to be able to communicate other stateful information to the children processes so that the submodules know how best to format their output or match against a pathspec...it ends up feeling like layering on hack after hack. What I would really like to do, is to have the ability to have a repository object so that I can open a submodule in-process.
Before this becomes a reality for all commands, much of the library code would need to be refactored in order to work purely on handles instead of global state. As it turned out, ls-files is a pretty simple command and doesn't have *too* many dependencies. The biggest thing that needed to be changed was piping through an index into a couple library routines so that they don't inherently rely on '
the_index'. A few of these changes I've sent out and can be found at 'origin/bw/pathspec-sans-the-index' and 'origin/bw/dir-c-stops-relying-on-the-index' which this series is based on.Patches 1-16 are refactorings to prepare either library code or ls-files itself to be ready to handle passing around an index struct. Patches 17-22 introduce a repository struct and change a couple of things about how submodule caches work (getting submodule information from .gitmodules). And Patch 23 converts ls-files to use a repository struct.
The most interesting part of the series is from 17-23. And 1-16 could be taken as is without the rest of the series.
Quoted 6 lines
This is still very much in a WIP state, though it does pass all tests. What I'm hoping for here is to get a discussion started about the feasibility of a change like this and hopefully to get the ball rolling. Is this a direction we want to move in? Is it worth the pain?
Thanks for taking the time to look at this and entertain my insane ideas :)
Very nice and thanks for starting this.
Jeff
Glad to see you tackling this. This is definitely a step in the right direction.
I realize that it will take a lot of work and that intermediate steps may just be pushing it the global state one level higher but eventually it would be great to see an entire code path global state free!
I'm personally interested because reducing the reliance on global state also helps us in our performance work as it makes it more possible to use threading to scale up the performance.
Ben
Quoted 90 lines · Brandon Williams
When I first started working on the git project I found it very difficult to understand parts of the code base because of the inherently global nature of our code. It also made working on submodules very difficult. Since we can only open up a single repository per process, you need to launch a child process in order to process a submodule. But you also need to be able to communicate other stateful information to the children processes so that the submodules know how best to format their output or match against a pathspec...it ends up feeling like layering on hack after hack. What I would really like to do, is to have the ability to have a repository object so that I can open a submodule in-process.
Before this becomes a reality for all commands, much of the library code would need to be refactored in order to work purely on handles instead of global state. As it turned out, ls-files is a pretty simple command and doesn't have *too* many dependencies. The biggest thing that needed to be changed was piping through an index into a couple library routines so that they don't inherently rely on '
the_index'. A few of these changes I've sent out and can be found at 'origin/bw/pathspec-sans-the-index' and 'origin/bw/dir-c-stops-relying-on-the-index' which this series is based on.Patches 1-16 are refactorings to prepare either library code or ls-files itself to be ready to handle passing around an index struct. Patches 17-22 introduce a repository struct and change a couple of things about how submodule caches work (getting submodule information from .gitmodules). And Patch 23 converts ls-files to use a repository struct.
The most interesting part of the series is from 17-23. And 1-16 could be taken as is without the rest of the series.
This is still very much in a WIP state, though it does pass all tests. What I'm hoping for here is to get a discussion started about the feasibility of a change like this and hopefully to get the ball rolling. Is this a direction we want to move in? Is it worth the pain?
Thanks for taking the time to look at this and entertain my insane ideas :)
Brandon Williams (23):
convert: convertget_cached_convert_stats_asciito take an index
convert: convertcrlf_to_gitto take an index
convert: convertconvert_to_git_filter_fdto take an index
convert: convertconvert_to_gitto take an index
convert: convertrenormalize_bufferto take an index
tree: convertread_treeto take an index parameter
ls-files: convertoverlay_tree_on_cacheto take an index
ls-files: convertwrite_eolinfoto take an index
ls-files: convertshow_killed_filesto take an index
ls-files: convertshow_other_filesto take an index
ls-files: convertshow_ru_infoto take an index
ls-files: convertce_excludedto take an index
ls-files: convertprune_cacheto take an index
ls-files: convertshow_filesto take an index
ls-files: factor out debug info into a function
ls-files: factor out tag calculation
repo: introduce new repository object
repo: addindex_stateto struct repo
repo: add per repo config
submodule-config: refactor to allow for multiple submodule_cache's
repo: addrepo_read_gitmodules
submodule: addis_submodule_active
ls-files: use repository objectMakefile | 1 + apply.c | 2 +- builtin/blame.c | 2 +- builtin/commit.c | 3 +- builtin/ls-files.c | 348 ++++++++++++++++----------------- cache.h | 4 +- combine-diff.c | 2 +- config.c | 2 +- convert.c | 31 +-- convert.h | 19 +- diff.c | 6 +- dir.c | 2 +- git.c | 2 +- ll-merge.c | 2 +- merge-recursive.c | 4 +- repo.c | 112 +++++++++++ repo.h | 22 +++ sha1_file.c | 6 +- submodule-config.c | 40 +++- submodule-config.h | 10 + submodule.c | 51 +++++ submodule.h | 2 + t/t3007-ls-files-recurse-submodules.sh | 39 ++++ tree.c | 28 ++- tree.h | 3 +- 25 files changed, 513 insertions(+), 230 deletions(-) create mode 100644 repo.c create mode 100644 repo.h
Quoted 11 lines · Ben Peart
Glad to see you tackling this. This is definitely a step in the right direction.
I realize that it will take a lot of work and that intermediate steps may just be pushing it the global state one level higher but eventually it would be great to see an entire code path global state free!
I'm personally interested because reducing the reliance on global state also helps us in our performance work as it makes it more possible to use threading to scale up the performance.
I'm glad that more people than just me are interesting in moving in this direction. It's definitely going to take some work and hopefully I can convince some more people to help achieve this goal :D
Quoted 94 lines
Ben
Quoted 90 lines · Brandon Williams
When I first started working on the git project I found it very difficult to understand parts of the code base because of the inherently global nature of our code. It also made working on submodules very difficult. Since we can only open up a single repository per process, you need to launch a child process in order to process a submodule. But you also need to be able to communicate other stateful information to the children processes so that the submodules know how best to format their output or match against a pathspec...it ends up feeling like layering on hack after hack. What I would really like to do, is to have the ability to have a repository object so that I can open a submodule in-process.
Before this becomes a reality for all commands, much of the library code would need to be refactored in order to work purely on handles instead of global state. As it turned out, ls-files is a pretty simple command and doesn't have *too* many dependencies. The biggest thing that needed to be changed was piping through an index into a couple library routines so that they don't inherently rely on '
the_index'. A few of these changes I've sent out and can be found at 'origin/bw/pathspec-sans-the-index' and 'origin/bw/dir-c-stops-relying-on-the-index' which this series is based on.Patches 1-16 are refactorings to prepare either library code or ls-files itself to be ready to handle passing around an index struct. Patches 17-22 introduce a repository struct and change a couple of things about how submodule caches work (getting submodule information from .gitmodules). And Patch 23 converts ls-files to use a repository struct.
The most interesting part of the series is from 17-23. And 1-16 could be taken as is without the rest of the series.
This is still very much in a WIP state, though it does pass all tests. What I'm hoping for here is to get a discussion started about the feasibility of a change like this and hopefully to get the ball rolling. Is this a direction we want to move in? Is it worth the pain?
Thanks for taking the time to look at this and entertain my insane ideas :)
Brandon Williams (23):
convert: convertget_cached_convert_stats_asciito take an index
convert: convertcrlf_to_gitto take an index
convert: convertconvert_to_git_filter_fdto take an index
convert: convertconvert_to_gitto take an index
convert: convertrenormalize_bufferto take an index
tree: convertread_treeto take an index parameter
ls-files: convertoverlay_tree_on_cacheto take an index
ls-files: convertwrite_eolinfoto take an index
ls-files: convertshow_killed_filesto take an index
ls-files: convertshow_other_filesto take an index
ls-files: convertshow_ru_infoto take an index
ls-files: convertce_excludedto take an index
ls-files: convertprune_cacheto take an index
ls-files: convertshow_filesto take an index
ls-files: factor out debug info into a function
ls-files: factor out tag calculation
repo: introduce new repository object
repo: addindex_stateto struct repo
repo: add per repo config
submodule-config: refactor to allow for multiple submodule_cache's
repo: addrepo_read_gitmodules
submodule: addis_submodule_active
ls-files: use repository objectMakefile | 1 + apply.c | 2 +- builtin/blame.c | 2 +- builtin/commit.c | 3 +- builtin/ls-files.c | 348 ++++++++++++++++----------------- cache.h | 4 +- combine-diff.c | 2 +- config.c | 2 +- convert.c | 31 +-- convert.h | 19 +- diff.c | 6 +- dir.c | 2 +- git.c | 2 +- ll-merge.c | 2 +- merge-recursive.c | 4 +- repo.c | 112 +++++++++++ repo.h | 22 +++ sha1_file.c | 6 +- submodule-config.c | 40 +++- submodule-config.h | 10 + submodule.c | 51 +++++ submodule.h | 2 + t/t3007-ls-files-recurse-submodules.sh | 39 ++++ tree.c | 28 ++- tree.h | 3 +- 25 files changed, 513 insertions(+), 230 deletions(-)create mode 100644
repo.c
create mode 100644repo.h
Signature · 1 line
Brandon Williams
Quoted 4 lines · Brandon Williams
This is still very much in a WIP state, though it does pass all tests. What I'm hoping for here is to get a discussion started about the feasibility of a change like this and hopefully to get the ball rolling. Is this a direction we want to move in? Is it worth the pain?
I would be really happy to see this series land eventually.
The introduction of a repo object will deliver performance at a higher level, such as
* (remarked by Ben): enabling of threading
* submodules do not need to spawn processes
* I would imagine that developer velocity will go up by having less global state in the long run.
Thanks for working on this.
Stefan
Hi Stefan,
Quoted 7 lines · Stefan Beller
Quoted 4 lines · Brandon Williams
This is still very much in a WIP state, though it does pass all tests. What I'm hoping for here is to get a discussion started about the feasibility of a change like this and hopefully to get the ball rolling. Is this a direction we want to move in? Is it worth the pain?
Quoted 8 lines
I would be really happy to see this series land eventually.
The introduction of a repo object will deliver performance at a higher level, such as
* (remarked by Ben): enabling of threading
* submodules do not need to spawn processes
* I would imagine that developer velocity will go up by having less global state in the long run.
* avoiding really ugly hacks such as save_env_before_alias() which are even incomplete, as git_dir is not reset upon restore_env().
Ciao,
Dscho
Quoted 7 lines · Brandon Williams
This is still very much in a WIP state, though it does pass all tests. What I'm hoping for here is to get a discussion started about the feasibility of a change like this and hopefully to get the ball rolling. Is this a direction we want to move in? Is it worth the pain?
Thanks for taking the time to look at this and entertain my insane ideas :)
I haven't had time to read the patches yet, but the goal I think is worthy, and worth the pain. The ultimate goal allows us to more easily write submodule features (some of which are quite difficult today, due to having to pass in all kinds of state to the submodule, and launching a process is expensive when you have a lot of them).
I can't say much about this particular code yet, but I hope to be able to look in more detail in the next few days.
Thanks,
Jake
Quoted 4 lines · Brandon Williams
This is still very much in a WIP state, though it does pass all tests. What I'm hoping for here is to get a discussion started about the feasibility of a change like this and hopefully to get the ball rolling. Is this a direction we want to move in? Is it worth the pain?
I haven't spent a lot of time looking at the patches, but I like this idea. I expect that moving in this direction will help us avoid a lot of globals and move us in a direction of more re-entrant code. I can think of a lot of things that could really benefit from this.
Signature · 3 lines
brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: https://keybase.io/bk2204
Quoted 10 lines · Brandon Williams
When I first started working on the git project I found it very difficult to understand parts of the code base because of the inherently global nature of our code. It also made working on submodules very difficult. Since we can only open up a single repository per process, you need to launch a child process in order to process a submodule. But you also need to be able to communicate other stateful information to the children processes so that the submodules know how best to format their output or match against a pathspec...it ends up feeling like layering on hack after hack. What I would really like to do, is to have the ability to have a repository object so that I can open a submodule in-process.
We could always buy in fully to the multi-process model and just implement a generic RPC protocol between the parent and submodule gits. Does CORBA still exist?
(No, I am not serious about any of that).
Quoted 4 lines
This is still very much in a WIP state, though it does pass all tests. What I'm hoping for here is to get a discussion started about the feasibility of a change like this and hopefully to get the ball rolling. Is this a direction we want to move in? Is it worth the pain?
I think the really painful part is going to be all of the system calls that rely on global state provided by the OS. Like, say, every filesystem call that expects to find working tree files without prepending the working tree path.
That said, even if we never reached the point where we could handle all submodule requests in-process, I think sticking the repo-related global state in a struct certainly could not hurt general readability. So it's a good direction regardless of whether we take it all the way.
-Peff
Quoted 24 lines · Jeff King
Quoted 10 lines · Brandon Williams
When I first started working on the git project I found it very difficult to understand parts of the code base because of the inherently global nature of our code. It also made working on submodules very difficult. Since we can only open up a single repository per process, you need to launch a child process in order to process a submodule. But you also need to be able to communicate other stateful information to the children processes so that the submodules know how best to format their output or match against a pathspec...it ends up feeling like layering on hack after hack. What I would really like to do, is to have the ability to have a repository object so that I can open a submodule in-process.
We could always buy in fully to the multi-process model and just implement a generic RPC protocol between the parent and submodule gits. Does CORBA still exist?
(No, I am not serious about any of that).
Quoted 4 lines
This is still very much in a WIP state, though it does pass all tests. What I'm hoping for here is to get a discussion started about the feasibility of a change like this and hopefully to get the ball rolling. Is this a direction we want to move in? Is it worth the pain?
Quoted 4 lines
I think the really painful part is going to be all of the system calls that rely on global state provided by the OS. Like, say, every filesystem call that expects to find working tree files without prepending the working tree path.
Yeah that's going to be one of the more challenging things to deal with...
Quoted 5 lines
That said, even if we never reached the point where we could handle all submodule requests in-process, I think sticking the repo-related global state in a struct certainly could not hurt general readability. So it's a good direction regardless of whether we take it all the way.
Glad you think so!
Signature · 1 line
Brandon Williams
Quoted 7 lines · Brandon Williams
Quoted 4 lines · Jeff King
That said, even if we never reached the point where we could handle all submodule requests in-process, I think sticking the repo-related global state in a struct certainly could not hurt general readability. So it's a good direction regardless of whether we take it all the way.
Glad you think so!
Yup, what Jeff said ;-)
Quoted 13 lines · Jeff King
Quoted 10 lines · Brandon Williams
When I first started working on the git project I found it very difficult to understand parts of the code base because of the inherently global nature of our code. It also made working on submodules very difficult. Since we can only open up a single repository per process, you need to launch a child process in order to process a submodule. But you also need to be able to communicate other stateful information to the children processes so that the submodules know how best to format their output or match against a pathspec...it ends up feeling like layering on hack after hack. What I would really like to do, is to have the ability to have a repository object so that I can open a submodule in-process.
Quoted 5 lines
We could always buy in fully to the multi-process model and just implement a generic RPC protocol between the parent and submodule gits. Does CORBA still exist?
(No, I am not serious about any of that).
CORBA or not, submodule IPC is a real pain. That was what I felt reading the super-prefix changes a few weeks ago. Some operations might benefit from staying in the same process, but probably not all (and we lose process protection, which sometimes is a good thing)
Quoted 10 lines
Quoted 4 lines
This is still very much in a WIP state, though it does pass all tests. What I'm hoping for here is to get a discussion started about the feasibility of a change like this and hopefully to get the ball rolling. Is this a direction we want to move in? Is it worth the pain?
I think the really painful part is going to be all of the system calls that rely on global state provided by the OS. Like, say, every filesystem call that expects to find working tree files without prepending the working tree path.
Quoted 4 lines
That said, even if we never reached the point where we could handle all submodule requests in-process, I think sticking the repo-related global state in a struct certainly could not hurt general readability. So it's a good direction regardless of whether we take it all the way.
I doubt we would reach the point where libgit.a can handle all submodule operations in-process either. That would put libgit.a in a direct competitor position with libgit2. I do hope though that having clearer/modular data structure will improve readability, not hurt it (e.g. you see the data model and could largely guess how the code interacts before digging deep in).
Signature · 1 line
Duy
Quoted 35 lines · Duy Nguyen
Quoted 13 lines · Jeff King
Quoted 10 lines · Brandon Williams
When I first started working on the git project I found it very difficult to understand parts of the code base because of the inherently global nature of our code. It also made working on submodules very difficult. Since we can only open up a single repository per process, you need to launch a child process in order to process a submodule. But you also need to be able to communicate other stateful information to the children processes so that the submodules know how best to format their output or match against a pathspec...it ends up feeling like layering on hack after hack. What I would really like to do, is to have the ability to have a repository object so that I can open a submodule in-process.
Quoted 5 lines
We could always buy in fully to the multi-process model and just implement a generic RPC protocol between the parent and submodule gits. Does CORBA still exist?
(No, I am not serious about any of that).
CORBA or not, submodule IPC is a real pain. That was what I felt reading the super-prefix changes a few weeks ago. Some operations might benefit from staying in the same process, but probably not all (and we lose process protection, which sometimes is a good thing)
Quoted 10 lines
Quoted 4 lines
This is still very much in a WIP state, though it does pass all tests. What I'm hoping for here is to get a discussion started about the feasibility of a change like this and hopefully to get the ball rolling. Is this a direction we want to move in? Is it worth the pain?
I think the really painful part is going to be all of the system calls that rely on global state provided by the OS. Like, say, every filesystem call that expects to find working tree files without prepending the working tree path.
Quoted 8 lines
Quoted 4 lines
That said, even if we never reached the point where we could handle all submodule requests in-process, I think sticking the repo-related global state in a struct certainly could not hurt general readability. So it's a good direction regardless of whether we take it all the way.
I doubt we would reach the point where libgit.a can handle all submodule operations in-process either. That would put libgit.a in a direct competitor position with libgit2.
Wouldn't that be a good thing? We already have some users (e.g. Microsoft) choosing not to use libgit and instead use git.git because the latter is generally more mature, if git.git gains more libgit features without harming other things it'll be more useful to more people.
Quoted 9 lines · Ævar Arnfjörð Bjarmason
Quoted 8 lines
Quoted 4 lines
That said, even if we never reached the point where we could handle all submodule requests in-process, I think sticking the repo-related global state in a struct certainly could not hurt general readability. So it's a good direction regardless of whether we take it all the way.
I doubt we would reach the point where libgit.a can handle all submodule operations in-process either. That would put libgit.a in a direct competitor position with libgit2.
Quoted 5 lines
Wouldn't that be a good thing? We already have some users (e.g. Microsoft) choosing not to use libgit and instead use git.git because the latter is generally more mature, if git.git gains more libgit features without harming other things it'll be more useful to more people.
Whether it's a good thing might depend on view point. Similar to linux kernel, we might want some freedom to break ABI and refactor the code. But I think it's too early to discuss this right now. There's lots of work (the "die()" minefield comes to mind) before we reach the point where libgit.a may be good enough for external use.
Signature · 1 line
Duy
Quoted 17 lines · Duy Nguyen
Quoted 9 lines · Ævar Arnfjörð Bjarmason
Quoted 8 lines
Quoted 4 lines
That said, even if we never reached the point where we could handle all submodule requests in-process, I think sticking the repo-related global state in a struct certainly could not hurt general readability. So it's a good direction regardless of whether we take it all the way.
I doubt we would reach the point where libgit.a can handle all submodule operations in-process either. That would put libgit.a in a direct competitor position with libgit2.
Quoted 5 lines
Wouldn't that be a good thing? We already have some users (e.g. Microsoft) choosing not to use libgit and instead use git.git because the latter is generally more mature, if git.git gains more libgit features without harming other things it'll be more useful to more people.
Quoted 5 lines
Whether it's a good thing might depend on view point. Similar to linux kernel, we might want some freedom to break ABI and refactor the code. But I think it's too early to discuss this right now. There's lots of work (the "
die()" minefield comes to mind) before we reach the point where libgit.a may be good enough for external use.
My personal end goal is making the code easier to parse and to make submodule work easier. I know I've only worked on the project for a short time but I already regret some of the submodule changes that I've made because they are straight up hacks which I want to eradicate sooner rather than later (just look at my patches to get 'git grep' to handle submodules...).
I wasn't thinking about libgit2 and don't really haven't thought too much about the possibility of being in competition with that project.
Signature · 1 line
Brandon Williams