Re: [PATCH v5 2/5] pack-redundant: new algorithm to find min packs
From: SZEDER Gábor <hidden>
Date: 2019-01-11 01:19:53
On Thu, Jan 10, 2019 at 08:01:39PM +0800, Jiang Xin wrote:
quoted hunk ↗ jump to hunk
diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c index cf9a9aabd4..3655cc7dc6 100644 --- a/builtin/pack-redundant.c +++ b/builtin/pack-redundant.c
quoted hunk ↗ jump to hunk
@@ -446,49 +484,37 @@ static void minimize(struct pack_list **min) pl = pl->next; } + *min = unique; + /* return if there are no objects missing from the unique set */ if (missing->size == 0) { - *min = unique; free(missing); return; } - /* find the permutations which contain all missing objects */ - for (n = 1; n <= pack_list_size(non_unique) && !perm_ok; n++) { - perm_all = perm = get_permutations(non_unique, n); - while (perm) { - if (is_superset(perm->pl, missing)) { - new_perm = xmalloc(sizeof(struct pll)); - memcpy(new_perm, perm, sizeof(struct pll)); - new_perm->next = perm_ok; - perm_ok = new_perm; - } - perm = perm->next; - } - if (perm_ok) - break; - pll_free(perm_all); - }
Please make sure that all commits in the patch series can be build
cleanly without any warnings (with '-Werror' or preferably with 'make
DEVELOPER=1') and pass the test suite. This is important, because
unbuildable commits will cause trouble later on, when e.g. 'git
bisect' happens to pick such a commit.
In this case, the removal of the above loop removes all callsites of
the static functions get_permutations(), is_superset(), and
pll_free(), resulting the following compiler error:
builtin/pack-redundant.c: At top level:
builtin/pack-redundant.c:289:13: error: ‘pll_free’ defined but not used [-Werror=unused-function]
static void pll_free(struct pll *l)
^
builtin/pack-redundant.c:309:21: error: ‘get_permutations’ defined but not used [-Werror=unused-function]
static struct pll * get_permutations(struct pack_list *list, int n)
^
builtin/pack-redundant.c:343:12: error: ‘is_superset’ defined but not used [-Werror=unused-function]
static int is_superset(struct pack_list *pl, struct llist *list)
^
I see that the last patch in this series removes those three
unused functions, but that patch should be squashed into this one to
keep Git buildable with '-Werror' or DEVELOPER=1.
Furthermore, after building this patch (without '-Werror'), several
tests in 't5323-pack-redundant.sh' fail. To avoid the test failure I
think the fourth patch ensuring a consistent sort order should be
squashed in as well.