# [PATCH] Generalize and libify index_is_dirty() to index_differs_from(...) 4 messages from 2009-02-09 to 2009-02-10. Participants: Stephan Beyer, Jeff King, Junio C Hamano. Thread: https://gitlist.dev/t/17687 ## Stephan Beyer, 2009-02-09 23:40 Subject: [PATCH] Generalize and libify index_is_dirty() to index_differs_from(...) Message-ID: <1234222843-15577-1-git-send-email-s-beyer@gmx.net> URL: https://gitlist.dev/e/1234222843-15577-1-git-send-email-s-beyer%40gmx.net ``` index_is_dirty() in builtin-revert.c checks if the index is dirty. This patch generalizes this function to check if the index differs from a revision, i.e. the former index_is_dirty() behavior can now be achieved by index_differs_from("HEAD", 0). The second argument "diff_flags" allows to set further diff option flags like DIFF_OPT_IGNORE_SUBMODULES. See DIFF_OPT_* macros in diff.h for a list. index_differs_from() seems to be useful for more than builtin-revert.c, so it is moved into revision.c and also used in builtin-commit.c to remove code duplication. Yet to mention: - "rev.abbrev = 0;" in builtin-commit.c can be safely removed. This has no impact on performance or functioning of neither setup_revisions() nor run_diff_index(). - rev.pending.objects is free()d because this fixes a leak. (Also see 295dd2ad "Fix memory leak in traverse_commit_list") Mentored-by: Daniel Barkalow Mentored-by: Christian Couder Signed-off-by: Stephan Beyer --- This is one of the sequencer-preparing patches. (The function is used in sequencer several times, most of the time with diff_flags set to DIFF_OPT_IGNORE_SUBMODULES.) Alex is on Cc because he introduced the "Is commitable?" (i.e. "Is index dirty?") part in builtin-commit.c. Peff is on Cc because he introduced index_is_dirty() in builtin-revert.c. builtin-commit.c | 13 ++----------- builtin-revert.c | 13 +------------ revision.c | 15 +++++++++++++++ revision.h | 2 ++ 4 files changed, 20 insertions(+), 23 deletions(-) diff --git a/builtin-commit.c b/builtin-commit.c index d6a3a62..46e649c 100644 --- a/builtin-commit.c +++ b/builtin-commit.c @@ -561,7 +561,6 @@ static int prepare_to_commit(const char *index_file, const char *prefix) commitable = run_status(fp, index_file, prefix, 1); wt_status_use_color = saved_color_setting; } else { - struct rev_info rev; unsigned char sha1[20]; const char *parent = "HEAD"; @@ -573,16 +572,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix) if (get_sha1(parent, sha1)) commitable = !!active_nr; - else { - init_revisions(&rev, ""); - rev.abbrev = 0; - setup_revisions(0, NULL, &rev, parent); - DIFF_OPT_SET(&rev.diffopt, QUIET); - DIFF_OPT_SET(&rev.diffopt, EXIT_WITH_STATUS); - run_diff_index(&rev, 1 /* cached */); - - commitable = !!DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES); - } + else + commitable = index_differs_from(parent, 0); } fclose(fp); diff --git a/builtin-revert.c b/builtin-revert.c index d48313c..d210150 100644 --- a/builtin-revert.c +++ b/builtin-revert.c @@ -223,17 +223,6 @@ static char *help_msg(const unsigned char *sha1) return helpbuf; } -static int index_is_dirty(void) -{ - struct rev_info rev; - init_revisions(&rev, NULL); - setup_revisions(0, NULL, &rev, "HEAD"); - DIFF_OPT_SET(&rev.diffopt, QUIET); - DIFF_OPT_SET(&rev.diffopt, EXIT_WITH_STATUS); - run_diff_index(&rev, 1); - return !!DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES); -} - static struct tree *empty_tree(void) { struct tree *tree = xcalloc(1, sizeof(struct tree)); @@ -279,7 +268,7 @@ static int revert_or_cherry_pick(int argc, const char **argv) } else { if (get_sha1("HEAD", head)) die ("You do not have a valid HEAD"); - if (index_is_dirty()) + if (index_differs_from("HEAD", 0)) die ("Dirty index: cannot %s", me); } discard_cache(); diff --git a/revision.c b/revision.c index 8603c14..de489db 100644 --- a/revision.c +++ b/revision.c @@ -1926,3 +1926,18 @@ struct commit *get_revision(struct rev_info *revs) graph_update(revs->graph, c); return c; } + +int index_differs_from(const char *def, int diff_flags) +{ + struct rev_info rev; + + init_revisions(&rev, NULL); + setup_revisions(0, NULL, &rev, def); + DIFF_OPT_SET(&rev.diffopt, QUIET); + DIFF_OPT_SET(&rev.diffopt, EXIT_WITH_STATUS); + rev.diffopt.flags |= diff_flags; + run_diff_index(&rev, 1); + if (rev.pending.alloc) + free(rev.pending.objects); + return (DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES) != 0); +} diff --git a/revision.h b/revision.h index 7cf8487..bc17949 100644 --- a/revision.h +++ b/revision.h @@ -164,4 +164,6 @@ enum commit_action { extern enum commit_action simplify_commit(struct rev_info *revs, struct commit *commit); +extern int index_differs_from(const char *def, int diff_flags); + #endif -- 1.6.2.rc0.458.g97dd ``` ## Jeff King, 2009-02-10 00:12 Subject: Re: [PATCH] Generalize and libify index_is_dirty() to index_differs_from(...) Message-ID: <20090210001220.GB5551@sigill.intra.peff.net> URL: https://gitlist.dev/e/20090210001220.GB5551%40sigill.intra.peff.net In-Reply-To: <1234222843-15577-1-git-send-email-s-beyer@gmx.net> ``` On Tue, Feb 10, 2009 at 12:40:43AM +0100, Stephan Beyer wrote: > Peff is on Cc because he introduced index_is_dirty() in > builtin-revert.c. Certainly it looks to me like a straight-forward libification of what I had added before. Acked-by: Jeff King -Peff ``` ## Junio C Hamano, 2009-02-10 05:16 Subject: Re: [PATCH] Generalize and libify index_is_dirty() to index_differs_from(...) Message-ID: <7v63jirhw8.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7v63jirhw8.fsf%40gitster.siamese.dyndns.org In-Reply-To: <1234222843-15577-1-git-send-email-s-beyer@gmx.net> ``` Stephan Beyer writes: > This is one of the sequencer-preparing patches. > (The function is used in sequencer several times, most of the time > with diff_flags set to DIFF_OPT_IGNORE_SUBMODULES.) > > Alex is on Cc because he introduced the "Is commitable?" (i.e. > "Is index dirty?") part in builtin-commit.c. > > Peff is on Cc because he introduced index_is_dirty() in > builtin-revert.c. > > builtin-commit.c | 13 ++----------- > builtin-revert.c | 13 +------------ > revision.c | 15 +++++++++++++++ > revision.h | 2 ++ > 4 files changed, 20 insertions(+), 23 deletions(-) It is a straightforward and clean restructuring, but please do not contaminate revision.[ch] with this function about "internally running diff-index". revision.[ch] is a library for revision/ancestry traversal and it is already one of the largest library-ish files. It does not know nor care about the index, and we want to keep it that way. Please keep its focus to revision traversal. Perhaps diff-lib.c would be a better home for your helper function. ``` ## Stephan Beyer, 2009-02-10 14:30 Subject: [PATCH v2] Generalize and libify index_is_dirty() to index_differs_from(...) Message-ID: <1234276235-21822-1-git-send-email-s-beyer@gmx.net> URL: https://gitlist.dev/e/1234276235-21822-1-git-send-email-s-beyer%40gmx.net In-Reply-To: <7v63jirhw8.fsf@gitster.siamese.dyndns.org> ``` index_is_dirty() in builtin-revert.c checks if the index is dirty. This patch generalizes this function to check if the index differs from a revision, i.e. the former index_is_dirty() behavior can now be achieved by index_differs_from("HEAD", 0). The second argument "diff_flags" allows to set further diff option flags like DIFF_OPT_IGNORE_SUBMODULES. See DIFF_OPT_* macros in diff.h for a list. index_differs_from() seems to be useful for more than builtin-revert.c, so it is moved into diff-lib.c and also used in builtin-commit.c. Yet to mention: - "rev.abbrev = 0;" can be safely removed. This has no impact on performance or functioning of neither setup_revisions() nor run_diff_index(). - rev.pending.objects is free()d because this fixes a leak. (Also see 295dd2ad "Fix memory leak in traverse_commit_list") Mentored-by: Daniel Barkalow Mentored-by: Christian Couder Signed-off-by: Stephan Beyer --- Err, this didn't get to the list, did it? builtin-commit.c | 13 ++----------- builtin-revert.c | 13 +------------ diff-lib.c | 15 +++++++++++++++ diff.h | 2 ++ 4 files changed, 20 insertions(+), 23 deletions(-) diff --git a/builtin-commit.c b/builtin-commit.c index d6a3a62..46e649c 100644 --- a/builtin-commit.c +++ b/builtin-commit.c @@ -561,7 +561,6 @@ static int prepare_to_commit(const char *index_file, const char *prefix) commitable = run_status(fp, index_file, prefix, 1); wt_status_use_color = saved_color_setting; } else { - struct rev_info rev; unsigned char sha1[20]; const char *parent = "HEAD"; @@ -573,16 +572,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix) if (get_sha1(parent, sha1)) commitable = !!active_nr; - else { - init_revisions(&rev, ""); - rev.abbrev = 0; - setup_revisions(0, NULL, &rev, parent); - DIFF_OPT_SET(&rev.diffopt, QUIET); - DIFF_OPT_SET(&rev.diffopt, EXIT_WITH_STATUS); - run_diff_index(&rev, 1 /* cached */); - - commitable = !!DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES); - } + else + commitable = index_differs_from(parent, 0); } fclose(fp); diff --git a/builtin-revert.c b/builtin-revert.c index d48313c..d210150 100644 --- a/builtin-revert.c +++ b/builtin-revert.c @@ -223,17 +223,6 @@ static char *help_msg(const unsigned char *sha1) return helpbuf; } -static int index_is_dirty(void) -{ - struct rev_info rev; - init_revisions(&rev, NULL); - setup_revisions(0, NULL, &rev, "HEAD"); - DIFF_OPT_SET(&rev.diffopt, QUIET); - DIFF_OPT_SET(&rev.diffopt, EXIT_WITH_STATUS); - run_diff_index(&rev, 1); - return !!DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES); -} - static struct tree *empty_tree(void) { struct tree *tree = xcalloc(1, sizeof(struct tree)); @@ -279,7 +268,7 @@ static int revert_or_cherry_pick(int argc, const char **argv) } else { if (get_sha1("HEAD", head)) die ("You do not have a valid HEAD"); - if (index_is_dirty()) + if (index_differs_from("HEAD", 0)) die ("Dirty index: cannot %s", me); } discard_cache(); diff --git a/diff-lib.c b/diff-lib.c index a41e1ec..79d0606 100644 --- a/diff-lib.c +++ b/diff-lib.c @@ -513,3 +513,18 @@ int do_diff_cache(const unsigned char *tree_sha1, struct diff_options *opt) exit(128); return 0; } + +int index_differs_from(const char *def, int diff_flags) +{ + struct rev_info rev; + + init_revisions(&rev, NULL); + setup_revisions(0, NULL, &rev, def); + DIFF_OPT_SET(&rev.diffopt, QUIET); + DIFF_OPT_SET(&rev.diffopt, EXIT_WITH_STATUS); + rev.diffopt.flags |= diff_flags; + run_diff_index(&rev, 1); + if (rev.pending.alloc) + free(rev.pending.objects); + return (DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES) != 0); +} diff --git a/diff.h b/diff.h index 23cd90c..6703a4f 100644 --- a/diff.h +++ b/diff.h @@ -265,4 +265,6 @@ extern int diff_result_code(struct diff_options *, int); extern void diff_no_index(struct rev_info *, int, const char **, int, const char *); +extern int index_differs_from(const char *def, int diff_flags); + #endif /* DIFF_H */ -- 1.6.2.rc0.464.g3ec3 ```