{"thread":{"id":"9008","subject":"[PATCH 0/6] Add git-rewrite-commits v2","startedAt":"2007-07-12T19:05:57Z","lastAt":"2007-07-19T12:40:53Z","messageCount":23,"participants":["skimo@liacs.nl","Sven Verdoolaege","Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"47195","messageId":"11842671631744-git-send-email-skimo@liacs.nl","threadId":"9008","inReplyTo":null,"subject":"[PATCH 0/6] Add git-rewrite-commits v2","fromName":"","fromEmail":"skimo@liacs.nl","sentAt":"2007-07-12T19:05:57Z","receivedAt":"2007-07-12T19:05:57Z","isPatch":true,"sender":{"key":"skimo@kotnet.org","avatar":null},"body":"From: Sven Verdoolaege \n\n[PATCH 1/6] revision: allow selection of commits that do not match a pattern\n[PATCH 2/6] export get_short_sha1\n[PATCH 3/6] Define ishex(x) in git-compat-util.h\n[PATCH 4/6] refs.c: lock cached_refs during for_each_ref\n[PATCH 5/6] revision: mark commits that didn't match a pattern for later use\n[PATCH 6/6] Add git-rewrite-commits\n\nThe first is fairly independent, but it is used in the tests\nof the last patch and may be replaced by Johannes' extended patch.\nThe next three should be fairly uncontroversial.\nThe fifth may be considered a waste of a precious bit.\nIf so, any suggestions for other ways of passing on this information\nare welcomed.\nThe sixth contains the actual git-rewrite-commits builtin.\nMy main motivation was that cg-admin-rewritehist doesn't\nchange the SHA1's in commit message and I don't like shell\nprogramming.\n\nThe main difference with the previous series is that\nthe options are now called --index-filter and --commit-filter\ninstead of --index-map and --commit-map.\n\nThe commit filter should now produce a list of commit SHA1\ninstead of the modified raw commit.\nThe refs are now rewritten at the very end, which should\nbe safer if anything goes wrong along the way and obviates\nthe need for the call to add_ref_decoration of the previous\nseries.\n\nMost of the changes were requested by Johannes Schindelin.\nI'm sure he'll let me know if I forgot anything.\nIn a follow-up patch (series), he will add more helper functions\nfor the filters.\n\nskimo\n"},{"id":"47201","messageId":"1184267163786-git-send-email-skimo@liacs.nl","threadId":"9008","inReplyTo":"11842671631744-git-send-email-skimo@liacs.nl","subject":"[PATCH 1/6] revision: allow selection of commits that do not match a pattern","fromName":"","fromEmail":"skimo@liacs.nl","sentAt":"2007-07-12T19:05:58Z","receivedAt":"2007-07-12T19:05:58Z","isPatch":true,"sender":{"key":"skimo@kotnet.org","avatar":null},"body":"From: Sven Verdoolaege \n\nWe do this by maintaining two lists of patterns, one for\nthose that should match and one for those that should not match.\n\nA negative pattern is specified by putting a '!' in front.\nFor example, to show the commits of Jakub Narebski that\nare not about gitweb, you'd do a\n\n\tgit log --author='Narebski' --grep='!gitweb' --all-match\n\nAs an added bonus, this patch also documents --all-match.\n\nSigned-off-by: Sven Verdoolaege \nReviewed-by: Johannes Schindelin \n---\n Documentation/git-rev-list.txt | 17 ++++++++++\n revision.c | 64 +++++++++++++++++++++++++++++++++------\n revision.h | 1 +\n 3 files changed, 72 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt\nindex 20dcac6..c462f5d 100644\n--- a/Documentation/git-rev-list.txt\n+++ b/Documentation/git-rev-list.txt\n@@ -214,11 +214,19 @@ limiting may be applied.\n \n \tLimit the commits output to ones with author/committer\n \theader lines that match the specified pattern (regular expression).\n+\tA pattern starting with a '!' will show only commits that do\n+\tnot match the remainder of the pattern.\n+\tTo match lines starting with '!', escape the initial '!'\n+\twith a backslash.\n \n --grep='pattern'::\n \n \tLimit the commits output to ones with log message that\n \tmatches the specified pattern (regular expression).\n+\tA pattern starting with a '!' will show only commits that do\n+\tnot match the remainder of the pattern.\n+\tTo match lines starting with '!', escape the initial '!'\n+\twith a backslash.\n \n --regexp-ignore-case::\n \n@@ -229,6 +237,15 @@ limiting may be applied.\n \tConsider the limiting patterns to be extended regular expressions\n \tinstead of the default basic regular expressions.\n \n+--all-match::\n+\n+\tWithout this option, a commit is shown if any of the\n+\t(positive or negative) patterns matches, i.e., there\n+\tis at least one positive match or not all of the negative\n+\tpatterns match. With this options, a commit is only\n+\tshown if all of the patterns match, i.e., all positive\n+\tpatterns match and no negative pattern matches.\n+\n --remove-empty::\n \n \tStop when a given path disappears from the tree.\ndiff --git a/revision.c b/revision.c\nindex 27cce09..38061f9 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -826,34 +826,50 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,\n \treturn 0;\n }\n \n-static void add_grep(struct rev_info *revs, const char *ptn, enum grep_pat_token what)\n+static void add_grep(struct rev_info *revs, const char *ptn,\n+\t\t enum grep_pat_token what)\n {\n-\tif (!revs->grep_filter) {\n+\tint negated = 0;\n+\tstruct grep_opt **filter;\n+\n+\tif (ptn[0] == '\\\\' && ptn[1] == '!')\n+\t\tptn++;\n+\tif (*ptn == '!') {\n+\t\tnegated = 1;\n+\t\tptn++;\n+\t}\n+\tfilter = negated ? &revs->grep_neg_filter : &revs->grep_filter;\n+\tif (!*filter) {\n \t\tstruct grep_opt *opt = xcalloc(1, sizeof(*opt));\n \t\topt->status_only = 1;\n \t\topt->pattern_tail = &(opt->pattern_list);\n \t\topt->regflags = REG_NEWLINE;\n-\t\trevs->grep_filter = opt;\n+\t\t*filter = opt;\n \t}\n-\tappend_grep_pattern(revs->grep_filter, ptn,\n-\t\t\t \"command line\", 0, what);\n+\tappend_grep_pattern(*filter, ptn, \"command line\", 0, what);\n }\n \n-static void add_header_grep(struct rev_info *revs, const char *field, const char *pattern)\n+static void add_header_grep(struct rev_info *revs, const char *field,\n+\t\t\t const char *pattern)\n {\n \tchar *pat;\n-\tconst char *prefix;\n+\tconst char *prefix, *negated;\n \tint patlen, fldlen;\n \n \tfldlen = strlen(field);\n \tpatlen = strlen(pattern);\n \tpat = xmalloc(patlen + fldlen + 10);\n+\tnegated = \"\";\n+\tif (*pattern == '!') {\n+\t\tnegated = \"!\";\n+\t\tpattern++;\n+\t}\n \tprefix = \".*\";\n \tif (*pattern == '^') {\n \t\tprefix = \"\";\n \t\tpattern++;\n \t}\n-\tsprintf(pat, \"^%s %s%s\", field, prefix, pattern);\n+\tsprintf(pat, \"%s^%s %s%s\", negated, field, prefix, pattern);\n \tadd_grep(revs, pat, GREP_PATTERN_HEAD);\n }\n \n@@ -1217,6 +1233,9 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch\n \tif (revs->grep_filter)\n \t\trevs->grep_filter->regflags |= regflags;\n \n+\tif (revs->grep_neg_filter)\n+\t\trevs->grep_neg_filter->regflags |= regflags;\n+\n \tif (show_merge)\n \t\tprepare_show_merge(revs);\n \tif (def && !revs->pending.nr) {\n@@ -1254,6 +1273,11 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch\n \t\tcompile_grep_patterns(revs->grep_filter);\n \t}\n \n+\tif (revs->grep_neg_filter) {\n+\t\trevs->grep_neg_filter->all_match = !all_match;\n+\t\tcompile_grep_patterns(revs->grep_neg_filter);\n+\t}\n+\n \treturn left;\n }\n \n@@ -1352,11 +1376,31 @@ static int rewrite_parents(struct rev_info *revs, struct commit *commit)\n \treturn 0;\n }\n \n+/*\n+ * If all_match is set, then a commit matches if all the positive\n+ * patterns match and not one of the negative patterns matches.\n+ * If all_match is not set, then a commit matches if at least one\n+ * of the positive patterns matches or not all of the negative\n+ * patterns match.\n+ */\n static int commit_match(struct commit *commit, struct rev_info *opt)\n {\n-\tif (!opt->grep_filter)\n+\tint pos_match, all_match;\n+\n+\tpos_match = !opt->grep_filter ||\n+\t\t grep_buffer(opt->grep_filter,\n+\t\t\t NULL, /* we say nothing, not even filename */\n+\t\t\t commit->buffer, strlen(commit->buffer));\n+\tif (!opt->grep_neg_filter)\n+\t\treturn pos_match;\n+\n+\tall_match = !opt->grep_neg_filter->all_match;\n+\tif (!all_match && opt->grep_filter && pos_match)\n \t\treturn 1;\n-\treturn grep_buffer(opt->grep_filter,\n+\tif (all_match && !pos_match)\n+\t\treturn 0;\n+\n+\treturn !grep_buffer(opt->grep_neg_filter,\n \t\t\t NULL, /* we say nothing, not even filename */\n \t\t\t commit->buffer, strlen(commit->buffer));\n }\ndiff --git a/revision.h b/revision.h\nindex f46b4d5..9728d4c 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -84,6 +84,7 @@ struct rev_info {\n \n \t/* Filter by commit log message */\n \tstruct grep_opt\t*grep_filter;\n+\tstruct grep_opt\t*grep_neg_filter;\n \n \t/* special limits */\n \tint skip_count;\n-- \n1.5.3.rc0.100.ge60b4\n"},{"id":"47197","messageId":"11842671633205-git-send-email-skimo@liacs.nl","threadId":"9008","inReplyTo":"11842671631744-git-send-email-skimo@liacs.nl","subject":"[PATCH 2/6] export get_short_sha1","fromName":"","fromEmail":"skimo@liacs.nl","sentAt":"2007-07-12T19:05:59Z","receivedAt":"2007-07-12T19:05:59Z","isPatch":true,"sender":{"key":"skimo@kotnet.org","avatar":null},"body":"From: Sven Verdoolaege \n\nSometimes it is useful to check whether a given string\nis exactly a short SHA1.\n\nSigned-off-by: Sven Verdoolaege \n---\n cache.h | 1 +\n sha1_name.c | 3 +--\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex e64071e..8fda8ee 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -391,6 +391,7 @@ static inline unsigned int hexval(unsigned char c)\n \n extern int get_sha1(const char *str, unsigned char *sha1);\n extern int get_sha1_with_mode(const char *str, unsigned char *sha1, unsigned *mode);\n+extern int get_short_sha1(const char *name, int len, unsigned char *sha1, int quietly);\n extern int get_sha1_hex(const char *hex, unsigned char *sha1);\n extern char *sha1_to_hex(const unsigned char *sha1);\t/* static buffer result! */\n extern int read_ref(const char *filename, unsigned char *sha1);\ndiff --git a/sha1_name.c b/sha1_name.c\nindex 858f08c..0bed79d 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -154,8 +154,7 @@ static int find_unique_short_object(int len, char *canonical,\n \treturn 0;\n }\n \n-static int get_short_sha1(const char *name, int len, unsigned char *sha1,\n-\t\t\t int quietly)\n+int get_short_sha1(const char *name, int len, unsigned char *sha1, int quietly)\n {\n \tint i, status;\n \tchar canonical[40];\n-- \n1.5.3.rc0.100.ge60b4\n"},{"id":"47198","messageId":"11842671632000-git-send-email-skimo@liacs.nl","threadId":"9008","inReplyTo":"11842671631744-git-send-email-skimo@liacs.nl","subject":"[PATCH 3/6] Define ishex(x) in git-compat-util.h","fromName":"","fromEmail":"skimo@liacs.nl","sentAt":"2007-07-12T19:06:00Z","receivedAt":"2007-07-12T19:06:00Z","isPatch":true,"sender":{"key":"skimo@kotnet.org","avatar":null},"body":"From: Sven Verdoolaege \n\nSigned-off-by: Sven Verdoolaege \n---\n builtin-name-rev.c | 1 -\n ctype.c | 5 +++--\n git-compat-util.h | 5 ++++-\n 3 files changed, 7 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin-name-rev.c b/builtin-name-rev.c\nindex 61eba34..b2ac40c 100644\n--- a/builtin-name-rev.c\n+++ b/builtin-name-rev.c\n@@ -233,7 +233,6 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)\n \t\t\t\tbreak;\n \n \t\t\tfor (p_start = p; *p; p++) {\n-#define ishex(x) (isdigit((x)) || ((x) >= 'a' && (x) <= 'f'))\n \t\t\t\tif (!ishex(*p))\n \t\t\t\t\tforty = 0;\n \t\t\t\telse if (++forty == 40 &&\ndiff --git a/ctype.c b/ctype.c\nindex ee06eb7..97b5724 100644\n--- a/ctype.c\n+++ b/ctype.c\n@@ -6,7 +6,8 @@\n #include \"cache.h\"\n \n #define SS GIT_SPACE\n-#define AA GIT_ALPHA\n+#define HA GIT_HEXAL\n+#define AA GIT_OTHAL\n #define DD GIT_DIGIT\n \n unsigned char sane_ctype[256] = {\n@@ -16,7 +17,7 @@ unsigned char sane_ctype[256] = {\n \tDD, DD, DD, DD, DD, DD, DD, DD, DD, DD, 0, 0, 0, 0, 0, 0,\t\t/* 48-15 */\n \t 0, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA,\t\t/* 64-15 */\n \tAA, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, 0, 0, 0, 0, 0,\t\t/* 80-15 */\n-\t 0, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA,\t\t/* 96-15 */\n+\t 0, HA, HA, HA, HA, HA, HA, AA, AA, AA, AA, AA, AA, AA, AA, AA,\t\t/* 96-15 */\n \tAA, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, 0, 0, 0, 0, 0,\t\t/* 112-15 */\n \t/* Nothing in the 128.. range */\n };\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 362e040..1a36f4c 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -325,12 +325,15 @@ static inline int has_extension(const char *filename, const char *ext)\n extern unsigned char sane_ctype[256];\n #define GIT_SPACE 0x01\n #define GIT_DIGIT 0x02\n-#define GIT_ALPHA 0x04\n+#define GIT_HEXAL 0x04\n+#define GIT_OTHAL 0x08\n+#define GIT_ALPHA (GIT_HEXAL | GIT_OTHAL)\n #define sane_istest(x,mask) ((sane_ctype[(unsigned char)(x)] & (mask)) != 0)\n #define isspace(x) sane_istest(x,GIT_SPACE)\n #define isdigit(x) sane_istest(x,GIT_DIGIT)\n #define isalpha(x) sane_istest(x,GIT_ALPHA)\n #define isalnum(x) sane_istest(x,GIT_ALPHA | GIT_DIGIT)\n+#define ishex(x) sane_istest(x,GIT_HEXAL | GIT_DIGIT)\n #define tolower(x) sane_case((unsigned char)(x), 0x20)\n #define toupper(x) sane_case((unsigned char)(x), 0)\n \n-- \n1.5.3.rc0.100.ge60b4\n"},{"id":"47199","messageId":"11842671632300-git-send-email-skimo@liacs.nl","threadId":"9008","inReplyTo":"11842671631744-git-send-email-skimo@liacs.nl","subject":"[PATCH 4/6] refs.c: lock cached_refs during for_each_ref","fromName":"","fromEmail":"skimo@liacs.nl","sentAt":"2007-07-12T19:06:01Z","receivedAt":"2007-07-12T19:06:01Z","isPatch":true,"sender":{"key":"skimo@kotnet.org","avatar":null},"body":"From: Sven Verdoolaege \n\nIf the function called by for_each_ref modifies a ref in any way,\nthe cached_refs that for_each_ref was looping over would be\nremoved, resulting in undefined behavior.\n\nThis patch prevents the cached_refs from being removed\nwhile for_each_ref is still iterating over them.\n\nSigned-off-by: Sven Verdoolaege \n---\n refs.c | 37 +++++++++++++++++++++++++++++++++----\n 1 files changed, 33 insertions(+), 4 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 4dc7e8b..e710903 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -153,6 +153,8 @@ static struct ref_list *sort_ref_list(struct ref_list *list)\n static struct cached_refs {\n \tchar did_loose;\n \tchar did_packed;\n+\tchar is_locked;\n+\tchar is_invalidated;\n \tstruct ref_list *loose;\n \tstruct ref_list *packed;\n } cached_refs;\n@@ -170,6 +172,11 @@ static void invalidate_cached_refs(void)\n {\n \tstruct cached_refs *ca = &cached_refs;\n \n+\tif (ca->is_locked) {\n+\t\tca->is_invalidated = 1;\n+\t\treturn;\n+\t}\n+\n \tif (ca->did_loose && ca->loose)\n \t\tfree_ref_list(ca->loose);\n \tif (ca->did_packed && ca->packed)\n@@ -178,6 +185,24 @@ static void invalidate_cached_refs(void)\n \tca->did_loose = ca->did_packed = 0;\n }\n \n+static void lock_cached_refs(void)\n+{\n+\tstruct cached_refs *ca = &cached_refs;\n+\n+\tca->is_locked = 1;\n+}\n+\n+static void unlock_cached_refs(void)\n+{\n+\tstruct cached_refs *ca = &cached_refs;\n+\n+\tca->is_locked = 0;\n+\tif (ca->is_invalidated) {\n+\t\tinvalidate_cached_refs();\n+\t\tca->is_invalidated = 0;\n+\t}\n+}\n+\n static void read_packed_refs(FILE *f, struct cached_refs *cached_refs)\n {\n \tstruct ref_list *list = NULL;\n@@ -518,10 +543,12 @@ int peel_ref(const char *ref, unsigned char *sha1)\n static int do_for_each_ref(const char *base, each_ref_fn fn, int trim,\n \t\t\t void *cb_data)\n {\n-\tint retval;\n+\tint retval = 0;\n \tstruct ref_list *packed = get_packed_refs();\n \tstruct ref_list *loose = get_loose_refs();\n \n+\tlock_cached_refs();\n+\n \twhile (packed && loose) {\n \t\tstruct ref_list *entry;\n \t\tint cmp = strcmp(packed->name, loose->name);\n@@ -538,15 +565,17 @@ static int do_for_each_ref(const char *base, each_ref_fn fn, int trim,\n \t\t}\n \t\tretval = do_one_ref(base, fn, trim, cb_data, entry);\n \t\tif (retval)\n-\t\t\treturn retval;\n+\t\t\tgoto out;\n \t}\n \n \tfor (packed = packed ? packed : loose; packed; packed = packed->next) {\n \t\tretval = do_one_ref(base, fn, trim, cb_data, packed);\n \t\tif (retval)\n-\t\t\treturn retval;\n+\t\t\tgoto out;\n \t}\n-\treturn 0;\n+ out:\n+\tunlock_cached_refs();\n+\treturn retval;\n }\n \n int head_ref(each_ref_fn fn, void *cb_data)\n-- \n1.5.3.rc0.100.ge60b4\n"},{"id":"47200","messageId":"11842671632265-git-send-email-skimo@liacs.nl","threadId":"9008","inReplyTo":"11842671631744-git-send-email-skimo@liacs.nl","subject":"[PATCH 5/6] revision: mark commits that didn't match a pattern for later use","fromName":"","fromEmail":"skimo@liacs.nl","sentAt":"2007-07-12T19:06:02Z","receivedAt":"2007-07-12T19:06:02Z","isPatch":true,"sender":{"key":"skimo@kotnet.org","avatar":null},"body":"From: Sven Verdoolaege \n\nSigned-off-by: Sven Verdoolaege \n---\n revision.c | 4 +++-\n revision.h | 1 +\n 2 files changed, 4 insertions(+), 1 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 38061f9..cbeb137 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1446,8 +1446,10 @@ static struct commit *get_revision_1(struct rev_info *revs)\n \t\tif (revs->no_merges &&\n \t\t commit->parents && commit->parents->next)\n \t\t\tcontinue;\n-\t\tif (!commit_match(commit, revs))\n+\t\tif (!commit_match(commit, revs)) {\n+\t\t\tcommit->object.flags |= PRUNED;\n \t\t\tcontinue;\n+\t\t}\n \t\tif (revs->prune_fn && revs->dense) {\n \t\t\t/* Commit without changes? */\n \t\t\tif (!(commit->object.flags & TREECHANGE)) {\ndiff --git a/revision.h b/revision.h\nindex 9728d4c..d3609ab 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -10,6 +10,7 @@\n #define CHILD_SHOWN\t(1u<<6)\n #define ADDED\t\t(1u<<7)\t/* Parents already parsed and added? */\n #define SYMMETRIC_LEFT\t(1u<<8)\n+#define PRUNED\t\t(1u<<9)\n \n struct rev_info;\n struct log_info;\n-- \n1.5.3.rc0.100.ge60b4\n"},{"id":"47196","messageId":"11842671631635-git-send-email-skimo@liacs.nl","threadId":"9008","inReplyTo":"11842671631744-git-send-email-skimo@liacs.nl","subject":"[PATCH 6/6] Add git-rewrite-commits","fromName":"","fromEmail":"skimo@liacs.nl","sentAt":"2007-07-12T19:06:03Z","receivedAt":"2007-07-12T19:06:03Z","isPatch":true,"sender":{"key":"skimo@kotnet.org","avatar":null},"body":"From: Sven Verdoolaege \n\nThis builtin is similar to git-filter-branch (and cg-admin-rewritehist).\nThe main difference is that git-rewrite-commits will automatically\nrewrite any SHA1 in a commit message to the rewritten SHA1 as\nwell as any reference (in .git/refs/) that points to a rewritten commit.\n\nIt's also a lot faster than git-filter-branch if no external command\nis called. For example, running either to eliminating a commit specified\nas a graft results in the following timings, both being performed\non a freshly cloned copy of a small repo:\n\n\tbash-3.00$ time git-filter-branch test\n\tRewrite 274fe3dfb8e8c7d0a6ce05138bdb650de7b459ea (425/425)\n\tRewritten history saved to the test branch\n\n\treal 0m30.845s\n\tuser 0m13.400s\n\tsys 0m19.640s\n\n\tbash-3.00$ time git-rewrite-commits\n\n\treal 0m0.223s\n\tuser 0m0.080s\n\tsys 0m0.140s\n\nThe command line is more reminiscent of git-log.\nFor example you can say\n\n\tgit-rewrite-commits --all\n\nto incorporate grafts in all branches, or\n\n\tgit rewrite-commits --author='!Darl McBribe' --all\n\nto remove all commits by Darl McBribe.\n\nSigned-off-by: Sven Verdoolaege \nReviewed-by: Johannes Schindelin \n---\n .gitignore | 1 +\n Documentation/cmd-list.perl | 1 +\n Documentation/git-rewrite-commits.txt | 156 +++++++\n Makefile | 1 +\n builtin-rewrite-commits.c | 712 +++++++++++++++++++++++++++++++++\n builtin.h | 1 +\n git.c | 1 +\n t/t7005-rewrite-commits.sh | 109 +++++\n 8 files changed, 982 insertions(+), 0 deletions(-)\n create mode 100644 Documentation/git-rewrite-commits.txt\n create mode 100644 builtin-rewrite-commits.c\n create mode 100755 t/t7005-rewrite-commits.sh\n\ndiff --git a/.gitignore b/.gitignore\nindex 20ee642..bcd95a9 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -109,6 +109,7 @@ git-reset\n git-rev-list\n git-rev-parse\n git-revert\n+git-rewrite-commits\n git-rm\n git-runstatus\n git-send-email\ndiff --git a/Documentation/cmd-list.perl b/Documentation/cmd-list.perl\nindex 2143995..63911b7 100755\n--- a/Documentation/cmd-list.perl\n+++ b/Documentation/cmd-list.perl\n@@ -166,6 +166,7 @@ git-reset mainporcelain\n git-revert mainporcelain\n git-rev-list plumbinginterrogators\n git-rev-parse ancillaryinterrogators\n+git-rewrite-commits mainporcelain\n git-rm mainporcelain\n git-runstatus ancillaryinterrogators\n git-send-email foreignscminterface\ndiff --git a/Documentation/git-rewrite-commits.txt b/Documentation/git-rewrite-commits.txt\nnew file mode 100644\nindex 0000000..fb4e220\n--- /dev/null\n+++ b/Documentation/git-rewrite-commits.txt\n@@ -0,0 +1,156 @@\n+git-rewrite-commits(1)\n+======================\n+\n+NAME\n+----\n+git-rewrite-commits - Rewrite commits\n+\n+SYNOPSIS\n+--------\n+'git-rewrite-commits' [--index-filter ] [--commit-filter ]\n+\t[...]\n+\n+DESCRIPTION\n+-----------\n+Lets you rewrite the commits selected by the gitlink:git-rev-list[1]\n+options, optionally applying custom filters on each of them.\n+These filters either modify the tree associated to a commit\n+(through manipulations of the index) or the (raw) commit itself.\n+Any commit within the specified range that is filtered out\n+through a pattern is removed from its children (assuming they\n+are in range) and replaced by the closest ancestors that\n+are either not being rewritten or not filtered out.\n+\n+Any branch pointing to any of the rewritten commits is replaced\n+by a pointer to the new commit. The original pointers are saved\n+in the refs/original hierarchy. Any abbreviated SHA1 in the commit\n+message of any of the rewritten commits that points to (another)\n+rewritten commit is replaced by the SHA1 of the corresponding new commit.\n+\n+Besides the actions specified by the filters, the new commits\n+will also reflect any grafts that may apply to any of the selected commits.\n+\n+*WARNING*! The rewritten history will have different object names for all\n+the objects and will not converge with the original branch. You will not\n+be able to easily push and distribute the rewritten branch on top of the\n+original branch. Please do not use this command if you do not know the\n+full implications, and avoid using it anyway, if a simple single commit\n+would suffice to fix your problem.\n+\n+Filters\n+~~~~~~~\n+\n+The filters are applied in the order as listed below. The \n+argument is run as \"sh -c '\", with the $GIT_COMMIT\n+environment variable set to the commit that is being rewritten.\n+If any call to a filter fails, then git-rewrite-commits will abort.\n+\n+\n+OPTIONS\n+-------\n+\n+--index-filter ::\n+\tThis filter should only modify the index specified by 'GIT_INDEX_FILE'.\n+\tPrior to running the filter, this temporary index is populated\n+\twith the tree of the commit that is being rewritten.\n+\tThis tree will be replaced according to the new state of this\n+\ttemporary index after the filter finishes.\n+\n+--commit-filter ::\n+\tThis filter receives the (raw) commit on stdin and should produce\n+\tzero or more SHA1s of commits that should replace the given commit\n+\ton stdout.\n+\tIn other words, the filter \"git-hash-object -w -t commit --stdin\"\n+\tleaves the current commit intact. This command is also available\n+\tas the shell function \"commit\".\n+\tIn the input commit, the tree has already been modified by the\n+\tindex filter (if any) and has all references to older commits\n+\t(including the parents) changed to the possibly rewritten commits.\n+\n+--write-sha1-mapping\n+\tWrite mapping of old SHA1s to new SHA1s for use in filters.\n+\n+::\n+\tSelects the commits to be rewritten, defaulting to the history\n+\tthat lead to HEAD. If commits are filtered using a (negative)\n+\tpattern then all the commits filtered out will be removed\n+\tfrom the history of the selected commits.\n+\n+\n+Examples\n+--------\n+\n+Suppose you want to remove a file (containing confidential information\n+or copyright violation) from all commits:\n+\n+----------------------------------------------------------------------------\n+git rewrite-commits --index-filter 'git update-index --remove filename || :'\n+----------------------------------------------------------------------------\n+\n+Now, you will get the rewritten history saved in your current branch\n+(the old branch is saved in refs/original).\n+\n+To set a commit \"$graft-id\" (which typically is at the tip of another\n+history) to be the parent of the current initial commit \"$commit-id\", in\n+order to paste the other history behind the current history:\n+\n+-----------------------------------------------\n+echo \"$commit-id $graft-id\" >> .git/info/grafts\n+git rewrite-commits\n+-----------------------------------------------\n+\n+To remove commits authored by \"Darl McBribe\" from the history:\n+\n+--------------------------------------------\n+git rewrite-commits --author='!Darl McBribe'\n+--------------------------------------------\n+\n+Note that the changes introduced by the commits, and not reverted by\n+subsequent commits, will still be in the rewritten branch. If you want\n+to throw out _changes_ together with the commits, you should use the\n+interactive mode of gitlink:git-rebase[1].\n+\n+Consider this history:\n+\n+------------------\n+ D--E--F--G--H\n+ / /\n+A--B-----C\n+------------------\n+\n+To rewrite only commits D,E,F,G,H, but leave A, B and C alone, use:\n+\n+--------------------------------\n+git rewrite-commits ... C..H\n+--------------------------------\n+\n+To rewrite commits E,F,G,H, use one of these:\n+\n+----------------------------------------\n+git rewrite-commits ... C..H --not D\n+git rewrite-commits ... D..H --not C\n+----------------------------------------\n+\n+To move the whole tree into a subdirectory, or remove it from there:\n+\n+---------------------------------------------------------------\n+git rewrite-commits --index-filter \\\n+\t'git ls-files -s | sed \"s-\\t-&newsubdir/-\" |\n+\t\tGIT_INDEX_FILE=$GIT_INDEX_FILE.new \\\n+\t\t\tgit update-index --index-info &&\n+\t mv $GIT_INDEX_FILE.new $GIT_INDEX_FILE'\n+---------------------------------------------------------------\n+\n+\n+Author\n+------\n+Written by Sven Verdoolaege.\n+Inspired by cg-admin-rewritehist by Petr \"Pasky\" Baudis .\n+\n+Documentation\n+--------------\n+Documentation by Petr Baudis, Sven Verdoolaege and the git list.\n+\n+GIT\n+---\n+Part of the gitlink:git[7] suite\ndiff --git a/Makefile b/Makefile\nindex d7541b4..252ef8e 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -370,6 +370,7 @@ BUILTIN_OBJS = \\\n \tbuiltin-rev-list.o \\\n \tbuiltin-rev-parse.o \\\n \tbuiltin-revert.o \\\n+\tbuiltin-rewrite-commits.o \\\n \tbuiltin-rm.o \\\n \tbuiltin-runstatus.o \\\n \tbuiltin-shortlog.o \\\ndiff --git a/builtin-rewrite-commits.c b/builtin-rewrite-commits.c\nnew file mode 100644\nindex 0000000..d95a16c\n--- /dev/null\n+++ b/builtin-rewrite-commits.c\n@@ -0,0 +1,712 @@\n+#include \"cache.h\"\n+#include \"refs.h\"\n+#include \"builtin.h\"\n+#include \"commit.h\"\n+#include \"diff.h\"\n+#include \"revision.h\"\n+#include \"list-objects.h\"\n+#include \"run-command.h\"\n+#include \"cache-tree.h\"\n+#include \"grep.h\"\n+\n+struct decoration rewrite_decoration = { \"rewritten as\" };\n+static const char *original_prefix = \"original\";\n+static const char *commit_filter;\n+static const char *index_filter;\n+static const char *absolute_git_dir;\n+static const char *rewrite_dir;\n+static char commit_env[] = \"GIT_COMMIT=0000000000000000000000000000000000000000\";\n+static int write_sha1_mapping;\n+\n+struct rewrite_decoration {\n+\tstruct rewrite_decoration *next;\n+\tunsigned char sha1[20];\n+};\n+\n+static void add_rewrite_decoration(struct object *obj, unsigned char *sha1)\n+{\n+\tstruct rewrite_decoration *deco = xmalloc(sizeof(struct rewrite_decoration));\n+\thashcpy(deco->sha1, sha1);\n+\tdeco->next = add_decoration(&rewrite_decoration, obj, deco);\n+}\n+\n+static int get_rewritten_sha1(unsigned char *sha1)\n+{\n+\tstruct rewrite_decoration *deco;\n+\tstruct object *obj = lookup_object(sha1);\n+\n+\tif (!obj)\n+\t\treturn 1;\n+\n+\tdeco = lookup_decoration(&rewrite_decoration, obj);\n+\tif (!deco)\n+\t\treturn 1;\n+\n+\thashcpy(sha1, deco->sha1);\n+\treturn 0;\n+}\n+\n+static char *add_parents(char *dest, struct commit_list *parents)\n+{\n+\tunsigned char sha1[20];\n+\tstruct commit_list *list;\n+\n+\tfor (list = parents; list; list = list->next) {\n+\t\thashcpy(sha1, list->item->object.sha1);\n+\t\tget_rewritten_sha1(sha1);\n+\t\tmemcpy(dest, \"parent \", 7);\n+\t\tdest += 7;\n+\t\tmemcpy(dest, sha1_to_hex(sha1), 40);\n+\t\tdest += 40;\n+\t\t*dest++ = '\\n';\n+\t}\n+\treturn dest;\n+}\n+\n+static int skip_one_line(char **buf_p, unsigned long *len_p)\n+{\n+\tint linelen;\n+\tchar *end = memchr(*buf_p, '\\n', *len_p);\n+\n+\tif (!end)\n+\t\tlinelen = *len_p;\n+\tif (end)\n+\t\tlinelen = end - *buf_p + 1;\n+\t*buf_p += linelen;\n+\t*len_p -= linelen;\n+\n+\treturn linelen;\n+}\n+\n+static char *filter_index(char *orig_hex, struct commit *commit)\n+{\n+\tint argc;\n+\tconst char *argv[10];\n+\tstatic char index_env[16+PATH_MAX];\n+\tchar *tmp_index = mkpath(\"%s/rewrite_index\", absolute_git_dir);\n+\tconst char *env[] = { index_env, commit_env, NULL };\n+\tchar *hex;\n+\n+\tmemcpy(index_env, \"GIT_INDEX_FILE=\", 15);\n+\tstrcpy(index_env+15, tmp_index);\n+\ttmp_index = index_env+15;\n+\n+\tmemcpy(commit_env+sizeof(commit_env)-41,\n+\t\tsha1_to_hex(commit->object.sha1), 40);\n+\n+\t/* First write out tree to temporary index */\n+\targc = 0;\n+\targv[argc++] = \"read-tree\";\n+\targv[argc++] = orig_hex;\n+\targv[argc] = NULL;\n+\tif (run_command_v_opt_cd_env(argv, RUN_GIT_CMD, NULL, env))\n+\t\tdie(\"Cannot write index '%s' for filter\", tmp_index);\n+\n+\t/* Then filter the index */\n+\targc = 0;\n+\targv[argc++] = \"sh\";\n+\targv[argc++] = \"-c\";\n+\targv[argc++] = index_filter;\n+\targv[argc] = NULL;\n+\tif (run_command_v_opt_cd_env(argv, 0, rewrite_dir, env))\n+\t\tdie(\"Index filter '%s' failed\", index_filter);\n+\n+\t/* Finally read it back in */\n+\tif (read_cache_from(tmp_index) < 0)\n+\t\tdie(\"Error reading index '%s'\", tmp_index);\n+\tactive_cache_tree = cache_tree();\n+\tif (cache_tree_update(active_cache_tree, active_cache, active_nr,\n+\t\t\t 0, 0) < 0)\n+\t\tdie(\"Error building trees\");\n+\thex = sha1_to_hex(active_cache_tree->sha1);\n+\tdiscard_cache();\n+\n+\tunlink(tmp_index);\n+\n+\treturn hex;\n+}\n+\n+static char *rewrite_header(char *dest, unsigned long *len_p, char **buf_p,\n+\t\t\t struct commit *commit)\n+{\n+\tstruct commit_list *parents = commit->parents;\n+\tint linelen;\n+\n+\tdo {\n+\t\tchar *line = *buf_p;\n+\t\tlinelen = skip_one_line(buf_p, len_p);\n+\n+\t\tif (!linelen)\n+\t\t\treturn dest;\n+\n+\t\tif (index_filter && !memcmp(line, \"tree \", 5)) {\n+\t\t\tif (linelen != 46)\n+\t\t\t\tdie(\"bad tree line in commit\");\n+\t\t\tmemcpy(dest, \"tree \", 5);\n+\t\t\tline[45] = '\\0';\n+\t\t\tmemcpy(dest+5, filter_index(line+5, commit), 40);\n+\t\t\tline[45] = '\\n';\n+\t\t\tdest[45] = '\\n';\n+\t\t\tdest += 46;\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\t/* drop old parents */\n+\t\tif (!memcmp(line, \"parent \", 7)) {\n+\t\t\tif (linelen != 48)\n+\t\t\t\tdie(\"bad parent line in commit\");\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\t/* insert new parents before author */\n+\t\tif (!memcmp(line, \"author \", 7))\n+\t\t\tdest = add_parents(dest, parents);\n+\n+\t\tmemcpy(dest, line, linelen);\n+\t\tdest += linelen;\n+\t} while (linelen > 1);\n+\treturn dest;\n+}\n+\n+static size_t hex_len(const char *s, size_t n)\n+{\n+\tsize_t offset;\n+\n+\tfor (offset = 0; offset < n; ++offset)\n+\t\tif (!ishex(s[offset]))\n+\t\t\tbreak;\n+\treturn offset;\n+}\n+\n+static size_t non_hex_len(const char *s, size_t n)\n+{\n+\tsize_t offset;\n+\n+\tfor (offset = 0; offset < n; ++offset)\n+\t\tif (ishex(s[offset]))\n+\t\t\tbreak;\n+\treturn offset;\n+}\n+\n+/* Replace any (short) sha1 of a rewritten commit by the new (short) sha1 */\n+static char *rewrite_body(char *dest, unsigned long len, char *buf)\n+{\n+\tunsigned char sha1[20];\n+\n+\twhile (len) {\n+\t\tsize_t ll = non_hex_len(buf, len);\n+\t\tmemcpy(dest, buf, ll);\n+\t\tdest += ll;\n+\t\tbuf += ll;\n+\t\tlen -= ll;\n+\n+\t\tll = hex_len(buf, len);\n+\t\tif (ll >= 8 && ll <= 40 &&\n+\t\t !get_short_sha1(buf, ll, sha1, 1) &&\n+\t\t !get_rewritten_sha1(sha1))\n+\t\t\tmemcpy(dest, sha1_to_hex(sha1), ll);\n+\t\telse\n+\t\t\tmemcpy(dest, buf, ll);\n+\t\tdest += ll;\n+\t\tbuf += ll;\n+\t\tlen -= ll;\n+\t}\n+\treturn dest;\n+}\n+\n+static void write_ref_sha1_or_die(const char *ref, const unsigned char *old_sha1,\n+\t\t\t\t const unsigned char *new_sha1,\n+\t\t\t\t const char *logmsg, int flags)\n+{\n+\tstruct ref_lock *lock;\n+\n+\tlock = lock_any_ref_for_update(ref, old_sha1, flags);\n+\tif (!lock)\n+\t\tdie(\"%s: cannot lock the ref\", ref);\n+\tif (write_ref_sha1(lock, new_sha1, logmsg) < 0)\n+\t\tdie(\"%s: cannot update the ref\", ref);\n+}\n+\n+static int is_ref_to_be_rewritten(const char *ref)\n+{\n+\tunsigned char sha1[20];\n+\tint flag;\n+\n+\tif (prefixcmp(ref, \"refs/\"))\n+\t\treturn 0;\n+\tif (!prefixcmp(ref, \"refs/remotes/\"))\n+\t\treturn 0;\n+\tif (!prefixcmp(ref+5, original_prefix))\n+\t\treturn 0;\n+\n+\tresolve_ref(ref, sha1, 0, &flag);\n+\tif (flag & REF_ISSYMREF)\n+\t\treturn 0;\n+\n+\treturn 1;\n+}\n+\n+static int is_pruned(struct commit *commit, int path_pruning)\n+{\n+\tif (commit->object.flags & PRUNED)\n+\t\treturn 1;\n+\tif (path_pruning &&\n+\t !(commit->object.flags & (TREECHANGE | UNINTERESTING)))\n+\t\treturn 1;\n+\treturn 0;\n+}\n+\n+static void rewrite_sha1(struct object *obj, unsigned char *new_sha1)\n+{\n+\tif (!hashcmp(obj->sha1, new_sha1))\n+\t\treturn;\n+\n+\tadd_rewrite_decoration(obj, new_sha1);\n+}\n+\n+/*\n+ * Replace any parent that has been removed by its parents\n+ * and return the number of new parents.\n+ * We directly modify the parent list, so any libification\n+ * should probably adapt this function.\n+ */\n+static int rewrite_parents(struct commit *commit, int path_pruning)\n+{\n+\tint n;\n+\tstruct commit_list *list, *parents, **prev;\n+\tunsigned char sha1[20];\n+\n+\tfor (n = 0, prev = &commit->parents; *prev; ++n) {\n+\t\tlist = *prev;\n+\n+\t\trewrite_parents(list->item, path_pruning);\n+\t\tif (!is_pruned(list->item, path_pruning)) {\n+\t\t\tprev = &list->next;\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\thashcpy(sha1, list->item->object.sha1);\n+\t\tget_rewritten_sha1(sha1);\n+\t\tif (!is_null_sha1(sha1)) {\n+\t\t\thashclr(sha1);\n+\t\t\trewrite_sha1(&list->item->object, sha1);\n+\t\t}\n+\n+\t\tparents = list->item->parents;\n+\t\tif (!parents) {\n+\t\t\t*prev = list->next;\n+\t\t\tfree(list);\n+\t\t\t--n;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tlist->item = parents->item;\n+\t\tprev = &list->next;\n+\t\tlist = list->next;\n+\t\twhile ((parents = parents->next)) {\n+\t\t\tcommit_list_insert(parents->item, prev);\n+\t\t\tprev = &(*prev)->next;\n+\t\t\t++n;\n+\t\t}\n+\t\t*prev = list;\n+\t}\n+\n+\treturn n;\n+}\n+\n+static int rewrite_ref(const char *refname, const unsigned char *sha1,\n+\t\t\tint flags, void *cb_data)\n+{\n+\tint prefix_len;\n+\tint len;\n+\tchar buffer[256], *p;\n+\tstruct object *obj = parse_object(sha1);\n+\tunsigned char new_sha1[20];\n+\tstruct commit *commit;\n+\tint pruned;\n+\n+\tif (!obj)\n+\t\treturn 0;\n+\tif (obj->type == OBJ_TAG)\n+\t\treturn 0;\n+\tif (!is_ref_to_be_rewritten(refname))\n+\t\treturn 0;\n+\n+\tcommit = lookup_commit_reference(sha1);\n+\tpruned = is_pruned(commit, !!cb_data);\n+\n+\thashcpy(new_sha1, sha1);\n+\tif (!pruned && get_rewritten_sha1(new_sha1))\n+\t\treturn 0;\n+\n+\tprefix_len = strlen(original_prefix);\n+\tlen = strlen(refname);\n+\n+\tif (len + prefix_len + 1 + 1 > sizeof(buffer))\n+\t\tdie(\"rewrite ref of '%s' too long\", refname);\n+\tp = buffer;\n+\tmemcpy(p, \"refs/\", 5);\n+\tp += 5;\n+\tmemcpy(p, original_prefix, prefix_len);\n+\tp += prefix_len;\n+\tmemcpy(p, refname+4, len-4 + 1);\n+\n+\tif (safe_create_leading_directories(git_path(buffer)))\n+\t\tdie(\"Unable to create leading directories of '%s'\", buffer);\n+\twrite_ref_sha1_or_die(buffer, NULL, obj->sha1,\n+\t\t\t\t\"copied during rewrite\", 0);\n+\n+\tif (pruned) {\n+\t\tint i = 1;\n+\t\tdelete_ref(refname, obj->sha1);\n+\t\tmkdir(git_path(refname), 0777);\n+\t\tstruct commit_list *parent;\n+\t\trewrite_parents(commit, !!cb_data);\n+\t\tfor (parent = commit->parents; parent; parent = parent->next) {\n+\t\t\thashcpy(new_sha1, parent->item->object.sha1);\n+\t\t\tget_rewritten_sha1(new_sha1);\n+\t\t\tsnprintf(buffer, sizeof(buffer), \"%s/%d\", refname, i);\n+\t\t\twrite_ref_sha1_or_die(buffer, NULL, new_sha1,\n+\t\t\t\t\t\t\"ancestor of pruned\", 0);\n+\t\t\t++i;\n+\t\t}\n+\t\treturn 0;\n+\t}\n+\telse\n+\t\twrite_ref_sha1_or_die(refname, obj->sha1, new_sha1,\n+\t\t\t\t\t\"rewritten\", 0);\n+\n+\treturn 0;\n+}\n+\n+static void add_sha1_map(const char *old_sha1, const char *new_sha1)\n+{\n+\tint fd;\n+\n+\tif (!write_sha1_mapping)\n+\t\treturn;\n+\tif (new_sha1 && !hashcmp(old_sha1, new_sha1))\n+\t\treturn;\n+\n+\tfd = open(mkpath(\"%s/map/%s\", rewrite_dir, sha1_to_hex(old_sha1)),\n+\t\t O_CREAT | O_WRONLY | O_APPEND, 0777);\n+\tif (fd < 0)\n+\t\tdie(\"unable to open map file '%s/map/%s'\", rewrite_dir,\n+\t\t\tsha1_to_hex(old_sha1));\n+\tif (new_sha1)\n+\t\twrite_or_die(fd, sha1_to_hex(new_sha1), 41);\n+\tclose(fd);\n+}\n+\n+static void filter_and_write_commit(char *commit_body, size_t len,\n+\t\t\t\t struct commit *commit, unsigned char *sha1)\n+{\n+\tchar commit_path[PATH_MAX];\n+\tstruct child_process cmd;\n+\tint argc;\n+\tconst char *argv[10];\n+\tint fd;\n+\tconst char *env[] = { commit_env, NULL };\n+\tchar hex[41];\n+\tstruct commit_list *list = NULL, **end = &list;\n+\n+\tmemcpy(commit_env+sizeof(commit_env)-41,\n+\t\tsha1_to_hex(commit->object.sha1), 40);\n+\n+\tfd = git_mkstemp(commit_path, sizeof(commit_path), \".commit_XXXXXX\");;\n+\twrite_or_die(fd, commit_body, len);\n+\n+\targc = 0;\n+\targv[argc++] = \"sh\";\n+\targv[argc++] = \"-c\";\n+\targv[argc++] = commit_filter;\n+\targv[argc] = NULL;\n+\tmemset(&cmd, 0, sizeof(cmd));\n+\tcmd.in = open(commit_path, O_RDONLY);\n+\tif (cmd.in < 0)\n+\t\tdie(\"Unable to read commit from file '%s'\", commit_path);\n+\tunlink(commit_path);\n+\tcmd.out = -1;\n+\tcmd.argv = argv;\n+\tcmd.dir = rewrite_dir;\n+\tcmd.env = env;\n+\tif (start_command(&cmd))\n+\t\tdie(\"Commit filter '%s' failed to start\", commit_filter);\n+\twhile (xread(cmd.out, hex, 41) == 41) {\n+\t\tstruct commit *new_commit;\n+\t\thex[40] = '\\0';\n+\t\tif (get_sha1(hex, sha1))\n+\t\t\tdie(\"Unexpected output from commit filter: '%s'\",\n+\t\t\t hex);\n+\t\tif (!(new_commit = lookup_commit_reference(sha1)))\n+\t\t\tdie(\"Invalid SHA1 in output from commit filter: '%s'\",\n+\t\t\t hex);\n+\t\tcommit_list_insert(new_commit, end);\n+\t\tadd_sha1_map(commit->object.sha1, new_commit->object.sha1);\n+\t\tend = &(*end)->next;\n+\t}\n+\tif (list && !list->next)\n+\t\tfree_commit_list(list);\n+\telse {\n+\t\thashclr(sha1);\n+\t\tcommit->object.flags |= PRUNED;\n+\t\t/*\n+\t\t * If the filter returns two or more commits,\n+\t\t * we consider the original commit to have been\n+\t\t * removed and put the list in the old commit's\n+\t\t * parent list so that all the old commit's children\n+\t\t * will copy them.\n+\t\t */\n+\t\tif (list) {\n+\t\t\tfree_commit_list(commit->parents);\n+\t\t\tcommit->parents = list;\n+\t\t} else\n+\t\t add_sha1_map(commit->object.sha1, NULL);\n+\t}\n+\tif (finish_command(&cmd))\n+\t\tdie(\"Commit filter '%s' failed\", commit_filter);\n+\tclose(cmd.in);\n+}\n+\n+static void rewrite_commit(struct commit *commit, int path_pruning)\n+{\n+\tchar *buf, *p;\n+\tint n;\n+\tchar *orig_buf = commit->buffer;\n+\tunsigned long orig_len = strlen(orig_buf);\n+\tunsigned char sha1[20];\n+\n+\tn = rewrite_parents(commit, path_pruning);\n+\n+\t/* Make enough remove for n (possibly extra) parents */\n+\tp = buf = xmalloc(orig_len + n*48);\n+\tp = rewrite_header(p, &orig_len, &orig_buf, commit);\n+\tp = rewrite_body(p, orig_len, orig_buf);\n+\tif (!commit_filter) {\n+\t\tif (write_sha1_file(buf, p-buf, commit_type, sha1))\n+\t\t\tdie(\"Unable to write new commit\");\n+\t\tadd_sha1_map(commit->object.sha1, sha1);\n+\t} else\n+\t\tfilter_and_write_commit(buf, p-buf, commit, sha1);\n+\tfree(buf);\n+\n+\trewrite_sha1(&commit->object, sha1);\n+}\n+\n+static void move_head_forward(char *ref, unsigned char *old_sha1, int detached,\n+\t\t\t\tint path_limiting)\n+{\n+\tunsigned char new_sha1[20];\n+\tint rewritten = 0;\n+\tstruct commit *commit;\n+\n+\tcommit = lookup_commit_reference(old_sha1);\n+\tif (is_pruned(commit, path_limiting)) {\n+\t\tif (commit->parents) {\n+\t\t\thashcpy(new_sha1, commit->parents->item->object.sha1);\n+\t\t\tget_rewritten_sha1(new_sha1);\n+\t\t\trewritten = 1;\n+\t\t} else\n+\t\t\treturn;\n+\t} else {\n+\t\thashcpy(new_sha1, old_sha1);\n+\t\trewritten = !get_rewritten_sha1(new_sha1);\n+\t}\n+\tif (rewritten && !is_bare_repository()) {\n+\t\tint argc;\n+\t\tconst char *argv[10];\n+\n+\t\targc = 0;\n+\t\targv[argc++] = \"read-tree\";\n+\t\targv[argc++] = \"-m\";\n+\t\targv[argc++] = \"-u\";\n+\t\targv[argc++] = sha1_to_hex(old_sha1);\n+\t\targv[argc++] = sha1_to_hex(new_sha1);\n+\t\targv[argc] = NULL;\n+\t\tif (run_command_v_opt(argv, RUN_GIT_CMD))\n+\t\t\tdie(\"Cannot move HEAD forward\");\n+\t}\n+\tif (!detached) {\n+\t\tunsigned char sha1[20];\n+\n+\t\tif (resolve_ref(ref, sha1, 1, NULL))\n+\t\t\tcreate_symref(\"HEAD\", ref, \"reattached after rewrite\");\n+\t\telse {\n+\t\t\tchar buffer[256];\n+\t\t\tsnprintf(buffer, sizeof(buffer), \"%s/1\", ref);\n+\t\t\tif (resolve_ref(buffer, sha1, 1, NULL))\n+\t\t\t\tcreate_symref(\"HEAD\", buffer,\n+\t\t\t\t\t\"attached to ancestor of pruned commit\");\n+\t\t}\n+\t}\n+\telse if (rewritten)\n+\t\twrite_ref_sha1_or_die(\"HEAD\", NULL, new_sha1,\n+\t\t\t\t \"rewritten\", REF_NODEREF);\n+}\n+\n+static char aux_functions[] =\n+\"export GIT_REWRITE_DIR=`pwd`\\n\"\n+\"commit()\\n\"\n+\"{\\n\"\n+\"\tgit-hash-object -w -t commit --stdin\\n\"\n+\"}\\n\"\n+\"map()\\n\"\n+\"{\\n\"\n+\"\t# if it was not rewritten, take the original\\n\"\n+\"\tif test -r \\\"$GIT_REWRITE_DIR/map/$1\\\"\\n\"\n+\"\tthen\\n\"\n+\"\t\tcat \\\"$GIT_REWRITE_DIR/map/$1\\\"\\n\"\n+\"\telse\\n\"\n+\"\t\techo \\\"$1\\\"\\n\"\n+\"\tfi\\n\"\n+\"}\\n\"\n+\"cd t\\n\";\n+\n+static char filter_prefix[] = \". aux;\";\n+\n+static char *create_filter(const char *command)\n+{\n+\tint prefix_len = sizeof(filter_prefix)-1;\n+\tint command_len = strlen(command);\n+\tchar *filter = xmalloc(prefix_len + command_len + 1);\n+\n+\tmemcpy(filter, filter_prefix, prefix_len);\n+\tmemcpy(filter+prefix_len, command, command_len+1);\n+\treturn filter;\n+}\n+\n+static const char *create_absolute_path(const char *path)\n+{\n+\tint path_len;\n+\tstatic char cwd[PATH_MAX];\n+\tstatic int cwd_len = -1;\n+\tchar *absolute;\n+\n+\tif (path[0] == '/')\n+\t\treturn path;\n+\n+\tif (cwd_len == -1) {\n+\t\tif (!getcwd(cwd, sizeof(cwd)))\n+\t\t\tdie(\"unable to get current working directory\");\n+\t\tcwd_len = strlen(cwd);\n+\t}\n+\n+\tpath_len = strlen(path);\n+\tabsolute = xmalloc(cwd_len+path_len+2);\n+\tmemcpy(absolute, cwd, cwd_len);\n+\tabsolute[cwd_len] = '/';\n+\tmemcpy(absolute+cwd_len+1, path, path_len+1);\n+\treturn absolute;\n+}\n+\n+static int rm_rf(const char *dir)\n+{\n+\tint argc;\n+\tconst char *argv[10];\n+\n+\targc = 0;\n+\targv[argc++] = \"rm\";\n+\targv[argc++] = \"-rf\";\n+\targv[argc++] = dir;\n+\targv[argc] = NULL;\n+\treturn run_command_v_opt(argv, 0);\n+}\n+\n+static void cleanup_temp_dir()\n+{\n+\tif (rm_rf(rewrite_dir))\n+\t\tdie(\"Unable to clean up rewrite directory '%s'\", rewrite_dir);\n+}\n+\n+static void setup_temp_dir()\n+{\n+\tint aux;\n+\n+\tabsolute_git_dir = create_absolute_path(get_git_dir());\n+\tsetenv(GIT_DIR_ENVIRONMENT, absolute_git_dir, 1);\n+\n+\tif (!rewrite_dir) {\n+\t\trewrite_dir = xstrdup(git_path(\"rewrite\"));\n+\t}\n+\tif (mkdir(rewrite_dir, 0777)) {\n+\t\tif (errno == EEXIST)\n+\t\t\tdie(\"rewrite directory '%s' already exists\",\n+\t\t\t\trewrite_dir);\n+\t\tdie(\"unable to create rewrite directory '%s'\", rewrite_dir);\n+\t}\n+\tatexit(cleanup_temp_dir);\n+\n+\tif (mkdir(mkpath(\"%s/map\", rewrite_dir), 0777))\n+\t\tdie(\"unable to create map directory '%s/map'\", rewrite_dir);\n+\tif (mkdir(mkpath(\"%s/t\", rewrite_dir), 0777))\n+\t\tdie(\"unable to create temp directory '%s/t'\", rewrite_dir);\n+\taux = open(mkpath(\"%s/aux\", rewrite_dir), O_CREAT | O_WRONLY, 0777);\n+\tif (aux < 0)\n+\t\tdie(\"unable to create aux file '%s/aux'\", rewrite_dir);\n+\twrite_or_die(aux, aux_functions, sizeof(aux_functions)-1);\n+\tclose(aux);\n+}\n+\n+int cmd_rewrite_commits(int argc, const char **argv, const char *prefix)\n+{\n+\tstruct rev_info rev;\n+\tstruct commit *commit;\n+\tunsigned char HEAD_sha1[20];\n+\tchar *HEAD_ref;\n+\tint flag;\n+\tint i, j;\n+\n+\tinit_revisions(&rev, prefix);\n+\n+\tfor (i = 1, j = 1; i < argc; ++i) {\n+\t\tif (!strcmp(argv[i], \"--index-filter\")) {\n+\t\t\tif (++i == argc)\n+\t\t\t\tdie(\"Argument required for --index-filter\");\n+\t\t\tindex_filter = argv[i];\n+\t\t} else if (!strcmp(argv[i], \"--commit-filter\")) {\n+\t\t\tif (++i == argc)\n+\t\t\t\tdie(\"Argument required for --commit-filter\");\n+\t\t\tcommit_filter = argv[i];\n+\t\t} else if (!strcmp(argv[i], \"--write-sha1-mapping\")) {\n+\t\t\twrite_sha1_mapping = 1;\n+\t\t} else\n+\t\t\targv[j++] = argv[i];\n+\t}\n+\targc = j;\n+\targc = setup_revisions(argc, argv, &rev, \"HEAD\");\n+\trev.ignore_merges = 0;\n+\trev.topo_order = 1;\n+\trev.reverse = 1;\n+\trev.parents = 1;\n+\n+\tif (index_filter || commit_filter)\n+\t\tsetup_temp_dir();\n+\telse\n+\t\t/* They'll never know. BWUHAHA */\n+\t\twrite_sha1_mapping = 0;\n+\n+\tif (index_filter)\n+\t\tindex_filter = create_filter(index_filter);\n+\tif (commit_filter)\n+\t\tcommit_filter = create_filter(commit_filter);\n+\n+\tprepare_revision_walk(&rev);\n+\twhile ((commit = get_revision(&rev)) != NULL) {\n+\t\trewrite_commit(commit, !!rev.prune_fn);\n+\t}\n+\n+\tHEAD_ref = xstrdup(resolve_ref(\"HEAD\", HEAD_sha1, 1, &flag));\n+\tif (flag & REF_ISSYMREF) {\n+\t\t/* Detach HEAD at its current position */\n+\t\twrite_ref_sha1_or_die(\"HEAD\", NULL, HEAD_sha1,\n+\t\t\t\t\t\"detached for rewrite\", REF_NODEREF);\n+\t}\n+\n+\trm_rf(git_path(\"refs/%s\", original_prefix));\n+\tfor_each_ref(rewrite_ref, rev.prune_fn);\n+\n+\tmove_head_forward(HEAD_ref, HEAD_sha1, !(flag & REF_ISSYMREF),\n+\t\t\t !!rev.prune_fn);\n+\tfree(HEAD_ref);\n+\n+\treturn 0;\n+}\ndiff --git a/builtin.h b/builtin.h\nindex 4cc228d..5a08ec8 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -63,6 +63,7 @@ extern int cmd_rerere(int argc, const char **argv, const char *prefix);\n extern int cmd_rev_list(int argc, const char **argv, const char *prefix);\n extern int cmd_rev_parse(int argc, const char **argv, const char *prefix);\n extern int cmd_revert(int argc, const char **argv, const char *prefix);\n+extern int cmd_rewrite_commits(int argc, const char **argv, const char *prefix);\n extern int cmd_rm(int argc, const char **argv, const char *prefix);\n extern int cmd_runstatus(int argc, const char **argv, const char *prefix);\n extern int cmd_shortlog(int argc, const char **argv, const char *prefix);\ndiff --git a/git.c b/git.c\nindex a647f9c..ce02b0b 100644\n--- a/git.c\n+++ b/git.c\n@@ -356,6 +356,7 @@ static void handle_internal_command(int argc, const char **argv)\n \t\t{ \"rev-list\", cmd_rev_list, RUN_SETUP },\n \t\t{ \"rev-parse\", cmd_rev_parse, RUN_SETUP },\n \t\t{ \"revert\", cmd_revert, RUN_SETUP | NEED_WORK_TREE },\n+\t\t{ \"rewrite-commits\", cmd_rewrite_commits, RUN_SETUP },\n \t\t{ \"rm\", cmd_rm, RUN_SETUP | NEED_WORK_TREE },\n \t\t{ \"runstatus\", cmd_runstatus, RUN_SETUP | NEED_WORK_TREE },\n \t\t{ \"shortlog\", cmd_shortlog, RUN_SETUP | USE_PAGER },\ndiff --git a/t/t7005-rewrite-commits.sh b/t/t7005-rewrite-commits.sh\nnew file mode 100755\nindex 0000000..eef4129\n--- /dev/null\n+++ b/t/t7005-rewrite-commits.sh\n@@ -0,0 +1,109 @@\n+#!/bin/sh\n+\n+test_description='git-rewrite-commits'\n+. ./test-lib.sh\n+\n+make_commit () {\n+\tlower=$(echo $1 | tr A-Z a-z)\n+\techo $lower > $lower\n+\tgit add $lower\n+\ttest_tick\n+\tgit commit -m $1\n+\tgit tag $1\n+}\n+\n+test_expect_success 'setup' '\n+\tmake_commit A\n+\tmake_commit B\n+\tgit checkout -b branch B\n+\tmake_commit D\n+\tmake_commit E\n+\tgit checkout master\n+\tmake_commit C\n+\tgit checkout branch\n+\tgit merge C\n+\tgit tag F\n+\tmake_commit G\n+\tmake_commit H\n+'\n+\n+orig_H=$(git rev-parse H)\n+test_expect_success 'rewrite identically' '\n+\tgit-rewrite-commits\n+'\n+\n+test_expect_success 'result is really identical' '\n+\ttest $orig_H = $(git rev-parse H)\n+'\n+\n+test_expect_success 'rewrite identically using commit filter' '\n+\tgit-rewrite-commits --commit-filter=\"commit\"\n+'\n+\n+test_expect_success 'result is really identical' '\n+\ttest $orig_H = $(git rev-parse H)\n+'\n+\n+# for lack of 'git-mv --cached d doh'\n+test_expect_success 'rewrite, renaming a specific file' '\n+\tgit-rewrite-commits --index-filter \\\n+\t\t\"git-ls-files -s | sed \\\"s-\\\\td\\\\\\$-\\\\tdoh-\\\" |\n+\t GIT_INDEX_FILE=\\$GIT_INDEX_FILE.new \\\n+\t\t git-update-index --index-info &&\n+\t\t mv \\$GIT_INDEX_FILE.new \\$GIT_INDEX_FILE\"\n+'\n+\n+test_expect_success 'test that the file was renamed' '\n+\ttest d = $(git-show H:doh) &&\n+\t! git-show H:d --\n+'\n+\n+orig_H=$(git rev-parse H)\n+test_expect_success 'use index-filter to move into a subdirectory' '\n+\tgit-rewrite-commits --index-filter \\\n+\t\t \"git ls-files -s | sed \\\"s-\\\\t-&newsubdir/-\\\" |\n+\t GIT_INDEX_FILE=\\$GIT_INDEX_FILE.new \\\n+\t\t\tgit update-index --index-info &&\n+\t\t mv \\$GIT_INDEX_FILE.new \\$GIT_INDEX_FILE\" &&\n+\ttest -z \"$(git diff $orig_H H:newsubdir)\"'\n+\n+test_expect_success 'remove merge commit' '\n+\tgit-rewrite-commits --grep=\"!Merge\" &&\n+\ttest 2 = `git-log ^G^@ G --pretty=format:%P | wc -w`'\n+\n+test_expect_success 'remove new merge commit using commit filter' '\n+\tgit-rewrite-commits --commit-filter \\\n+\t\t\"if test \\$GIT_COMMIT = $(git rev-parse G); then \\\n+\t\t\tcat > /dev/null; \\\n+\t\t else \\\n+\t\t\tcommit; \\\n+\t\t fi\" &&\n+\ttest 2 = `git-log ^H^@ H --pretty=format:%P | wc -w`'\n+\n+test_expect_success 'remove first commit' '\n+\tgit-rewrite-commits --grep=\"!A\"'\n+\n+test_expect_success 'stops when index filter fails' '\n+\t! git-rewrite-commits --index-filter false &&\n+\tgit-checkout branch\n+'\n+\n+test_expect_success 'author information is preserved' '\n+\t: > i &&\n+\tgit add i &&\n+\ttest_tick &&\n+\tGIT_AUTHOR_NAME=\"B V Uips\" git commit -m bvuips &&\n+\tgit-rewrite-commits --commit-filter \"(cat; \\\n+\t\t\ttest \\$GIT_COMMIT != $(git rev-parse master) || \\\n+\t\t\techo Hallo) | commit\" &&\n+\ttest 1 = $(git rev-list --author=\"B V Uips\" HEAD | wc -l)\n+'\n+\n+test_expect_success 'use index-filter to select a subdirectory' '\n+\tgit-rewrite-commits --index-filter \\\n+\t\t \"git read-tree \\$GIT_COMMIT:newsubdir\" -- newsubdir &&\n+\ttest 0 = $(git rev-list HEAD -- i | wc -l) &&\n+\ttest 0 = $(git rev-list HEAD -- newsubdir | wc -l)\n+'\n+\n+test_done\n-- \n1.5.3.rc0.100.ge60b4\n"},{"id":"47242","messageId":"20070713080100.GN1528MdfPADPa@greensroom.kotnet.org","threadId":"9008","inReplyTo":"11842671631635-git-send-email-skimo@liacs.nl","subject":"Re: [PATCH 6/6] Add git-rewrite-commits","fromName":"Sven Verdoolaege","fromEmail":"skimo@kotnet.org","sentAt":"2007-07-13T08:01:00Z","receivedAt":"2007-07-13T08:01:00Z","isPatch":true,"sender":{"key":"skimo@kotnet.org","avatar":null},"body":"On Thu, Jul 12, 2007 at 09:06:03PM +0200, skimo@liacs.nl wrote:\n> +static int rewrite_parents(struct commit *commit, int path_pruning)\n> +{\n> +\tint n;\n> +\tstruct commit_list *list, *parents, **prev;\n> +\tunsigned char sha1[20];\n> +\n> +\tfor (n = 0, prev = &commit->parents; *prev; ++n) {\n> +\t\tlist = *prev;\n> +\n> +\t\trewrite_parents(list->item, path_pruning);\n> +\t\tif (!is_pruned(list->item, path_pruning)) {\n> +\t\t\tprev = &list->next;\n> +\t\t\tcontinue;\n> +\t\t}\n\nOops... that should be the other way around...\n\ndiff --git a/builtin-rewrite-commits.c b/builtin-rewrite-commits.c\nindex d95a16c..4cd17ae 100644\n--- a/builtin-rewrite-commits.c\n+++ b/builtin-rewrite-commits.c\n@@ -279,11 +279,11 @@ static int rewrite_parents(struct commit *commit, int path_pruning)\n \tfor (n = 0, prev = &commit->parents; *prev; ++n) {\n \t\tlist = *prev;\n \n-\t\trewrite_parents(list->item, path_pruning);\n \t\tif (!is_pruned(list->item, path_pruning)) {\n \t\t\tprev = &list->next;\n \t\t\tcontinue;\n \t\t}\n+\t\trewrite_parents(list->item, path_pruning);\n \n \t\thashcpy(sha1, list->item->object.sha1);\n \t\tget_rewritten_sha1(sha1);\n\nI'll include it in my next version.\n\nskimo\n"},{"id":"47338","messageId":"Pine.LNX.4.64.0707141115270.14781@racer.site","threadId":"9008","inReplyTo":"11842671632000-git-send-email-skimo@liacs.nl","subject":"Re: [PATCH 3/6] Define ishex(x) in git-compat-util.h","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-14T10:18:15Z","receivedAt":"2007-07-14T10:18:15Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 12 Jul 2007, skimo@liacs.nl wrote:\n\n> diff --git a/git-compat-util.h b/git-compat-util.h\n> index 362e040..1a36f4c 100644\n> --- a/git-compat-util.h\n> +++ b/git-compat-util.h\n> @@ -325,12 +325,15 @@ static inline int has_extension(const char *filename, const char *ext)\n> extern unsigned char sane_ctype[256];\n> #define GIT_SPACE 0x01\n> #define GIT_DIGIT 0x02\n> -#define GIT_ALPHA 0x04\n> +#define GIT_HEXAL 0x04\n> +#define GIT_OTHAL 0x08\n> +#define GIT_ALPHA (GIT_HEXAL | GIT_OTHAL)\n\nI'd have left GIT_ALPHA, and added GIT_HEXDIGIT.\n\n> #define sane_istest(x,mask) ((sane_ctype[(unsigned char)(x)] & (mask)) != 0)\n> #define isspace(x) sane_istest(x,GIT_SPACE)\n> #define isdigit(x) sane_istest(x,GIT_DIGIT)\n> #define isalpha(x) sane_istest(x,GIT_ALPHA)\n> #define isalnum(x) sane_istest(x,GIT_ALPHA | GIT_DIGIT)\n> +#define ishex(x) sane_istest(x,GIT_HEXAL | GIT_DIGIT)\n\nI know, I originally proposed this. In the mean time, however, I found \nthat this gem should be even better (in terms of diff size):\n\n#define ishex(x) (hexval(x) >= 0)\n\nCiao,\nDscho\n"},{"id":"47348","messageId":"Pine.LNX.4.64.0707141140510.14781@racer.site","threadId":"9008","inReplyTo":"11842671631635-git-send-email-skimo@liacs.nl","subject":"Re: [PATCH 6/6] Add git-rewrite-commits","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-14T12:49:59Z","receivedAt":"2007-07-14T12:49:59Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 12 Jul 2007, skimo@liacs.nl wrote:\n\n> +SYNOPSIS\n> +--------\n> +'git-rewrite-commits' [--index-filter ] [--commit-filter ]\n> +\t[...]\n\n--write-sha1-mappings is missing.\n\n> +Filters\n> +~~~~~~~\n> +\n> +The filters are applied in the order as listed below. The \n> +argument is run as \"sh -c '\", with the $GIT_COMMIT\n> +environment variable set to the commit that is being rewritten.\n> +If any call to a filter fails, then git-rewrite-commits will abort.\n\nHere you should note that a couple of helper functions are defined for \nease of use.\n\n> +--write-sha1-mapping\n> +\tWrite mapping of old SHA1s to new SHA1s for use in filters.\n\nWhere? How to use it?\n\n> +::\n> +\tSelects the commits to be rewritten, defaulting to the history\n> +\tthat lead to HEAD.\n\ns/the history that lead to HEAD/the current branch/\n\n> +Examples\n> +--------\n> +\n> +Suppose you want to remove a file (containing confidential information\n> +or copyright violation) from all commits:\n> +\n> +----------------------------------------------------------------------------\n> +git rewrite-commits --index-filter 'git update-index --remove filename || :'\n\nWe seem to prefer \"$ git\" instead of just \"git\" in the other man pages' \nexamples.\n\n> +----------------------------------------------------------------------------\n> +\n> +Now, you will get the rewritten history saved in your current branch\n> +(the old branch is saved in refs/original).\n\n\t\t\t\t\t\tThe \"|| :\" construct \n+ prevents the filter to fail when the given file was not present in the \n+ index.\n\n> +To move the whole tree into a subdirectory, or remove it from there:\n> +\n> +---------------------------------------------------------------\n> +git rewrite-commits --index-filter \\\n> +\t'git ls-files -s | sed \"s-\\t-&newsubdir/-\" |\n> +\t\tGIT_INDEX_FILE=$GIT_INDEX_FILE.new \\\n> +\t\t\tgit update-index --index-info &&\n> +\t mv $GIT_INDEX_FILE.new $GIT_INDEX_FILE'\n> +---------------------------------------------------------------\n\nI imagine that this should be replaced (later! in a different patch!) by \neither --subdir-filter, or by a shell helper function.\n\n> +#include \"grep.h\"\n\nI did not compile test, but do you really need that?\n\n> +struct decoration rewrite_decoration = { \"rewritten as\" };\n> [...]\n> +\n> +struct rewrite_decoration {\n> +\tstruct rewrite_decoration *next;\n> +\tunsigned char sha1[20];\n> +};\n\nI wonder why you give the instance of a \"struct decoration\" the same name \nas that of a _different_ struct. IOW it is confusing that there is a \n\"struct rewrite_decoration\", but the variable \"rewrite_decoration\" is no \ninstance of that struct.\n\n> +static void add_rewrite_decoration(struct object *obj, unsigned char *sha1)\n\nWhy \"struct object *\"? You only rewrite commits. Which makes me suspect \nthat your \"struct rewrite_decoration\" should be a \"struct commit_list\" to \nbegin with.\n\n> +static char *add_parents(char *dest, struct commit_list *parents)\n\nSince one commit can be rewritten to multiple commits now, you do not know \nhow much space you need to add the parents. Thus you should take a buf_p \nand a space_p, and use ALLOC_GROW() to grow the buffer as needed.\n\n> +static int skip_one_line(char **buf_p, unsigned long *len_p)\n\nBetter rename get_one_line to get_line_length and export it, avoiding code \nduplication. I have an upcoming patch series adding commit notes, which \nhas that patch.\n\nBesides, you do not _skip_ that line. You use the line, but advance \n*buf_p before doing so.\n\n> +static char *filter_index(char *orig_hex, struct commit *commit)\n\nconst char *, in both cases. You do not plan to modify orig_hex, and you \nreturn a sha1_to_hex() buffer, which you also do not plan to modify.\n\n> +{\n> +\tint argc;\n> +\tconst char *argv[10];\n> +\tstatic char index_env[16+PATH_MAX];\n\nStyle: \"16 + PATH_MAX\". That happens elsewhere, too.\n\n> +/* Replace any (short) sha1 of a rewritten commit by the new (short) sha1 */\n> +static char *rewrite_body(char *dest, unsigned long len, char *buf)\n> +{\n> +\tunsigned char sha1[20];\n> +\n> +\twhile (len) {\n> +\t\tsize_t ll = non_hex_len(buf, len);\n> +\t\tmemcpy(dest, buf, ll);\n> +\t\tdest += ll;\n> +\t\tbuf += ll;\n> +\t\tlen -= ll;\n\nWhy not make it simpler?\n\n\t\twhile (!ishex(buf[i]))\n\t\t\tdest[i] = buf[i++];\n\n> +\t\tll = hex_len(buf, len);\n\nIt is really shorter (because you spare the whole hex_len() function, to \nsay\n\n\t\tfor (ll = 0; i + ll < len && ishex(buf[i + ll]); ll++)\n\t\t\t; /* do nothing */\n\n> +\t\tif (ll >= 8 && ll <= 40 &&\n\nAFAICT our abbreviation allows for 4 characters and up. Default \nabbreviation is 7, IIRC.\n\n> +\t\t !get_short_sha1(buf, ll, sha1, 1) &&\n> +\t\t !get_rewritten_sha1(sha1))\n> +\t\t\tmemcpy(dest, sha1_to_hex(sha1), ll);\n> +\t\telse\n> +\t\t\tmemcpy(dest, buf, ll);\n\nWhat do you do if the rewritten sha1, truncated to ll characters, is \nambiguous? Wouldn't you need more than\n\n> +static int is_ref_to_be_rewritten(const char *ref)\n> +{\n> +\tunsigned char sha1[20];\n> +\tint flag;\n> +\n> +\tif (prefixcmp(ref, \"refs/\"))\n> +\t\treturn 0;\n> +\tif (!prefixcmp(ref, \"refs/remotes/\"))\n> +\t\treturn 0;\n> +\tif (!prefixcmp(ref+5, original_prefix))\n\nShould original_prefix not be \"refs/original\", then?\n\n> +static int is_pruned(struct commit *commit, int path_pruning)\n> +{\n> +\tif (commit->object.flags & PRUNED)\n> +\t\treturn 1;\n\nHmm. I thought about changing get_revision_1() to mark that commit as \nuninteresting, since the parents were already added. But I guess this has \ntoo much side effect potential.\n\nIn any case, I think that \"NO_MATCH\" would be a more descriptive name.\n\n> +\tif (path_pruning &&\n> +\t !(commit->object.flags & (TREECHANGE | UNINTERESTING)))\n> +\t\treturn 1;\n\nWhy only with \"path_pruning\"? Ah yes. Because otherwise, you would \nassume \"A\" in \"A..B\" to be pruned. But what do you do if someone says\n\n\tgit rewrite-commits A.. x/y\n\nbecause she wants _only_ commits later than A rewritten, but everything \nfrom A backwards kept as-is?\n\nIf I understand your code correctly, the given command would cut off the \nhistory at A.\n\nI wonder why you bother at all: my impression was that the revisions you \nwant to filter out here were already filtered out by \nrevision.c:rewrite_parents()...\n\n> +static void rewrite_sha1(struct object *obj, unsigned char *new_sha1)\n> +{\n> +\tif (!hashcmp(obj->sha1, new_sha1))\n> +\t\treturn;\n> +\n> +\tadd_rewrite_decoration(obj, new_sha1);\n\nThis is not so much \"rewrite_sha1\", as \"append_rewritten_sha1\", right?\n\n> +/*\n> + * Replace any parent that has been removed by its parents\n> + * and return the number of new parents.\n> + * We directly modify the parent list, so any libification\n> + * should probably adapt this function.\n\nI do not think that this code will be libified any time soon.\n\n> +static int rewrite_parents(struct commit *commit, int path_pruning)\n\nOf course, you could always pass a \"struct commit_list \n**rewritten_parents_p\" to this function.\n\n> +\t\tget_rewritten_sha1(sha1);\n> +\t\tif (!is_null_sha1(sha1)) {\n> +\t\t\thashclr(sha1);\n> +\t\t\trewrite_sha1(&list->item->object, sha1);\n\nI guess you'll admit that it is unintuitive to read \n\"get_rewritten_sha1(sha1); rewrite_sha1(o, sha1);\". I thought: \"What? \nAgain?\" IMHO that is a strong hint that \"rewrite_sha1\" is not an apt name \nfor that function.\n\n> +static int rewrite_ref(const char *refname, const unsigned char *sha1,\n> +\t\t\tint flags, void *cb_data)\n> +{\n> +\tint prefix_len;\n> +\tint len;\n> +\tchar buffer[256], *p;\n> +\tstruct object *obj = parse_object(sha1);\n> +\tunsigned char new_sha1[20];\n> +\tstruct commit *commit;\n> +\tint pruned;\n> +\n> +\tif (!obj)\n> +\t\treturn 0;\n> +\tif (obj->type == OBJ_TAG)\n\nSpeaking of tags... We will have to add (in a separate patch, to keep \nthings reviewable) a method to rewrite them, too.\n\n> +\tif (!is_ref_to_be_rewritten(refname))\n> +\t\treturn 0;\n\nHmm. When looking at that code, I wonder if\n\n\tgit rewrite-commits A..B\n\nwill rewrite C, too, if it happens to lie in A..B. That would be not \nbrilliant.\n\nI guess you will need to copy the positive refs in revs->pending, if there \nare any, and later _not_ call for_each_ref if that list was empty.\n\n> +\tcommit = lookup_commit_reference(sha1);\n> +\tpruned = is_pruned(commit, !!cb_data);\n> +\n> +\thashcpy(new_sha1, sha1);\n> +\tif (!pruned && get_rewritten_sha1(new_sha1))\n> +\t\treturn 0;\n\nSo if pruned == 1, you _do_ rewrite it? I'm not quite sure. Care to \nexplain?\n\n> +static void filter_and_write_commit(char *commit_body, size_t len,\n> +\t\t\t\t struct commit *commit, unsigned char *sha1)\n> +{\n> +\tchar commit_path[PATH_MAX];\n> +\tstruct child_process cmd;\n> +\tint argc;\n> +\tconst char *argv[10];\n> +\tint fd;\n> +\tconst char *env[] = { commit_env, NULL };\n> +\tchar hex[41];\n> +\tstruct commit_list *list = NULL, **end = &list;\n> +\n> +\tmemcpy(commit_env+sizeof(commit_env)-41,\n> +\t\tsha1_to_hex(commit->object.sha1), 40);\n> +\n> +\tfd = git_mkstemp(commit_path, sizeof(commit_path), \".commit_XXXXXX\");;\n> +\twrite_or_die(fd, commit_body, len);\n> +\n> +\targc = 0;\n> +\targv[argc++] = \"sh\";\n> +\targv[argc++] = \"-c\";\n> +\targv[argc++] = commit_filter;\n> +\targv[argc] = NULL;\n> +\tmemset(&cmd, 0, sizeof(cmd));\n> +\tcmd.in = open(commit_path, O_RDONLY);\n> +\tif (cmd.in < 0)\n> +\t\tdie(\"Unable to read commit from file '%s'\", commit_path);\n> +\tunlink(commit_path);\n\nThis will fail on Windows. You do not catch that error, so it is almost \nfine: just put the file into rewrite_dir, so the leftovers will be removed \nlater anyway.\n\n> +\t\thashclr(sha1);\n> +\t\tcommit->object.flags |= PRUNED;\n> +\t\t/*\n> +\t\t * If the filter returns two or more commits,\n> +\t\t * we consider the original commit to have been\n> +\t\t * removed and put the list in the old commit's\n> +\t\t * parent list so that all the old commit's children\n> +\t\t * will copy them.\n> +\t\t */\n> +\t\tif (list) {\n> +\t\t\tfree_commit_list(commit->parents);\n> +\t\t\tcommit->parents = list;\n> +\t\t} else\n> +\t\t add_sha1_map(commit->object.sha1, NULL);\n\nThat is almost certainly wrong. If you step away from the notion that \nget_rewritten_sha1() returns _one_ SHA-1, all will become clearer. And \nthe filters should know about them SHA-1s, too.\n\n> +\t/* Make enough remove for n (possibly extra) parents */\n> +\tp = buf = xmalloc(orig_len + n*48);\n\nAs stated above, I'd prefer ALLOC_GROW().\n\n> +\tp = rewrite_header(p, &orig_len, &orig_buf, commit);\n> +\tp = rewrite_body(p, orig_len, orig_buf);\n> +\tif (!commit_filter) {\n> +\t\tif (write_sha1_file(buf, p-buf, commit_type, sha1))\n> +\t\t\tdie(\"Unable to write new commit\");\n> +\t\tadd_sha1_map(commit->object.sha1, sha1);\n> +\t} else\n> +\t\tfilter_and_write_commit(buf, p-buf, commit, sha1);\n> +\tfree(buf);\n\nYou can really discard the commit->buffer, too, no?\n\n> +\n> +\trewrite_sha1(&commit->object, sha1);\n> +}\n\n> +static char aux_functions[] =\n> +\"export GIT_REWRITE_DIR=`pwd`\\n\"\n\nBetter put the correct absolute path there.\n\n> +\"commit()\\n\"\n> +\"{\\n\"\n> +\"\tgit-hash-object -w -t commit --stdin\\n\"\n> +\"}\\n\"\n> +\"map()\\n\"\n> +\"{\\n\"\n> +\"\t# if it was not rewritten, take the original\\n\"\n> +\"\tif test -r \\\"$GIT_REWRITE_DIR/map/$1\\\"\\n\"\n> +\"\tthen\\n\"\n> +\"\t\tcat \\\"$GIT_REWRITE_DIR/map/$1\\\"\\n\"\n> +\"\telse\\n\"\n> +\"\t\techo \\\"$1\\\"\\n\"\n> +\"\tfi\\n\"\n> +\"}\\n\"\n> +\"cd t\\n\";\n\nThen you do not need this extra fork() and cd all the time the filters \nsource the helper file.\n\nYou do not even need the one static global buffer, if you put that script \ninto the function writing it, inside an fprintf() call.\n\n> +static char filter_prefix[] = \". aux;\";\n\nAnd here, I'd put the absolute path, too.\n\n> +static char *create_filter(const char *command)\n> +{\n> +\tint prefix_len = sizeof(filter_prefix)-1;\n> +\tint command_len = strlen(command);\n> +\tchar *filter = xmalloc(prefix_len + command_len + 1);\n> +\n> +\tmemcpy(filter, filter_prefix, prefix_len);\n> +\tmemcpy(filter+prefix_len, command, command_len+1);\n> +\treturn filter;\n> +}\n\nI really like that approach. This makes it so much more convenient for \nusers.\n\n> +static void setup_temp_dir()\n> +{\n> +\tint aux;\n> +\n> +\tabsolute_git_dir = create_absolute_path(get_git_dir());\n> +\tsetenv(GIT_DIR_ENVIRONMENT, absolute_git_dir, 1);\n> +\n> +\tif (!rewrite_dir) {\n> +\t\trewrite_dir = xstrdup(git_path(\"rewrite\"));\n\nI might read the code wrong, but this does not guarantee that rewrite_dir \nis an absolute path, right?\n\n> +\tif (mkdir(mkpath(\"%s/map\", rewrite_dir), 0777))\n> +\t\tdie(\"unable to create map directory '%s/map'\", rewrite_dir);\n\nMaybe make this dependent on --write-sha1-mappings, and have a check in \n\"map ()\"?\n\n> +\tif (index_filter || commit_filter)\n> +\t\tsetup_temp_dir();\n> +\telse\n> +\t\t/* They'll never know. BWUHAHA */\n> +\t\twrite_sha1_mapping = 0;\n\nI like the comment ;-)\n\n> +\tprepare_revision_walk(&rev);\n> +\twhile ((commit = get_revision(&rev)) != NULL) {\n> +\t\trewrite_commit(commit, !!rev.prune_fn);\n> +\t}\n\nPlease lose the curly brackets for single lines; otherwise it would look \ntoo perl like.\n\n> +\trm_rf(git_path(\"refs/%s\", original_prefix));\n\nWould it not be better to move refs/original to refs/original/original?\n\nPooh. A lot of comments. Please take that as a sign that I am interested \nin rewrite-commits. BTW did I mention already that I like the name \n\"rewrite-commits\"?\n\nCiao,\nDscho\n"},{"id":"47371","messageId":"7v7ip2hjna.fsf@assigned-by-dhcp.cox.net","threadId":"9008","inReplyTo":"Pine.LNX.4.64.0707141140510.14781@racer.site","subject":"Re: [PATCH 6/6] Add git-rewrite-commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-14T19:26:01Z","receivedAt":"2007-07-14T19:26:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n>> +Examples\n>> +--------\n>> +\n>> +Suppose you want to remove a file (containing confidential information\n>> +or copyright violation) from all commits:\n>> +\n>> +----------------------------------------------------------------------------\n>> +git rewrite-commits --index-filter 'git update-index --remove filename || :'\n>\n> We seem to prefer \"$ git\" instead of just \"git\" in the other man pages' \n> examples.\n\n\"git update-index --remove Foo\" does not remove the index entry\nFoo if the file Foo still exists in the working tree (use \"git\nupdate-index --force-remove\" for that).\n\nBut this leads to more fundamental issues. It is not obvious\nfrom the description what environment rewrite-commits runs in.\nDoes it run at the toplevel of the current working tree, or is\nit run in a separate temporary directory like filter-branch\ndoes? What \"index\" and \"HEAD\" do operations done by filters\naffect (I think it is safe to assume that readers familiar\nenough with other parts of git would be able to guess that\nfilters should operate on the \"HEAD\" and index given by\nrewrite-commits to its execution environment without mucking\nwith GIT_DIR nor GIT_INDEX_FILE)? Are filters allowed to modify\nfiles in the working tree, and if so what is the consequence of\ndoing so?\n\n>> +----------------------------------------------------------------------------\n>> +\n>> +Now, you will get the rewritten history saved in your current branch\n>> +(the old branch is saved in refs/original).\n>\n> \t\t\t\t\t\tThe \"|| :\" construct \n> + prevents the filter to fail when the given file was not present in the \n> + index.\n\nprevents the filter from failing? But is that really what we\nwant? Why are we ignoring the error, and if there is a valid\nreason to ignore shouldn't we explain why?\n\n>> +To move the whole tree into a subdirectory, or remove it from there:\n>> +\n>> +---------------------------------------------------------------\n>> +git rewrite-commits --index-filter \\\n>> +\t'git ls-files -s | sed \"s-\\t-&newsubdir/-\" |\n>> +\t\tGIT_INDEX_FILE=$GIT_INDEX_FILE.new \\\n>> +\t\t\tgit update-index --index-info &&\n>> +\t mv $GIT_INDEX_FILE.new $GIT_INDEX_FILE'\n>> +---------------------------------------------------------------\n\nI see only one operation in the example, and \"or remove it from\nthere\" confuses the reader.\n\nI'll refrain from comments on the code right now, until I read\nthe series over.\n"},{"id":"47375","messageId":"20070714201519.GA999MdfPADPa@greensroom.kotnet.org","threadId":"9008","inReplyTo":"Pine.LNX.4.64.0707141140510.14781@racer.site","subject":"Re: [PATCH 6/6] Add git-rewrite-commits","fromName":"Sven Verdoolaege","fromEmail":"skimo@kotnet.org","sentAt":"2007-07-14T20:15:20Z","receivedAt":"2007-07-14T20:15:20Z","isPatch":true,"sender":{"key":"skimo@kotnet.org","avatar":null},"body":"On Sat, Jul 14, 2007 at 01:49:59PM +0100, Johannes Schindelin wrote:\n> Pooh. A lot of comments. Please take that as a sign that I am interested \n\nThanks. I'll look through them tomorrow.\n\n> in rewrite-commits. BTW did I mention already that I like the name \n> \"rewrite-commits\"?\n\nYou should like it. You suggested it.\n(http://article.gmane.org/gmane.comp.version-control.git/45637)\nNot that I had read that when I named it. I'm about 2.5 months\nbehind on my git mailing list reading.\n\nskimo\n"},{"id":"47429","messageId":"20070715140755.GG999MdfPADPa@greensroom.kotnet.org","threadId":"9008","inReplyTo":"7v7ip2hjna.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 6/6] Add git-rewrite-commits","fromName":"Sven Verdoolaege","fromEmail":"skimo@kotnet.org","sentAt":"2007-07-15T14:07:55Z","receivedAt":"2007-07-15T14:07:55Z","isPatch":true,"sender":{"key":"skimo@kotnet.org","avatar":null},"body":"On Sat, Jul 14, 2007 at 12:26:01PM -0700, Junio C Hamano wrote:\n> But this leads to more fundamental issues. It is not obvious\n> from the description what environment rewrite-commits runs in.\n\nI'll try to make that more clear in the next round.\n\n> >> +To move the whole tree into a subdirectory, or remove it from there:\n> >> +\n> >> +---------------------------------------------------------------\n> >> +git rewrite-commits --index-filter \\\n> >> +\t'git ls-files -s | sed \"s-\\t-&newsubdir/-\" |\n> >> +\t\tGIT_INDEX_FILE=$GIT_INDEX_FILE.new \\\n> >> +\t\t\tgit update-index --index-info &&\n> >> +\t mv $GIT_INDEX_FILE.new $GIT_INDEX_FILE'\n> >> +---------------------------------------------------------------\n> \n> I see only one operation in the example, and \"or remove it from\n> there\" confuses the reader.\n\nI found it confusing too (I copied it from the filter-branch manual).\nI'll remove it.\n\nskimo\n"},{"id":"47432","messageId":"20070715144435.GH999MdfPADPa@greensroom.kotnet.org","threadId":"9008","inReplyTo":"Pine.LNX.4.64.0707141140510.14781@racer.site","subject":"Re: [PATCH 6/6] Add git-rewrite-commits","fromName":"Sven Verdoolaege","fromEmail":"skimo@kotnet.org","sentAt":"2007-07-15T14:44:35Z","receivedAt":"2007-07-15T14:44:35Z","isPatch":true,"sender":{"key":"skimo@kotnet.org","avatar":null},"body":"On Sat, Jul 14, 2007 at 01:49:59PM +0100, Johannes Schindelin wrote:\n> On Thu, 12 Jul 2007, skimo@liacs.nl wrote:\n> > +#include \"grep.h\"\n> \n> I did not compile test, but do you really need that?\n\nNot anymore.\n\n> > +struct decoration rewrite_decoration = { \"rewritten as\" };\n> > [...]\n> > +\n> > +struct rewrite_decoration {\n> > +\tstruct rewrite_decoration *next;\n> > +\tunsigned char sha1[20];\n> > +};\n> \n> I wonder why you give the instance of a \"struct decoration\" the same name \n> as that of a _different_ struct. IOW it is confusing that there is a \n> \"struct rewrite_decoration\", but the variable \"rewrite_decoration\" is no \n> instance of that struct.\n\nI was just following Linus's example.\n\n> > +static char *add_parents(char *dest, struct commit_list *parents)\n> \n> Since one commit can be rewritten to multiple commits now, you do not know \n> how much space you need to add the parents.\n\nI count them beforehand.\n\n> > +\t\t !get_short_sha1(buf, ll, sha1, 1) &&\n> > +\t\t !get_rewritten_sha1(sha1))\n> > +\t\t\tmemcpy(dest, sha1_to_hex(sha1), ll);\n> > +\t\telse\n> > +\t\t\tmemcpy(dest, buf, ll);\n> \n> What do you do if the rewritten sha1, truncated to ll characters, is \n> ambiguous? Wouldn't you need more than\n\nMore than what?\nAre you saying I should add some more of it then?\n(As you can see, I simply don't consider that case here, so right now\nI just leave it possibly ambiguous.)\n\n> > +\tif (!prefixcmp(ref+5, original_prefix))\n> \n> Should original_prefix not be \"refs/original\", then?\n\nThe idea was that we could add a command line option later\nto override it.\n\n> > +static int is_pruned(struct commit *commit, int path_pruning)\n> > +{\n> > +\tif (commit->object.flags & PRUNED)\n> > +\t\treturn 1;\n> \n> Hmm. I thought about changing get_revision_1() to mark that commit as \n> uninteresting, since the parents were already added. But I guess this has \n> too much side effect potential.\n> \n> In any case, I think that \"NO_MATCH\" would be a more descriptive name.\n\nRight now, it's only set for matching stuff, but it could be set\nfor other kinds of pruning too.\n\n> > +\tif (path_pruning &&\n> > +\t !(commit->object.flags & (TREECHANGE | UNINTERESTING)))\n> > +\t\treturn 1;\n> \n> Why only with \"path_pruning\"? Ah yes. Because otherwise, you would \n> assume \"A\" in \"A..B\" to be pruned.\n\nTREECHANGE is only set when path pruning is in effect.\nIf I didn't check for path_pruning, then all commits would be\nconsidered to have been pruned. (Or am I missing something?\nHonestyl, I found all that TREECHANGE stuff difficult to follow.)\n\nrevision.c itself is also riddled with \"prune_fn && \".\nWouldn't it make sense to invert the meaning of this bit and call\nit, say, PRUNED, so that the default is off and you would only\nhave to check if the bit was set ?\n\n> I wonder why you bother at all: my impression was that the revisions you \n> want to filter out here were already filtered out by \n> revision.c:rewrite_parents()...\n\nI also want to make reference that pointed to something that has\nbeen pruned (in either way) to now point to something in the new\nhistory.\n\n> > +static void rewrite_sha1(struct object *obj, unsigned char *new_sha1)\n> > +{\n> > +\tif (!hashcmp(obj->sha1, new_sha1))\n> > +\t\treturn;\n> > +\n> > +\tadd_rewrite_decoration(obj, new_sha1);\n> \n> This is not so much \"rewrite_sha1\", as \"append_rewritten_sha1\", right?\n\nWell, it's only called once on each commit.\n\n> > +\t\tget_rewritten_sha1(sha1);\n> > +\t\tif (!is_null_sha1(sha1)) {\n> > +\t\t\thashclr(sha1);\n> > +\t\t\trewrite_sha1(&list->item->object, sha1);\n> \n> I guess you'll admit that it is unintuitive to read \n> \"get_rewritten_sha1(sha1); rewrite_sha1(o, sha1);\". I thought: \"What? \n> Again?\" IMHO that is a strong hint that \"rewrite_sha1\" is not an apt name \n> for that function.\n\nI guess our brains work in a slightly different way.\n\n> Hmm. When looking at that code, I wonder if\n> \n> \tgit rewrite-commits A..B\n> \n> will rewrite C, too, if it happens to lie in A..B. That would be not \n> brilliant.\n\nThat was the idea. Why is it not brilliant?\nWhat would you like this to do instead?\n\nAssume that in your new history all the commit that used to be pointed\nto have been removed. If these pointers are left to point to the\nold history, then how are you going to get at the new history?\n\n> > +\tcommit = lookup_commit_reference(sha1);\n> > +\tpruned = is_pruned(commit, !!cb_data);\n> > +\n> > +\thashcpy(new_sha1, sha1);\n> > +\tif (!pruned && get_rewritten_sha1(new_sha1))\n> > +\t\treturn 0;\n> \n> So if pruned == 1, you _do_ rewrite it? I'm not quite sure. Care to \n> explain?\n\nYes. See above.\n\n> > +\tcmd.in = open(commit_path, O_RDONLY);\n> > +\tif (cmd.in < 0)\n> > +\t\tdie(\"Unable to read commit from file '%s'\", commit_path);\n> > +\tunlink(commit_path);\n> \n> This will fail on Windows. You do not catch that error, so it is almost \n> fine: just put the file into rewrite_dir, so the leftovers will be removed \n> later anyway.\n\nYou mean unlinking an opened file?\n\n> \n> > +\t\thashclr(sha1);\n> > +\t\tcommit->object.flags |= PRUNED;\n> > +\t\t/*\n> > +\t\t * If the filter returns two or more commits,\n> > +\t\t * we consider the original commit to have been\n> > +\t\t * removed and put the list in the old commit's\n> > +\t\t * parent list so that all the old commit's children\n> > +\t\t * will copy them.\n> > +\t\t */\n> > +\t\tif (list) {\n> > +\t\t\tfree_commit_list(commit->parents);\n> > +\t\t\tcommit->parents = list;\n> > +\t\t} else\n> > +\t\t add_sha1_map(commit->object.sha1, NULL);\n> \n> That is almost certainly wrong.\n\nWhat specifically ?\n\n> If you step away from the notion that \n> get_rewritten_sha1() returns _one_ SHA-1, all will become clearer. And \n> the filters should know about them SHA-1s, too.\n\nThe filters do know about them. The corresponding map file contains\nall the new SHA1s.\n\n> > +\t\tadd_sha1_map(commit->object.sha1, sha1);\n> > +\t} else\n> > +\t\tfilter_and_write_commit(buf, p-buf, commit, sha1);\n> > +\tfree(buf);\n> \n> You can really discard the commit->buffer, too, no?\n\nI'm not familiar enough with the internals to say for sure.\n\n> > +static void setup_temp_dir()\n> > +{\n> > +\tint aux;\n> > +\n> > +\tabsolute_git_dir = create_absolute_path(get_git_dir());\n> > +\tsetenv(GIT_DIR_ENVIRONMENT, absolute_git_dir, 1);\n> > +\n> > +\tif (!rewrite_dir) {\n> > +\t\trewrite_dir = xstrdup(git_path(\"rewrite\"));\n> \n> I might read the code wrong, but this does not guarantee that rewrite_dir \n> is an absolute path, right?\n\nIt doesn't right now.\n\n> > +\tif (mkdir(mkpath(\"%s/map\", rewrite_dir), 0777))\n> > +\t\tdie(\"unable to create map directory '%s/map'\", rewrite_dir);\n> \n> Maybe make this dependent on --write-sha1-mappings, and have a check in \n> \"map ()\"?\n\nI'll leave that to you :-)\n\n> > +\twhile ((commit = get_revision(&rev)) != NULL) {\n> > +\t\trewrite_commit(commit, !!rev.prune_fn);\n> > +\t}\n> \n> Please lose the curly brackets for single lines; otherwise it would look \n> too perl like.\n\nThat would be\n\t\n\trewrite_commit($commit) while $commit = get_revision;\n\n> > +\trm_rf(git_path(\"refs/%s\", original_prefix));\n> \n> Would it not be better to move refs/original to refs/original/original?\n\nIs that what you want?\nYou'd end up with refs/original/original/original/original/original/original/original/\npretty quickly.\n\nskimo\n"},{"id":"47486","messageId":"Pine.LNX.4.64.0707160054340.14781@racer.site","threadId":"9008","inReplyTo":"20070715144435.GH999MdfPADPa@greensroom.kotnet.org","subject":"Re: [PATCH 6/6] Add git-rewrite-commits","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-16T00:38:11Z","receivedAt":"2007-07-16T00:38:11Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 15 Jul 2007, Sven Verdoolaege wrote:\n\n> On Sat, Jul 14, 2007 at 01:49:59PM +0100, Johannes Schindelin wrote:\n> > On Thu, 12 Jul 2007, skimo@liacs.nl wrote:\n> > > +struct decoration rewrite_decoration = { \"rewritten as\" };\n> > > [...]\n> > > +\n> > > +struct rewrite_decoration {\n> > > +\tstruct rewrite_decoration *next;\n> > > +\tunsigned char sha1[20];\n> > > +};\n> > \n> > I wonder why you give the instance of a \"struct decoration\" the same name \n> > as that of a _different_ struct. IOW it is confusing that there is a \n> > \"struct rewrite_decoration\", but the variable \"rewrite_decoration\" is no \n> > instance of that struct.\n> \n> I was just following Linus's example.\n\n*Sigh* I hate to repeat arguments.\n\n> > > +static char *add_parents(char *dest, struct commit_list *parents)\n> > \n> > Since one commit can be rewritten to multiple commits now, you do not \n> > know how much space you need to add the parents.\n> \n> I count them beforehand.\n\nYes. And it is extremely hard to follow through your code, just to verify \nthat it really works. Using ALLOC_GROW instead of trying to play cute \ngames would spare these tours, and I am quite convinced that it makes for \nless bugs. If not now, then when somebody _else_ than you touches the \ncode.\n\n> > > +\t\t !get_short_sha1(buf, ll, sha1, 1) &&\n> > > +\t\t !get_rewritten_sha1(sha1))\n> > > +\t\t\tmemcpy(dest, sha1_to_hex(sha1), ll);\n> > > +\t\telse\n> > > +\t\t\tmemcpy(dest, buf, ll);\n> > \n> > What do you do if the rewritten sha1, truncated to ll characters, is \n> > ambiguous? Wouldn't you need more than\n> \n> More than what?\n\nRight. I did not complete that sentence. But isn't it obvious? Usually, \nyou put in short, but (at least for some time) _unambiguous_ short names. \nIMHO people would expect the same to be true of rewritten commit messages.\n\n> Are you saying I should add some more of it then? (As you can see, I \n> simply don't consider that case here, so right now I just leave it \n> possibly ambiguous.)\n\nYes, I saw that. Just wanted to remind you that some people might \nconsider this a bug.\n\n> > > +\tif (!prefixcmp(ref+5, original_prefix))\n> > \n> > Should original_prefix not be \"refs/original\", then?\n> \n> The idea was that we could add a command line option later to override \n> it.\n\nOkay, I'll try again. You do not use the canonical form. Why not use \nit, if only for consistency? Besides, if you want to be able to override \nit with a command line option, you _will_ have to canonicalise _that_ \noriginal_prefix.\n\n> > > +static int is_pruned(struct commit *commit, int path_pruning)\n> > > +{\n> > > +\tif (commit->object.flags & PRUNED)\n> > > +\t\treturn 1;\n> > \n> > Hmm. I thought about changing get_revision_1() to mark that commit as \n> > uninteresting, since the parents were already added. But I guess this \n> > has too much side effect potential.\n> > \n> > In any case, I think that \"NO_MATCH\" would be a more descriptive name.\n> \n> Right now, it's only set for matching stuff, but it could be set for \n> other kinds of pruning too.\n\nYes, it could.\n\n> > > +\tif (path_pruning &&\n> > > +\t !(commit->object.flags & (TREECHANGE | UNINTERESTING)))\n> > > +\t\treturn 1;\n> > \n> > Why only with \"path_pruning\"? Ah yes. Because otherwise, you would \n> > assume \"A\" in \"A..B\" to be pruned.\n> \n> TREECHANGE is only set when path pruning is in effect.\n> If I didn't check for path_pruning, then all commits would be\n> considered to have been pruned. (Or am I missing something?\n> Honestyl, I found all that TREECHANGE stuff difficult to follow.)\n\nAFAICT TREECHANGE means that parents were rewritten.\n\n> revision.c itself is also riddled with \"prune_fn && \".\n> Wouldn't it make sense to invert the meaning of this bit and call\n> it, say, PRUNED, so that the default is off and you would only\n> have to check if the bit was set ?\n\nYou meant the TREECHANGE bit? No.\n\nBTW what do you plan to do about my objection to UNINTERESTING, given the \nexample \"git rewrite-commits A..B x/y\"?\n\n> > I wonder why you bother at all: my impression was that the revisions \n> > you want to filter out here were already filtered out by \n> > revision.c:rewrite_parents()...\n> \n> I also want to make reference that pointed to something that has\n> been pruned (in either way) to now point to something in the new\n> history.\n\nI did not understand that intention. Maybe I am too dumb, that's all.\n\n> > > +static void rewrite_sha1(struct object *obj, unsigned char *new_sha1)\n> > > +{\n> > > +\tif (!hashcmp(obj->sha1, new_sha1))\n> > > +\t\treturn;\n> > > +\n> > > +\tadd_rewrite_decoration(obj, new_sha1);\n> > \n> > This is not so much \"rewrite_sha1\", as \"append_rewritten_sha1\", right?\n> \n> Well, it's only called once on each commit.\n\nDidn't I mention that it was a severe limitation to think of the sha1 \nmapping of a 1-to-1 mapping? Think of it more as a relation.\n\n> > > +\t\tget_rewritten_sha1(sha1);\n> > > +\t\tif (!is_null_sha1(sha1)) {\n> > > +\t\t\thashclr(sha1);\n> > > +\t\t\trewrite_sha1(&list->item->object, sha1);\n> > \n> > I guess you'll admit that it is unintuitive to read \n> > \"get_rewritten_sha1(sha1); rewrite_sha1(o, sha1);\". I thought: \"What? \n> > Again?\" IMHO that is a strong hint that \"rewrite_sha1\" is not an apt \n> > name for that function.\n> \n> I guess our brains work in a slightly different way.\n\nI doubt that. What would you expect a code\n\n\tget_tagged_commit(commit);\n\ttag_commit(tag, commit);\n\nto do, if not tag a tagged commit _again_? So I bet if it wasn't your \ncode to begin with, you would have experienced the same confusion as me.\n\n> > Hmm. When looking at that code, I wonder if\n> > \n> > \tgit rewrite-commits A..B\n> > \n> > will rewrite C, too, if it happens to lie in A..B. That would be not \n> > brilliant.\n> \n> That was the idea. Why is it not brilliant?\n\nBecause it is called rewrite-commits, not \nrewrite-all-refs-that-touch-this-commit-range.\n\n> What would you like this to do instead?\n\nOnly touch the positive refs given in the command line. If you say \n\"--all\", then it's all. If you say \"A..B\", then it's \"B\".\n\n> Assume that in your new history all the commit that used to be pointed \n> to have been removed. If these pointers are left to point to the old \n> history, then how are you going to get at the new history?\n\nI still have the superproject splitting as the main application in mind. \nOnly for big applications like these does the performance improvement of \nrewrite-commits over filter-branch buy us anything.\n\n> > > +\tcommit = lookup_commit_reference(sha1);\n> > > +\tpruned = is_pruned(commit, !!cb_data);\n> > > +\n> > > +\thashcpy(new_sha1, sha1);\n> > > +\tif (!pruned && get_rewritten_sha1(new_sha1))\n> > > +\t\treturn 0;\n> > \n> > So if pruned == 1, you _do_ rewrite it? I'm not quite sure. Care to \n> > explain?\n> \n> Yes. See above.\n\nSorry, this explanation does not help me. My impression was that a pruned \ncommit should be _mapped_ to the rewritten sha1s of the original parents, \nbut not be rewritten.\n\n> > > +\tcmd.in = open(commit_path, O_RDONLY);\n> > > +\tif (cmd.in < 0)\n> > > +\t\tdie(\"Unable to read commit from file '%s'\", commit_path);\n> > > +\tunlink(commit_path);\n> > \n> > This will fail on Windows. You do not catch that error, so it is almost \n> > fine: just put the file into rewrite_dir, so the leftovers will be removed \n> > later anyway.\n> \n> You mean unlinking an opened file?\n\nYes, I mean unlinking an opened file.\n\n> > > +\t\thashclr(sha1);\n> > > +\t\tcommit->object.flags |= PRUNED;\n> > > +\t\t/*\n> > > +\t\t * If the filter returns two or more commits,\n> > > +\t\t * we consider the original commit to have been\n> > > +\t\t * removed and put the list in the old commit's\n> > > +\t\t * parent list so that all the old commit's children\n> > > +\t\t * will copy them.\n> > > +\t\t */\n> > > +\t\tif (list) {\n> > > +\t\t\tfree_commit_list(commit->parents);\n> > > +\t\t\tcommit->parents = list;\n> > > +\t\t} else\n> > > +\t\t add_sha1_map(commit->object.sha1, NULL);\n> > \n> > That is almost certainly wrong.\n> \n> What specifically ?\n\nIt takes a few minutes to verify that the correct calls to add_sha1_map() \nare done in every case.\n\nAlso, why should commit->parents be reset only in the multiple-sha1 case?\n\nBut most seriously, why do you replace the _parents_ of a given commit by \nthe sha1s of the _rewritten_ commits? They are not even the rewritten \n_parents_.\n\n> > If you step away from the notion that \n> > get_rewritten_sha1() returns _one_ SHA-1, all will become clearer. And \n> > the filters should know about them SHA-1s, too.\n> \n> The filters do know about them. The corresponding map file contains\n> all the new SHA1s.\n\nBut the code does not reflect that in some parts. For example when you \ncall \"get_rewritten_sha1(sha1);\" which one is it? And what is it if the \ncommit was rewritten to no commit at all, i.e. the history was cut \nforcefully?\n\n> > > +\t\tadd_sha1_map(commit->object.sha1, sha1);\n> > > +\t} else\n> > > +\t\tfilter_and_write_commit(buf, p-buf, commit, sha1);\n> > > +\tfree(buf);\n> > \n> > You can really discard the commit->buffer, too, no?\n> \n> I'm not familiar enough with the internals to say for sure.\n\nHmm. You play with them pretty heavily, then, for example when you just \nreset commit->parents.\n\n> > > +static void setup_temp_dir()\n> > > +{\n> > > +\tint aux;\n> > > +\n> > > +\tabsolute_git_dir = create_absolute_path(get_git_dir());\n> > > +\tsetenv(GIT_DIR_ENVIRONMENT, absolute_git_dir, 1);\n> > > +\n> > > +\tif (!rewrite_dir) {\n> > > +\t\trewrite_dir = xstrdup(git_path(\"rewrite\"));\n> > \n> > I might read the code wrong, but this does not guarantee that rewrite_dir \n> > is an absolute path, right?\n> \n> It doesn't right now.\n\nUmm. Sorry to ask so bluntly: are you planning to change that?\n\n> > > +\tif (mkdir(mkpath(\"%s/map\", rewrite_dir), 0777))\n> > > +\t\tdie(\"unable to create map directory '%s/map'\", rewrite_dir);\n> > \n> > Maybe make this dependent on --write-sha1-mappings, and have a check in \n> > \"map ()\"?\n> \n> I'll leave that to you :-)\n\nIMHO the first part should be in the initial commit of \nbuiltin-rewrite-commits.c, so I will not do that.\n\n> > > +\twhile ((commit = get_revision(&rev)) != NULL) {\n> > > +\t\trewrite_commit(commit, !!rev.prune_fn);\n> > > +\t}\n> > \n> > Please lose the curly brackets for single lines; otherwise it would look \n> > too perl like.\n> \n> That would be\n> \t\n> \trewrite_commit($commit) while $commit = get_revision;\n\nNo, it would not, because it is not valid C.\n\n> > > +\trm_rf(git_path(\"refs/%s\", original_prefix));\n> > \n> > Would it not be better to move refs/original to \n> > refs/original/original?\n> \n> Is that what you want? You'd end up with \n> refs/original/original/original/original/original/original/original/ \n> pretty quickly.\n\nWe are pretty anal about not losing data too quickly, and possibly \nunwantedly.\n\nIt would be surprising to me if this would be the first program to change \nthat behaviour.\n\nIOW, refs/original/ is not a temporary directory that you easily blow \naway. It is meant for the user to be inspected. And it is the user's \nobligation to say when that happened, and that it should go now.\n\nThinking about it again, I'd even go so far as to disallow operation with \nan existing refs/original/.\n\nCiao,\nDscho\n"},{"id":"47527","messageId":"20070716102407.GL999MdfPADPa@greensroom.kotnet.org","threadId":"9008","inReplyTo":"Pine.LNX.4.64.0707160054340.14781@racer.site","subject":"Re: [PATCH 6/6] Add git-rewrite-commits","fromName":"Sven Verdoolaege","fromEmail":"skimo@kotnet.org","sentAt":"2007-07-16T10:24:07Z","receivedAt":"2007-07-16T10:24:07Z","isPatch":true,"sender":{"key":"skimo@kotnet.org","avatar":null},"body":"On Mon, Jul 16, 2007 at 01:38:11AM +0100, Johannes Schindelin wrote:\n> On Sun, 15 Jul 2007, Sven Verdoolaege wrote:\n> > > > +\tif (path_pruning &&\n> > > > +\t !(commit->object.flags & (TREECHANGE | UNINTERESTING)))\n> > > > +\t\treturn 1;\n> > > \n> > > Why only with \"path_pruning\"? Ah yes. Because otherwise, you would \n> > > assume \"A\" in \"A..B\" to be pruned.\n> > \n> > TREECHANGE is only set when path pruning is in effect.\n> > If I didn't check for path_pruning, then all commits would be\n> > considered to have been pruned. (Or am I missing something?\n> > Honestly, I found all that TREECHANGE stuff difficult to follow.)\n> \n> AFAICT TREECHANGE means that parents were rewritten.\n\nI think you'll find that if all commits touch a path in the\npath specifiers then all commits will have TREECHANGE set and\nso no parents will be rewritten.\n\n> > revision.c itself is also riddled with \"prune_fn && \".\n> > Wouldn't it make sense to invert the meaning of this bit and call\n> > it, say, PRUNED, so that the default is off and you would only\n> > have to check if the bit was set ?\n> \n> You meant the TREECHANGE bit? No.\n\nYes. Why?\n\n> BTW what do you plan to do about my objection to UNINTERESTING, given the \n> example \"git rewrite-commits A..B x/y\"?\n\nThat was based on an apparent misunderstanding of my code\nthat I tried to address above. I did not intend to do what\nyou claim I do and a quick test confirms that my code does\nindeed not to what you claim it does.\n\nMore specifically, the history will not be cut off at A\nbecause A is marked UNINTERESTING and is therefore not considered\nto have been pruned.\nA commit is considered pruned if it was either explicitly marked\nas such or if TREECHANGE is not set, but the later check (in is_pruned)\nis only done on commits that were checked for tree changes.\n\nskimo\n"},{"id":"47585","messageId":"20070716200404.GT999MdfPADPa@greensroom.kotnet.org","threadId":"9008","inReplyTo":"Pine.LNX.4.64.0707160054340.14781@racer.site","subject":"Re: [PATCH 6/6] Add git-rewrite-commits","fromName":"Sven Verdoolaege","fromEmail":"skimo@kotnet.org","sentAt":"2007-07-16T20:04:04Z","receivedAt":"2007-07-16T20:04:04Z","isPatch":true,"sender":{"key":"skimo@kotnet.org","avatar":null},"body":"On Mon, Jul 16, 2007 at 01:38:11AM +0100, Johannes Schindelin wrote:\n> Didn't I mention that it was a severe limitation to think of the sha1 \n> mapping of a 1-to-1 mapping? Think of it more as a relation.\n\nThe mapping is used in several operations.\nFirst, there are several things that can happen to a commit\n\n- it's pruned. This includes, for me, path pruning, matching\n and a commit filter returning no SHA1s.\n- it's rewritten to another commit that can be considered the\n \"moral equivalent\" of that commit. This occurs when a commit\n is not pruned, but something else happened to the commit itself or\n one of its ancestors. This excludes, for me, the case\n where a commit filter returns more than one SHA1.\n- it's replaced by more than one SHA1. This can only happen\n in a commit filter.\n\nThere are at least four operations in which this mapping is used:\n\n- if the parents of a commit have been rewritten to one or more\n commits, then they are replaced by the new commits.\n If any parent has been pruned, then it is replaced by\n the result of applying this operation on _its_ parents.\n- any reference (in refs/) that points to a rewritten or pruned\n commit is removed and\n * if the commit was rewritten to a single commit, then it is\n replaced by this commit\n * otherwise, there is no moral equivalent single commit, but\n we want to ensure we can still access the new commits, so\n I create several references, either to each of the many\n commits the old commit was rewritten to, or to each of\n its nearest unpruned ancestors (i.e., the same set as\n described in the previous operation).\n- a SHA1 of a commit that appears in a commit message is replaced\n by the rewritten commit iff it was rewritten to a single commit.\n That is, if the commit was pruned or rewritten (through a commit\n filter to more than one commit), then the SHA1 is left alone.\n- the mapping available to filters\n * if the commit was pruned, an empty file is created\n * otherwise a file is created containing all rewritten SHA1s\n\nI understand you want the second operation to only apply\nto refs explicitly mentioned on the command line.\nWhat else would you like to change?\n\nskimo\n"},{"id":"47599","messageId":"20070716214756.GA15007MdfPADPa@greensroom.kotnet.org","threadId":"9008","inReplyTo":"20070716200404.GT999MdfPADPa@greensroom.kotnet.org","subject":"Re: [PATCH 6/6] Add git-rewrite-commits","fromName":"Sven Verdoolaege","fromEmail":"skimo@kotnet.org","sentAt":"2007-07-16T21:47:56Z","receivedAt":"2007-07-16T21:47:56Z","isPatch":true,"sender":{"key":"skimo@kotnet.org","avatar":null},"body":"On Mon, Jul 16, 2007 at 10:04:04PM +0200, Sven Verdoolaege wrote:\n> - a SHA1 of a commit that appears in a commit message is replaced\n> by the rewritten commit iff it was rewritten to a single commit.\n> That is, if the commit was pruned or rewritten (through a commit\n> filter) to more than one commit, then the SHA1 is left alone.\n\nSorry. I misremembered. I considered doing it this way, but\nthen thought it was better to replace the SHA1 with a(n abbreviated)\nnull SHA1 to signify that the commit had gone.\n\nskimo\n"},{"id":"47721","messageId":"Pine.LNX.4.64.0707181153200.14781@racer.site","threadId":"9008","inReplyTo":"20070716102407.GL999MdfPADPa@greensroom.kotnet.org","subject":"Re: [PATCH 6/6] Add git-rewrite-commits","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-18T11:02:50Z","receivedAt":"2007-07-18T11:02:50Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 16 Jul 2007, Sven Verdoolaege wrote:\n\n> On Mon, Jul 16, 2007 at 01:38:11AM +0100, Johannes Schindelin wrote:\n> > On Sun, 15 Jul 2007, Sven Verdoolaege wrote:\n> > > > > +\tif (path_pruning &&\n> > > > > +\t !(commit->object.flags & (TREECHANGE | UNINTERESTING)))\n> > > > > +\t\treturn 1;\n> > > > \n> > > > Why only with \"path_pruning\"? Ah yes. Because otherwise, you would \n> > > > assume \"A\" in \"A..B\" to be pruned.\n> > > \n> > > TREECHANGE is only set when path pruning is in effect.\n> > > If I didn't check for path_pruning, then all commits would be\n> > > considered to have been pruned. (Or am I missing something?\n> > > Honestly, I found all that TREECHANGE stuff difficult to follow.)\n> > \n> > AFAICT TREECHANGE means that parents were rewritten.\n> \n> I think you'll find that if all commits touch a path in the\n> path specifiers then all commits will have TREECHANGE set and\n> so no parents will be rewritten.\n\nThe code suggests otherwise.\n\nBut I really have to wonder: why do you play games with TREECHANGE? I had \nthe impression that commit->parents is set appropriately by the revision \nwalker, and that you do not have to do _anything_ for that to work.\n\nMaybe the \"--grep\" thing does not yet. But then you should fix it in \nrevision.c. Not in builtin-rewrite-commits.c\n\n> > > revision.c itself is also riddled with \"prune_fn && \".\n> > > Wouldn't it make sense to invert the meaning of this bit and call\n> > > it, say, PRUNED, so that the default is off and you would only\n> > > have to check if the bit was set ?\n> > \n> > You meant the TREECHANGE bit? No.\n> \n> Yes. Why?\n\nWhy invert the meaning of a perfectly fine bit? Because you can? It is \nworking right now, and it is not even a buglet, so what is there to fix?\n\n> > BTW what do you plan to do about my objection to UNINTERESTING, given \n> > the example \"git rewrite-commits A..B x/y\"?\n> \n> That was based on an apparent misunderstanding of my code\n> that I tried to address above. I did not intend to do what\n> you claim I do and a quick test confirms that my code does\n> indeed not to what you claim it does.\n> \n> More specifically, the history will not be cut off at A\n> because A is marked UNINTERESTING and is therefore not considered\n> to have been pruned.\n\nWhy do you test for TREECHANGE | UNINTERESTING then?\n\n> A commit is considered pruned if it was either explicitly marked\n> as such or if TREECHANGE is not set, but the later check (in is_pruned)\n> is only done on commits that were checked for tree changes.\n\nI don't understand. What do you mean by \"a commit is pruned\"? Does it \nmean that this commit was left out from the revision walk? What does that \nhave to do with TREECHANGE, which means that the parents set was modified?\n\nCiao,\nDscho\n"},{"id":"47723","messageId":"Pine.LNX.4.64.0707181204300.14781@racer.site","threadId":"9008","inReplyTo":"20070716214756.GA15007MdfPADPa@greensroom.kotnet.org","subject":"Re: [PATCH 6/6] Add git-rewrite-commits","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-18T11:05:17Z","receivedAt":"2007-07-18T11:05:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 16 Jul 2007, Sven Verdoolaege wrote:\n\n> On Mon, Jul 16, 2007 at 10:04:04PM +0200, Sven Verdoolaege wrote:\n> > - a SHA1 of a commit that appears in a commit message is replaced\n> > by the rewritten commit iff it was rewritten to a single commit.\n> > That is, if the commit was pruned or rewritten (through a commit\n> > filter) to more than one commit, then the SHA1 is left alone.\n> \n> Sorry. I misremembered. I considered doing it this way, but then \n> thought it was better to replace the SHA1 with a(n abbreviated) null \n> SHA1 to signify that the commit had gone.\n\nYes. This shows that you did not get my objection. The commit is not \nreplaced with _no_ commits, but with _multiple_ commits.\n\nCiao,\nDscho\n"},{"id":"47724","messageId":"Pine.LNX.4.64.0707181205260.14781@racer.site","threadId":"9008","inReplyTo":"20070716200404.GT999MdfPADPa@greensroom.kotnet.org","subject":"Re: [PATCH 6/6] Add git-rewrite-commits","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-18T11:17:03Z","receivedAt":"2007-07-18T11:17:03Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 16 Jul 2007, Sven Verdoolaege wrote:\n\n> On Mon, Jul 16, 2007 at 01:38:11AM +0100, Johannes Schindelin wrote:\n> > Didn't I mention that it was a severe limitation to think of the sha1 \n> > mapping of a 1-to-1 mapping? Think of it more as a relation.\n> \n> The mapping is used in several operations.\n> First, there are several things that can happen to a commit\n> \n> - it's pruned. This includes, for me, path pruning, matching\n> and a commit filter returning no SHA1s.\n\nOkay.\n\n> - it's rewritten to another commit that can be considered the\n> \"moral equivalent\" of that commit. This occurs when a commit\n> is not pruned, but something else happened to the commit itself or\n> one of its ancestors. This excludes, for me, the case\n> where a commit filter returns more than one SHA1.\n\nOkay. For me it does not at all exclude that. If I want to replace a \ncommit by no commit, I write a commit-filter which does not return \nanything. If I return more than one SHA1s, I damned well want all of \nthose be the replacement \"commit\".\n\nTo say \"the\" replacement \"commit\", means to mistake the mapping as a \nfunction, a non-relation.\n\n> - it's replaced by more than one SHA1. This can only happen\n> in a commit filter.\n\nFor the moment, yes.\n\n> There are at least four operations in which this mapping is used:\n> \n> - if the parents of a commit have been rewritten to one or more\n> commits, then they are replaced by the new commits.\n\nYes, that is the primary use for the mapping.\n\n> If any parent has been pruned, then it is replaced by\n> the result of applying this operation on _its_ parents.\n\nWhy? This is overy complicated. If a commit has been pruned, why does \nthe mapping not point to the _non-pruned_ parent? IOW if you have \nsomething like this:\n\n\tA - B - C - D - E - F\n\nand all commits except A and F are pruned, the mapping for A, B, C, D and \nE should _all_ point to the (possibly rewritten) A.\n\n> - any reference (in refs/) that points to a rewritten or pruned\n> commit is removed and\n> * if the commit was rewritten to a single commit, then it is\n> replaced by this commit\n> * otherwise, there is no moral equivalent single commit, but\n> we want to ensure we can still access the new commits, so\n> I create several references, either to each of the many\n> commits the old commit was rewritten to, or to each of\n> its nearest unpruned ancestors (i.e., the same set as\n> described in the previous operation).\n\nI'd argue that it should be an error if a to-be-rewritten ref (and I still \nstrongly disagree with you that all refs should be rewritten) would point \nto multiple commits. Possibly overridable with \"--allow-octopus-refs\". \nBut the default should be to error out.\n\n> - a SHA1 of a commit that appears in a commit message is replaced\n> by the rewritten commit iff it was rewritten to a single commit.\n> That is, if the commit was pruned or rewritten (through a commit\n> filter to more than one commit), then the SHA1 is left alone.\n\nBoth this behaviour and the one you described in your reply are wrong.\n\n> - the mapping available to filters\n> * if the commit was pruned, an empty file is created\n> * otherwise a file is created containing all rewritten SHA1s\n\nAs I stated above: it is utterly wrong to create an empty mapping for a \ncommit that was pruned. It does not take long to think of an example:\n\n\tA - B - C - D\n\nNow, A and D get pruned. Do you want the whole branch to vanish? _Hell, \nno_.\n\n> I understand you want the second operation to only apply to refs \n> explicitly mentioned on the command line.\n\nYou have to at least give the users a chance to grasp what they are doing. \nAnd if that means to change the semantics to something saner, then so be \nit.\n\nCiao,\nDscho\n"},{"id":"47727","messageId":"20070718120502.GZ999MdfPADPa@greensroom.kotnet.org","threadId":"9008","inReplyTo":"Pine.LNX.4.64.0707181153200.14781@racer.site","subject":"Re: [PATCH 6/6] Add git-rewrite-commits","fromName":"Sven Verdoolaege","fromEmail":"skimo@kotnet.org","sentAt":"2007-07-18T12:05:02Z","receivedAt":"2007-07-18T12:05:02Z","isPatch":true,"sender":{"key":"skimo@kotnet.org","avatar":null},"body":"On Wed, Jul 18, 2007 at 12:02:50PM +0100, Johannes Schindelin wrote:\n> Hi,\n> \n> On Mon, 16 Jul 2007, Sven Verdoolaege wrote:\n> \n> > On Mon, Jul 16, 2007 at 01:38:11AM +0100, Johannes Schindelin wrote:\n> > > On Sun, 15 Jul 2007, Sven Verdoolaege wrote:\n> > > > TREECHANGE is only set when path pruning is in effect.\n> > > > If I didn't check for path_pruning, then all commits would be\n> > > > considered to have been pruned. (Or am I missing something?\n> > > > Honestly, I found all that TREECHANGE stuff difficult to follow.)\n> > > \n> > > AFAICT TREECHANGE means that parents were rewritten.\n> > \n> > I think you'll find that if all commits touch a path in the\n> > path specifiers then all commits will have TREECHANGE set and\n> > so no parents will be rewritten.\n> \n> The code suggests otherwise.\n\nCheck again.\n\n> But I really have to wonder: why do you play games with TREECHANGE? I had \n> the impression that commit->parents is set appropriately by the revision \n> walker,\n\nOnly for unpruned commits and the references (explicitly specified\non the command line if you wish) may have been pruned.\n\n> > > > revision.c itself is also riddled with \"prune_fn && \".\n> > > > Wouldn't it make sense to invert the meaning of this bit and call\n> > > > it, say, PRUNED, so that the default is off and you would only\n> > > > have to check if the bit was set ?\n> > > \n> > > You meant the TREECHANGE bit? No.\n> > \n> > Yes. Why?\n> \n> Why invert the meaning of a perfectly fine bit? Because you can? It is \n> working right now, and it is not even a buglet, so what is there to fix?\n\nBecause it is confusing. As explained above, the bit doesn't have a\nmeaning of its own. You can only interpret the bit if some other\nconditions are met.\nIt would be even more confusing if it meant what you claim it means.\n\n> \n> > > BTW what do you plan to do about my objection to UNINTERESTING, given \n> > > the example \"git rewrite-commits A..B x/y\"?\n> > \n> > That was based on an apparent misunderstanding of my code\n> > that I tried to address above. I did not intend to do what\n> > you claim I do and a quick test confirms that my code does\n> > indeed not to what you claim it does.\n> > \n> > More specifically, the history will not be cut off at A\n> > because A is marked UNINTERESTING and is therefore not considered\n> > to have been pruned.\n> \n> Why do you test for TREECHANGE | UNINTERESTING then?\n\nExactly for the reason mentioned above.\nIf the commit is marked UNINTERESTING then it has not been pruned,\nbecause it hasn't even been checked for TREECHANGE.\n\n> > A commit is considered pruned if it was either explicitly marked\n> > as such or if TREECHANGE is not set, but the later check (in is_pruned)\n> > is only done on commits that were checked for tree changes.\n> \n> I don't understand. What do you mean by \"a commit is pruned\"? Does it \n> mean that this commit was left out from the revision walk? What does that \n> have to do with TREECHANGE, which means that the parents set was modified?\n\nYou just claim that that is what it means. The code (see try_to_simplify_commit\nwhere the bit is set) and a simple experiment (explained above) show otherwise.\n\nskimo\n"},{"id":"47857","messageId":"20070719124053.GC999MdfPADPa@greensroom.kotnet.org","threadId":"9008","inReplyTo":"Pine.LNX.4.64.0707181205260.14781@racer.site","subject":"Re: [PATCH 6/6] Add git-rewrite-commits","fromName":"Sven Verdoolaege","fromEmail":"skimo@kotnet.org","sentAt":"2007-07-19T12:40:53Z","receivedAt":"2007-07-19T12:40:53Z","isPatch":true,"sender":{"key":"skimo@kotnet.org","avatar":null},"body":"On Wed, Jul 18, 2007 at 12:17:03PM +0100, Johannes Schindelin wrote:\n> Okay. For me it does not at all exclude that. If I want to replace a \n> commit by no commit, I write a commit-filter which does not return \n> anything. If I return more than one SHA1s, I damned well want all of \n> those be the replacement \"commit\".\n\nSo how about you telling me what it _means_ for one commit to\nbe replaced by more than one commit or at least giving me an\nexample?\n\n> > - if the parents of a commit have been rewritten to one or more\n> > commits, then they are replaced by the new commits.\n> \n> Yes, that is the primary use for the mapping.\n> \n> > If any parent has been pruned, then it is replaced by\n> > the result of applying this operation on _its_ parents.\n> \n> Why? This is overy complicated. If a commit has been pruned, why does \n> the mapping not point to the _non-pruned_ parent?\n\nIt may not have any non-pruned parents and for the pruned ones, we\nwouldn't want to lose the relation with the non-pruned ancestors.\n\n> IOW if you have \n> something like this:\n> \n> \tA - B - C - D - E - F\n> \n> and all commits except A and F are pruned, the mapping for A, B, C, D and \n> E should _all_ point to the (possibly rewritten) A.\n\nI'm not sure what you mean by \"mapping\" here, but the operation\ndescribed above would make all of B, C, D, E and F have (the\npossibly rewritten) A as single parent (and parenthood was all\nI was talking about above).\n\n> > - a SHA1 of a commit that appears in a commit message is replaced\n> > by the rewritten commit iff it was rewritten to a single commit.\n> > That is, if the commit was pruned or rewritten (through a commit\n> > filter to more than one commit), then the SHA1 is left alone.\n> \n> Both this behaviour and the one you described in your reply are wrong.\n\nSo tell me what you would do then and why that would make sense.\n\n> > - the mapping available to filters\n> > * if the commit was pruned, an empty file is created\n> > * otherwise a file is created containing all rewritten SHA1s\n> \n> As I stated above: it is utterly wrong to create an empty mapping for a \n> commit that was pruned. It does not take long to think of an example:\n> \n> \tA - B - C - D\n> \n> Now, A and D get pruned. Do you want the whole branch to vanish? _Hell, \n> no_.\n\nDefine \"vanish\" and, again, tell me what you would do.\n\n> You have to at least give the users a chance to grasp what they are doing. \n> And if that means to change the semantics to something saner, then so be \n> it.\n\nLet's get things straight. I've added the map files and the possibility\nfor a commit filter to return more than one commit because you asked me to.\nI've tried to make sense of it, but if you think the behavior I defined\nis not what it is supposed to be, then it is up to _you_ to tell me what\nyou think it should be instead of letting me guess.\n\nI think I'll just remove the possibility for the commit filter to\nreturn more than one SHA1 (or maybe even no SHA1s).\nfilter-branch doesn't seem to allow either of those either.\n\nskimo\n"}]}