On 4/30/07, Shawn O. Pearce <spearce@xxxxxxxxxxx> wrote:
Dana How <danahow@xxxxxxxxx> wrote:
> Add our own version of the one in fast-import.c here.
> Needed later to correct bad object count in header for split pack.
...
> diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
> --- a/builtin-pack-objects.c
> +++ b/builtin-pack-objects.c
> @@ -562,6 +562,42 @@ static off_t write_one(struct sha1file *f,
> +static void fixup_header_footer(int pack_fd, unsigned char *pack_file_sha1,
> + char *pack_name, uint32_t object_count)
> +{
This looks a *lot* like the code in fast-import.c.
I hope so! That's where I copied it from.
Why not
refactor both to use the same implementation and stuff it away in
say pack-check.c (for lack of a better place), or start a new file
(pack-write.c)?
Actually I didn't just copy it, I tried to rewrite it for my use
as well as the fast-import.c use (note there is a 3rd copy
in some *index*.c file which I didn't try to merge in yet).
However I didn't yet put it in a new file or change fast-import.c
to call it since I wanted to change as little as possible.
There is a *lot* of code in fast-import.c (over 2,000 lines) that
was half-copied from other core code, and that was half created
on its own. This is also true in index-pack.c and pack-objects.c;
I'd like to see these implementations unify more rather than copy
code from each other.
I know git-blame will identify the original author quite well,
but I'd really like to avoid adding lots more code to maintain
if we can avoid it.
I agree with all your arguments. I had several reasons
to avoid extra rearrangements/refactorings:
(a) First patch to git, not previously known to me;
(b) I prefer to separate new functionality from "clean-up" work;
(c) I didn't really view myself as the person to make the *writing*
code in git as well organized/minimized as the *reading* code
[e.g. the sliding mmap stuff -- nice!];
(d) Apparently you and Nicolas Pitre have a lot of pending changes
affecting the packing code.
I'd have no problem submitting a follow-on patch later containing
some clean-up work if you & NP clear it, so I know I won't have
problems from (d). Note I had to completely rewrite this patch
when NP submitted some of his pending stuff.
NP wrote that you posted a summary of v3/v4 pack ideas,
but I couldn't find it in the list archives. Could you either
email it to me, or (re-)post it to the list?
Thanks,
--
Dana L. How danahow@xxxxxxxxx +1 650 804 5991 cell
-
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