On Wed, May 26 2021, Taylor Blau wrote:
> On Wed, May 26, 2021 at 08:30:49PM +0200, Ævar Arnfjörð Bjarmason wrote:
>>
>> On Wed, Mar 31 2021, Taylor Blau wrote:
>>
>> > Add a new 'bitmap' test-tool which can be used to list the commits that
>> > have received bitmaps.
>> > [...]
>> > @@ -0,0 +1,24 @@
>> > +#include "test-tool.h"
>> > +#include "cache.h"
>> > +#include "pack-bitmap.h"
>>
>> Since this commit SunCC (on Solaris) refuses to compile git, a new bug
>> in v2.32.0-rc*.
>>
>> It's because it can't find the oe_get_size_slow symbol, you include
>> pack-bitmap.h, which in turn includes pack-objects.h. That file
>> references oe_get_size_slow, but it's only defined in
>> builtin/pack-objects.c.
>
> I'm not sure I understand. pack-objects.h has a declaration of that
> method, but the implementation is in builtin/pack-objects.c. That should
> be fine, but I don't know about how SunCC works.
>
> What needs to be changed here?
I think that oe_get_size_slow needs to be in libgit as long as
pack-objects.h defines other (inline) functions that reference it, or
maybe most of that stuff can just be moved to builtin/pack-objects.h?
This obviously not OK patch makes things "ok" again under SunCC:
diff --git a/Makefile b/Makefile
index c3565fc0f8f..c2b05aeddac 100644
--- a/Makefile
+++ b/Makefile
@@ -1186,7 +1186,7 @@ THIRD_PARTY_SOURCES += compat/regex/%
THIRD_PARTY_SOURCES += sha1collisiondetection/%
THIRD_PARTY_SOURCES += sha1dc/%
-GITLIBS = common-main.o $(LIB_FILE) $(XDIFF_LIB)
+GITLIBS = builtin/pack-objects.o common-main.o $(LIB_FILE) $(XDIFF_LIB)
EXTLIBS =
GIT_USER_AGENT = git/$(GIT_VERSION)
I.e. the problem is that in the final program we're linking there's
references to this function we can't find:
Undefined first referenced
symbol in file
oe_get_size_slow t/helper/test-bitmap.o
ld: fatal: symbol referencing errors. No output written to t/helper/test-tool
I think e.g. the GNU toolchain and others are probably OK with it
because they do some analysis to figure out that none of those inline
functions that need oe_get_size_slow are themselves needed. SunCC (or
Solaris's) linker seems to be more eager.
It seems to me that the best way to solve this is something like the
below code-move-only patch of just moving this stuff only used by
builtin/pack-objects.c to that file. This also fixes the build on
Solaris.
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 6ded130e45b..70072b07b6b 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -37,6 +37,100 @@
#include "shallow.h"
#include "promisor-remote.h"
+/*
+ * Objects we are going to pack are collected in the `to_pack` structure.
+ * It contains an array (dynamically expanded) of the object data, and a map
+ * that can resolve SHA1s to their position in the array.
+ */
+static struct packing_data to_pack;
+
+/*
+ * Return the size of the object without doing any delta
+ * reconstruction (so non-deltas are true object sizes, but deltas
+ * return the size of the delta data).
+ */
+unsigned long oe_get_size_slow(struct packing_data *pack,
+ const struct object_entry *e);
+unsigned long oe_get_size_slow(struct packing_data *pack,
+ const struct object_entry *e)
+{
+ struct packed_git *p;
+ struct pack_window *w_curs;
+ unsigned char *buf;
+ enum object_type type;
+ unsigned long used, avail, size;
+
+ if (e->type_ != OBJ_OFS_DELTA && e->type_ != OBJ_REF_DELTA) {
+ packing_data_lock(&to_pack);
+ if (oid_object_info(the_repository, &e->idx.oid, &size) < 0)
+ die(_("unable to get size of %s"),
+ oid_to_hex(&e->idx.oid));
+ packing_data_unlock(&to_pack);
+ return size;
+ }
+
+ p = oe_in_pack(pack, e);
+ if (!p)
+ BUG("when e->type is a delta, it must belong to a pack");
+
+ packing_data_lock(&to_pack);
+ w_curs = NULL;
+ buf = use_pack(p, &w_curs, e->in_pack_offset, &avail);
+ used = unpack_object_header_buffer(buf, avail, &type, &size);
+ if (used == 0)
+ die(_("unable to parse object header of %s"),
+ oid_to_hex(&e->idx.oid));
+
+ unuse_pack(&w_curs);
+ packing_data_unlock(&to_pack);
+ return size;
+}
+
+static inline unsigned long oe_size(struct packing_data *pack,
+ const struct object_entry *e)
+{
+ if (e->size_valid)
+ return e->size_;
+
+ return oe_get_size_slow(pack, e);
+}
+
+static inline int oe_size_less_than(struct packing_data *pack,
+ const struct object_entry *lhs,
+ unsigned long rhs)
+{
+ if (lhs->size_valid)
+ return lhs->size_ < rhs;
+ if (rhs < pack->oe_size_limit) /* rhs < 2^x <= lhs ? */
+ return 0;
+ return oe_get_size_slow(pack, lhs) < rhs;
+}
+
+static inline int oe_size_greater_than(struct packing_data *pack,
+ const struct object_entry *lhs,
+ unsigned long rhs)
+{
+ if (lhs->size_valid)
+ return lhs->size_ > rhs;
+ if (rhs < pack->oe_size_limit) /* rhs < 2^x <= lhs ? */
+ return 1;
+ return oe_get_size_slow(pack, lhs) > rhs;
+}
+
+static inline void oe_set_size(struct packing_data *pack,
+ struct object_entry *e,
+ unsigned long size)
+{
+ if (size < pack->oe_size_limit) {
+ e->size_ = size;
+ e->size_valid = 1;
+ } else {
+ e->size_valid = 0;
+ if (oe_get_size_slow(pack, e) != size)
+ BUG("'size' is supposed to be the object size!");
+ }
+}
+
#define IN_PACK(obj) oe_in_pack(&to_pack, obj)
#define SIZE(obj) oe_size(&to_pack, obj)
#define SET_SIZE(obj,size) oe_set_size(&to_pack, obj, size)
@@ -56,13 +150,6 @@ static const char *pack_usage[] = {
NULL
};
-/*
- * Objects we are going to pack are collected in the `to_pack` structure.
- * It contains an array (dynamically expanded) of the object data, and a map
- * that can resolve SHA1s to their position in the array.
- */
-static struct packing_data to_pack;
-
static struct pack_idx_entry **written_list;
static uint32_t nr_result, nr_written, nr_seen;
static struct bitmap_index *bitmap_git;
@@ -2231,46 +2318,6 @@ static pthread_mutex_t progress_mutex;
* progress_mutex for protection.
*/
-/*
- * Return the size of the object without doing any delta
- * reconstruction (so non-deltas are true object sizes, but deltas
- * return the size of the delta data).
- */
-unsigned long oe_get_size_slow(struct packing_data *pack,
- const struct object_entry *e)
-{
- struct packed_git *p;
- struct pack_window *w_curs;
- unsigned char *buf;
- enum object_type type;
- unsigned long used, avail, size;
-
- if (e->type_ != OBJ_OFS_DELTA && e->type_ != OBJ_REF_DELTA) {
- packing_data_lock(&to_pack);
- if (oid_object_info(the_repository, &e->idx.oid, &size) < 0)
- die(_("unable to get size of %s"),
- oid_to_hex(&e->idx.oid));
- packing_data_unlock(&to_pack);
- return size;
- }
-
- p = oe_in_pack(pack, e);
- if (!p)
- BUG("when e->type is a delta, it must belong to a pack");
-
- packing_data_lock(&to_pack);
- w_curs = NULL;
- buf = use_pack(p, &w_curs, e->in_pack_offset, &avail);
- used = unpack_object_header_buffer(buf, avail, &type, &size);
- if (used == 0)
- die(_("unable to parse object header of %s"),
- oid_to_hex(&e->idx.oid));
-
- unuse_pack(&w_curs);
- packing_data_unlock(&to_pack);
- return size;
-}
-
static int try_delta(struct unpacked *trg, struct unpacked *src,
unsigned max_depth, unsigned long *mem_usage)
{
diff --git a/pack-objects.h b/pack-objects.h
index 9d88e3e518f..0c4d5d475f6 100644
--- a/pack-objects.h
+++ b/pack-objects.h
@@ -332,52 +332,6 @@ static inline void oe_set_delta_sibling(struct packing_data *pack,
e->delta_sibling_idx = 0;
}
-unsigned long oe_get_size_slow(struct packing_data *pack,
- const struct object_entry *e);
-static inline unsigned long oe_size(struct packing_data *pack,
- const struct object_entry *e)
-{
- if (e->size_valid)
- return e->size_;
-
- return oe_get_size_slow(pack, e);
-}
-
-static inline int oe_size_less_than(struct packing_data *pack,
- const struct object_entry *lhs,
- unsigned long rhs)
-{
- if (lhs->size_valid)
- return lhs->size_ < rhs;
- if (rhs < pack->oe_size_limit) /* rhs < 2^x <= lhs ? */
- return 0;
- return oe_get_size_slow(pack, lhs) < rhs;
-}
-
-static inline int oe_size_greater_than(struct packing_data *pack,
- const struct object_entry *lhs,
- unsigned long rhs)
-{
- if (lhs->size_valid)
- return lhs->size_ > rhs;
- if (rhs < pack->oe_size_limit) /* rhs < 2^x <= lhs ? */
- return 1;
- return oe_get_size_slow(pack, lhs) > rhs;
-}
-
-static inline void oe_set_size(struct packing_data *pack,
- struct object_entry *e,
- unsigned long size)
-{
- if (size < pack->oe_size_limit) {
- e->size_ = size;
- e->size_valid = 1;
- } else {
- e->size_valid = 0;
- if (oe_get_size_slow(pack, e) != size)
- BUG("'size' is supposed to be the object size!");
- }
-}
static inline unsigned long oe_delta_size(struct packing_data *pack,
const struct object_entry *e)
