Re: [RFC PATCH 2/5] Add delta-islands.{c,h}

On Sun, Jul 22, 2018 at 10:50 AM, Duy Nguyen [off-list ref] wrote:

On Sun, Jul 22, 2018 at 7:51 AM Christian Couder
[off-list ref] wrote:

quoted

+pack.island::
+       A regular expression configuring a set of delta islands. See
+       "DELTA ISLANDS" in linkgit:git-pack-objects[1] for details.
+

That section is not added until 3/5 though.

Yeah, so I guess it is better to move this hunk to 3/5 and keep
pack.island undocumented until the delta islands code is actually used
by pack-objects.

quoted

diff --git a/delta-islands.c b/delta-islands.c
new file mode 100644
index 0000000000..645fe966c5
--- /dev/null
+++ b/delta-islands.c

@@ -0,0 +1,490 @@
+#include "builtin.h"

A bit weird that builtin.h would be needed...

Yeah, I will get rid of that include in the next iteration.

quoted

+       if (progress)
+               progress_state = start_progress("Propagating island marks", nr);

_() (same comment for other strings too)

Ok, the strings will be marked for translation in the next iteration.

quoted

diff --git a/pack-objects.h b/pack-objects.h
index edf74dabdd..8eecd67991 100644
--- a/pack-objects.h
+++ b/pack-objects.h

@@ -100,6 +100,10 @@ struct object_entry {
        unsigned type_:TYPE_BITS;
        unsigned no_try_delta:1;
        unsigned in_pack_type:TYPE_BITS; /* could be delta */
+
+       unsigned int tree_depth; /* should be repositioned for packing? */
+       unsigned char layer;
+

This looks very much like an optional feature. To avoid increasing
pack-objects memory usage for common case, please move this to struct
packing_data.

Ok, I will take a look at that.

Thanks for the review,
Christian.