Jeff King <peff@xxxxxxxx> writes:
> On Thu, May 22, 2014 at 11:33:04PM +0200, Christian Couder wrote:
>
>> The usage string for this option is:
>>
>> git replace [-f] --graft <commit> [<parent>...]
>>
>> First we create a new commit that is the same as <commit>
>> except that its parents are [<parents>...]
>>
>> Then we create a replace ref that replace <commit> with
>> the commit we just created.
>>
>> With this new option, it should be straightforward to
>> convert grafts to replace refs, with something like:
>>
>> cat .git/info/grafts | while read line
>> do git replace --graft $line; done
>
> I think this script at the end should end up in the documentation or a
> contrib script (probably the former, as it is short enough that somebody
> might just cut-and-paste).
>
> The graft file ignores comments and blank lines, so maybe "grep '^[^#]'"
> would be in order.
>
> And maybe it's just me, but I think spacing it like:
>
> grep '^[^#]' .git/info/grafts |
> while read line; do
> git replace --graft $line
> done
>
> is more readable (I think some would even argue for putting the "do" on
> a separate line).
Yes, I would ;-)
I just read read_graft_line(); it allows an empty line (both
length-0 before the terminating LF or CRLF, and a line with
isspace() only) and ignore them, so "grep '^[^#]'" is not
sufficient.
>> + /* make sure the commit is not corrupt */
>> + if (parse_commit_buffer(commit, buf.buf, buf.len))
>> + die(_("Could not parse commit: '%s'"), old_ref);
>
> I guess the checks here are sufficient to make...
>
>> + /* find existing parents */
>> + parent_start = buf.buf;
>> + parent_start += 46; /* "tree " + "hex sha1" + "\n" */
>> + parent_end = parent_start;
>> +
>> + while (starts_with(parent_end, "parent "))
>> + parent_end += 48; /* "parent " + "hex sha1" + "\n" */
>
> ...this number-based parsing safe, though it would miss removing a stray
> parent line elsewhere in the commit. It still feels rather magical to
> me, as we are depending on unspoken format guarantees defined inside
> parse_commit_buffer.
Do you mean "we have a carved-in-stone rule that all 'parent ' lines
must come immediately after a single 'tree ' line, and that is
implemented in parse_commit_buffer(). The above code follows the
exact same rule. It would be nice to have some mechanism to tell
somebody who wants to update the former that s/he must update this
new code at the same time"?
I think a commit object that violates the rule is simply broken, and
it is OK to add a mode to tell parse-commit-buffer to tolerate such
a broken object, if only so that filter-branch or some other tools
can be used to correct a history that contains it.
Perhaps a more future-proof way to write Christian's code may be:
- find "tree ";
- splice the new parents in immediately after that "tree " line;
- starting from the end of these new parents, scan up to the end
of the header line-by-line, and splice out any line that
begins with "parent ".
which may not be too bad.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html