Re: [PATCH 08/10] fast-export: add --reference-excluded-parents option
- From
Jeff King <peff@peff.net>
- Date
- Nov 11, 2018, 07:11 UTC
- Message-ID
- <20181111071102.GH30850@sigill.intra.peff.net>
- In-Reply-To
- <20181111062312.16342-9-newren@gmail.com>
On Sat, Nov 10, 2018 at 10:23:10PM -0800, Elijah Newren wrote:
Show 7 quoted lines
> git filter-branch has a nifty feature allowing you to rewrite, e.g. just > the last 8 commits of a linear history > git filter-branch $OPTIONS HEAD~8..HEAD > > If you try the same with git fast-export, you instead get a history of > only 8 commits, with HEAD~7 being rewritten into a root commit. There > are two alternatives:
Ah, I think this maybe answers some of my earlier questions, too. You cannot use fast-import as it stands to do a partial rewrite.
Show 13 quoted lines
> 1) Don't use the negative revision specification, and when you're > filtering the output to make modifications to the last 8 commits, > just be careful to not modify any earlier commits somehow. > > 2) First run 'git fast-export --export-marks=somefile HEAD~8', then > run 'git fast-export --import-marks=somefile HEAD~8..HEAD'. > > Both are more error prone than I'd like (the first for obvious reasons; > with the second option I have sometimes accidentally included too many > revisions in the first command and then found that the corresponding > extra revisions were not exported by the second command and thus were > not modified as I expected). Also, both are poor from a performance > perspective.
Yeah, this should be O(commits you're touching), and it the current code does not allow that at all. So I think this feature makes a lot of sense (it probably _should_ have been the default, but it's a bit late for that now).
Show 25 quoted lines
> @@ -638,13 +640,21 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
> unuse_commit_buffer(commit, commit_buffer);
>
> for (i = 0, p = commit->parents; p; p = p->next) {
> - int mark = get_object_mark(&p->item->object);
> - if (!mark)
> + struct object *obj = &p->item->object;
> + int mark = get_object_mark(obj);
> +
> + if (!mark && !reference_excluded_commits)
> continue;
> if (i == 0)
> - printf("from :%d\n", mark);
> + printf("from ");
> + else
> + printf("merge ");
> + if (mark)
> + printf(":%d\n", mark);
> else
> - printf("merge :%d\n", mark);
> + printf("%s\n", sha1_to_hex(anonymize ?
> + anonymize_sha1(&obj->oid) :
> + obj->oid.hash));
> i++;
> }OK, so this just teaches us to start with the sensible "from" directive. I think we might be able to do a little more optimization here. If we're exporting HEAD^..HEAD and there's an object in HEAD^ which is unchanged in HEAD, I think we'd still print it (because it would not be marked SHOWN), but we could omit it (by walking the tree of the boundary commits and marking them shown).
I don't think it's a blocker for what you're doing here, but just a possible future optimization.
Show 25 quoted lines
> @@ -925,13 +935,22 @@ static void handle_tags_and_duplicates(struct string_list *extras)
> /*
> * Getting here means we have a commit which
> * was excluded by a negative refspec (e.g.
> - * fast-export ^master master). If the user
> + * fast-export ^master master). If we are
> + * referencing excluded commits, set the ref
> + * to the exact commit. Otherwise, the user
> * wants the branch exported but every commit
> - * in its history to be deleted, that sounds
> - * like a ref deletion to me.
> + * in its history to be deleted, which basically
> + * just means deletion of the ref.
> */
> - printf("reset %s\nfrom %s\n\n",
> - name, sha1_to_hex(null_sha1));
> + if (!reference_excluded_commits) {
> + /* delete the ref */
> + printf("reset %s\nfrom %s\n\n",
> + name, sha1_to_hex(null_sha1));
> + continue;
> + }
> + /* set ref to commit using oid, not mark */
> + printf("reset %s\nfrom %s\n\n", name,
> + sha1_to_hex(commit->object.oid.hash));OK, and this is basically answering my earlier questions again: yes, you _would_ want to keep old tags pointing at their commits. But only in this much more sensible mode.
-Peff