Re: [PATCH 01/15] contrib: add cocci script to replace index compat macros
On Tue, Jun 19, 2018 at 5:21 PM Derrick Stolee wrote: > > On 6/19/2018 10:51 AM, Duy Nguyen wrote: > > Do we run 'make coccicheck' > > automatically somewhere? If true, I need to move this script elsewhere > > because it's meant to run manually. You run it when you intend to do > > more manual fixups afterwards. For builtin/, I think I'll wait until > > 'struct repository *' conversion is complete then maybe fix them one > > by one. > > I don't think it is part of the CI runs, but some community members run > this themselves on 'next' and 'master'. Travis CI does run 'make coccicheck' already, that's the "StaticAnalysis" build job. Alas, it's not particularly useful as it is, because Coccinelle's and in turn 'make coccicheck's exit code only indicates that Coccinelle managed to finish its analysis without any errors (e.g. no unknown --options, no missing files, no syntax errors in the semantic patches, etc.), but it doesn't indicate whether it found any undesired code patterns to transform or not. I have been sitting on two patches implementing two different approaches to improve this situation for several months now (sorry :) I think I'll just pick the shorter-simpler of the two, and submit it shortly.
Re: [PATCH 01/15] contrib: add cocci script to replace index compat macros
On 6/19/2018 10:51 AM, Duy Nguyen wrote: On Tue, Jun 19, 2018 at 1:41 PM Derrick Stolee wrote: Duy, Here is the patch that was generated by `make coccicheck`. Thanks, -Stolee -->8-- --- builtin/add.c Ah right. This is on purpose. I think I mentioned in the commit message that builtin/ is not touched. Do we run 'make coccicheck' automatically somewhere? If true, I need to move this script elsewhere because it's meant to run manually. You run it when you intend to do more manual fixups afterwards. For builtin/, I think I'll wait until 'struct repository *' conversion is complete then maybe fix them one by one. I don't think it is part of the CI runs, but some community members run this themselves on 'next' and 'master'. I'm CC'ing Szeder, as he has contributed a fix to my own Coccinelle script [1]. [1] https://public-inbox.org/git/20180430093153.13040-1-szeder@gmail.com/ [PATCH] coccinelle: avoid wrong transformation suggestions from commit.cocci
Re: [PATCH 01/15] contrib: add cocci script to replace index compat macros
On Tue, Jun 19, 2018 at 1:41 PM Derrick Stolee wrote: > > Duy, > > Here is the patch that was generated by `make coccicheck`. > > Thanks, > -Stolee > > -->8-- > > --- builtin/add.c Ah right. This is on purpose. I think I mentioned in the commit message that builtin/ is not touched. Do we run 'make coccicheck' automatically somewhere? If true, I need to move this script elsewhere because it's meant to run manually. You run it when you intend to do more manual fixups afterwards. For builtin/, I think I'll wait until 'struct repository *' conversion is complete then maybe fix them one by one. > +++ /tmp/cocci-output-206193-4c91ec-add.c > @@ -38,13 +38,13 @@ static void chmod_pathspec(struct pathsp > { > int i; > > - for (i = 0; i < active_nr; i++) { > - struct cache_entry *ce = active_cache[i]; > + for (i = 0; i < the_index.cache_nr; i++) { > + struct cache_entry *ce = the_index.cache[i]; > > if (pathspec && !ce_path_match(&the_index, ce, pathspec, > NULL)) > continue; > > - if (chmod_cache_entry(ce, flip) < 0) > + if (chmod_index_entry(&the_index, ce, flip) < 0) > fprintf(stderr, "cannot chmod %cx '%s'\n", flip, > ce->name); > } > } -- Duy
Re: [PATCH 01/15] contrib: add cocci script to replace index compat macros
Duy, Here is the patch that was generated by `make coccicheck`. Thanks, -Stolee -->8-- --- builtin/add.c +++ /tmp/cocci-output-206193-4c91ec-add.c @@ -38,13 +38,13 @@ static void chmod_pathspec(struct pathsp { int i; - for (i = 0; i < active_nr; i++) { - struct cache_entry *ce = active_cache[i]; + for (i = 0; i < the_index.cache_nr; i++) { + struct cache_entry *ce = the_index.cache[i]; if (pathspec && !ce_path_match(&the_index, ce, pathspec, NULL)) continue; - if (chmod_cache_entry(ce, flip) < 0) + if (chmod_index_entry(&the_index, ce, flip) < 0) fprintf(stderr, "cannot chmod %cx '%s'\n", flip, ce->name); } } @@ -129,8 +129,8 @@ static int renormalize_tracked_files(con { int i, retval = 0; - for (i = 0; i < active_nr; i++) { - struct cache_entry *ce = active_cache[i]; + for (i = 0; i < the_index.cache_nr; i++) { + struct cache_entry *ce = the_index.cache[i]; if (ce_stage(ce)) continue; /* do not touch unmerged paths */ @@ -138,7 +138,8 @@ static int renormalize_tracked_files(con continue; /* do not touch non blobs */ if (pathspec && !ce_path_match(&the_index, ce, pathspec, NULL)) continue; - retval |= add_file_to_cache(ce->name, flags | HASH_RENORMALIZE); + retval |= add_file_to_index(&the_index, ce->name, + flags | HASH_RENORMALIZE); } return retval; @@ -230,7 +231,7 @@ static int edit_patch(int argc, const ch git_config(git_diff_basic_config, NULL); /* no "diff" UI options */ - if (read_cache() < 0) + if (read_index(&the_index) < 0) die(_("Could not read the index")); init_revisions(&rev, the_repository, prefix); @@ -445,7 +446,7 @@ int cmd_add(int argc, const char **argv, return 0; } - if (read_cache() < 0) + if (read_index(&the_index) < 0) die(_("index file corrupt")); die_in_unpopulated_submodule(&the_index, prefix); --- builtin/am.c +++ /tmp/cocci-output-206198-3fcbd5-am.c @@ -1144,7 +1144,7 @@ static void refresh_and_write_cache(void struct lock_file lock_file = LOCK_INIT; hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR); - refresh_cache(REFRESH_QUIET); + refresh_index(&the_index, REFRESH_QUIET, NULL, NULL, NULL); if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK)) die(_("unable to write index file")); } @@ -1505,8 +1505,8 @@ static int run_apply(const struct am_sta if (index_file) { /* Reload index as apply_all_patches() will have modified it. */ - discard_cache(); - read_cache_from(index_file); + discard_index(&the_index); + read_index_from(&the_index, index_file, get_git_dir()); } return 0; @@ -1548,8 +1548,8 @@ static int fall_back_threeway(const stru if (build_fake_ancestor(state, index_path)) return error("could not build fake ancestor"); - discard_cache(); - read_cache_from(index_path); + discard_index(&the_index); + read_index_from(&the_index, index_path, get_git_dir()); if (write_index_as_tree(&orig_tree, &the_index, index_path, 0, NULL)) return error(_("Repository lacks necessary blobs to fall back on 3-way merge.")); @@ -1581,8 +1581,8 @@ static int fall_back_threeway(const stru say(state, stdout, _("Falling back to patching base and 3-way merge...")); - discard_cache(); - read_cache(); + discard_index(&the_index); + read_index(&the_index); /* * This is not so wrong. Depending on which base we picked, orig_tree @@ -1886,7 +1886,7 @@ static void am_resolve(struct am_state * die_user_resolve(state); } - if (unmerged_cache()) { + if (unmerged_index(&the_index)) { printf_ln(_("You still have unmerged paths in your index.\n" "You should 'git add' each file with resolved conflicts to mark them as such.\n" "You might run `git rm` on a file to accept \"deleted by them\" for it.")); @@ -1925,7 +1925,7 @@ static int fast_forward_to(struct tree * hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR); - refresh_cache(REFRESH_QUIET); + refresh_index(&the_index, REFRESH_QUIET, NULL, NULL, NULL); memset(&opts, 0, sizeof(opts)); opts.head_idx = 1; @@ -2000,7 +2000,7 @@ static int clean_index(const struct obje if (!remote_tree) return error(_("Could not parse object '%s'."), oid_to_hex(remote)); - read_cache_unmerged(); + read_
Re: [PATCH 01/15] contrib: add cocci script to replace index compat macros
On 6/16/2018 1:41 AM, Nguyễn Thái Ngọc Duy wrote: Index compat macros are going to be removed to expose the_index and then reorganized to use the right index instead of simply the_index because sometimes we want to use a different index. This cocci script can help with the first step. It can be used later on on even builtin/ when we start to reorganize code in there, but for now this is the script that performs all the conversion outside builtin/ I pulled your code and ran `make coccicheck` and got quite a large patch as a result. Perhaps reorder the commits in your large patch with this one at the end? It's helpful to see what your mechanical changes are going to be, but let's save it for when `make coccicheck` is stable. Signed-off-by: Nguyễn Thái Ngọc Duy --- contrib/coccinelle/index-compat.cocci | 184 ++ 1 file changed, 184 insertions(+) create mode 100644 contrib/coccinelle/index-compat.cocci diff --git a/contrib/coccinelle/index-compat.cocci b/contrib/coccinelle/index-compat.cocci new file mode 100644 index 00..ca46711eb6 --- /dev/null +++ b/contrib/coccinelle/index-compat.cocci @@ -0,0 +1,184 @@ +@@ +@@ +-active_cache ++the_index.cache + +@@ +@@ +-active_nr ++the_index.cache_nr + +@@ +@@ +-active_alloc ++the_index.cache_alloc + +@@ +@@ +-active_cache_changed ++the_index.cache_changed + +@@ +@@ +-active_cache_tree ++the_index.cache_tree + +@@ +@@ +- read_cache() ++ read_index(&the_index) + +@@ +expression path; +@@ +- read_cache_from(path) ++ read_index_from(&the_index, path, get_git_dir()) + +@@ +expression pathspec; +@@ +- read_cache_preload(pathspec) ++ read_index_preload(&the_index, pathspec) + +@@ +@@ +- is_cache_unborn() ++ is_index_unborn(&the_index) + +@@ +@@ +- read_cache_unmerged() ++ read_index_unmerged(&the_index) + +@@ +@@ +- discard_cache() ++ discard_index(&the_index) + +@@ +@@ +- unmerged_cache() ++ unmerged_index(&the_index) + +@@ +expression name; +expression namelen; +@@ +- cache_name_pos(name, namelen) ++ index_name_pos(&the_index, name, namelen) + +@@ +expression ce; +expression option; +@@ +- add_cache_entry(ce, option) ++ add_index_entry(&the_index, ce, option) + +@@ +expression pos; +expression new_name; +@@ +- rename_cache_entry_at(pos, new_name) ++ rename_index_entry_at(&the_index, pos, new_name) + +@@ +expression pos; +@@ +- remove_cache_entry_at(pos) ++ remove_index_entry_at(&the_index, pos) + +@@ +expression path; +@@ +- remove_file_from_cache(path) ++ remove_file_from_index(&the_index, path) + +@@ +expression path; +expression st; +expression flags; +@@ +- add_to_cache(path, st, flags) ++ add_to_index(&the_index, path, st, flags) + +@@ +expression path; +expression flags; +@@ +- add_file_to_cache(path, flags) ++ add_file_to_index(&the_index, path, flags) + +@@ +expression ce; +expression flip; +@@ +-chmod_cache_entry(ce, flip) ++chmod_index_entry(&the_index, ce, flip) + +@@ +expression flags; +@@ +-refresh_cache(flags) ++refresh_index(&the_index, flags, NULL, NULL, NULL) + +@@ +expression ce; +expression st; +expression options; +@@ +-ce_match_stat(ce, st, options) ++ie_match_stat(&the_index, ce, st, options) + +@@ +expression ce; +expression st; +expression options; +@@ +-ce_modified(ce, st, options) ++ie_modified(&the_index, ce, st, options) + +@@ +expression name; +expression namelen; +@@ +-cache_dir_exists(name, namelen) ++index_dir_exists(&the_index, name, namelen) + +@@ +expression name; +expression namelen; +expression igncase; +@@ +-cache_file_exists(name, namelen, igncase) ++index_file_exists(&the_index, name, namelen, igncase) + +@@ +expression name; +expression namelen; +@@ +-cache_name_is_other(name, namelen) ++index_name_is_other(&the_index, name, namelen) + +@@ +@@ +-resolve_undo_clear() ++resolve_undo_clear_index(&the_index) + +@@ +expression at; +@@ +-unmerge_cache_entry_at(at) ++unmerge_index_entry_at(&the_index, at) + +@@ +expression pathspec; +@@ +-unmerge_cache(pathspec) ++unmerge_index(&the_index, pathspec) + +@@ +expression path; +expression sz; +@@ +-read_blob_data_from_cache(path, sz) ++read_blob_data_from_index(&the_index, path, sz)
[PATCH 01/15] contrib: add cocci script to replace index compat macros
Index compat macros are going to be removed to expose the_index and then reorganized to use the right index instead of simply the_index because sometimes we want to use a different index. This cocci script can help with the first step. It can be used later on on even builtin/ when we start to reorganize code in there, but for now this is the script that performs all the conversion outside builtin/ Signed-off-by: Nguyễn Thái Ngọc Duy --- contrib/coccinelle/index-compat.cocci | 184 ++ 1 file changed, 184 insertions(+) create mode 100644 contrib/coccinelle/index-compat.cocci diff --git a/contrib/coccinelle/index-compat.cocci b/contrib/coccinelle/index-compat.cocci new file mode 100644 index 00..ca46711eb6 --- /dev/null +++ b/contrib/coccinelle/index-compat.cocci @@ -0,0 +1,184 @@ +@@ +@@ +-active_cache ++the_index.cache + +@@ +@@ +-active_nr ++the_index.cache_nr + +@@ +@@ +-active_alloc ++the_index.cache_alloc + +@@ +@@ +-active_cache_changed ++the_index.cache_changed + +@@ +@@ +-active_cache_tree ++the_index.cache_tree + +@@ +@@ +- read_cache() ++ read_index(&the_index) + +@@ +expression path; +@@ +- read_cache_from(path) ++ read_index_from(&the_index, path, get_git_dir()) + +@@ +expression pathspec; +@@ +- read_cache_preload(pathspec) ++ read_index_preload(&the_index, pathspec) + +@@ +@@ +- is_cache_unborn() ++ is_index_unborn(&the_index) + +@@ +@@ +- read_cache_unmerged() ++ read_index_unmerged(&the_index) + +@@ +@@ +- discard_cache() ++ discard_index(&the_index) + +@@ +@@ +- unmerged_cache() ++ unmerged_index(&the_index) + +@@ +expression name; +expression namelen; +@@ +- cache_name_pos(name, namelen) ++ index_name_pos(&the_index, name, namelen) + +@@ +expression ce; +expression option; +@@ +- add_cache_entry(ce, option) ++ add_index_entry(&the_index, ce, option) + +@@ +expression pos; +expression new_name; +@@ +- rename_cache_entry_at(pos, new_name) ++ rename_index_entry_at(&the_index, pos, new_name) + +@@ +expression pos; +@@ +- remove_cache_entry_at(pos) ++ remove_index_entry_at(&the_index, pos) + +@@ +expression path; +@@ +- remove_file_from_cache(path) ++ remove_file_from_index(&the_index, path) + +@@ +expression path; +expression st; +expression flags; +@@ +- add_to_cache(path, st, flags) ++ add_to_index(&the_index, path, st, flags) + +@@ +expression path; +expression flags; +@@ +- add_file_to_cache(path, flags) ++ add_file_to_index(&the_index, path, flags) + +@@ +expression ce; +expression flip; +@@ +-chmod_cache_entry(ce, flip) ++chmod_index_entry(&the_index, ce, flip) + +@@ +expression flags; +@@ +-refresh_cache(flags) ++refresh_index(&the_index, flags, NULL, NULL, NULL) + +@@ +expression ce; +expression st; +expression options; +@@ +-ce_match_stat(ce, st, options) ++ie_match_stat(&the_index, ce, st, options) + +@@ +expression ce; +expression st; +expression options; +@@ +-ce_modified(ce, st, options) ++ie_modified(&the_index, ce, st, options) + +@@ +expression name; +expression namelen; +@@ +-cache_dir_exists(name, namelen) ++index_dir_exists(&the_index, name, namelen) + +@@ +expression name; +expression namelen; +expression igncase; +@@ +-cache_file_exists(name, namelen, igncase) ++index_file_exists(&the_index, name, namelen, igncase) + +@@ +expression name; +expression namelen; +@@ +-cache_name_is_other(name, namelen) ++index_name_is_other(&the_index, name, namelen) + +@@ +@@ +-resolve_undo_clear() ++resolve_undo_clear_index(&the_index) + +@@ +expression at; +@@ +-unmerge_cache_entry_at(at) ++unmerge_index_entry_at(&the_index, at) + +@@ +expression pathspec; +@@ +-unmerge_cache(pathspec) ++unmerge_index(&the_index, pathspec) + +@@ +expression path; +expression sz; +@@ +-read_blob_data_from_cache(path, sz) ++read_blob_data_from_index(&the_index, path, sz) -- 2.18.0.rc0.333.g22e6ee6cdf