shortlog: de-duplicate trailer values
The current documentation is vague about what happens with --group=trailer:signed-off-by when we see a commit with: Signed-off-by: One Signed-off-by: Two Signed-off-by: One We clearly should credit both "One" and "Two", but should "One" get credited twice? The current code does so, but mostly because that was the easiest thing to do. It's probably more useful to count each commit at most once. This will become especially important when we allow values from multiple sources in a future patch. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King committed
Sep 27, 2020 at 04:40 UTC
f17b0b99bf2dc2fcd74544ce35d058e558e6b056
3 files changed
+88
-1
Documentation/git-shortlog.txt
+2
-1
@@ -61,7 +61,8 @@ OPTIONS
61
+
62
Note that commits that do not include the trailer will not be counted.
63
Likewise, commits with multiple trailers (e.g., multiple signoffs) may
64
-be counted more than once.
64
+be counted more than once (but only once per unique trailer value in
65
+that commit).
66
+
67
The contents of each trailer value are taken literally and completely.
68
No mailmap is applied, and the `-e` option has no effect (if the trailer
builtin/shortlog.c
+58
@@ -166,6 +166,60 @@ static void read_from_stdin(struct shortlog *log)
166
strbuf_release(&oneline);
167
}
168
169
+struct strset_item {
170
+ struct hashmap_entry ent;
171
+ char value[FLEX_ARRAY];
172
+};
173
+
174
+struct strset {
175
+ struct hashmap map;
176
+};
177
+
178
+#define STRSET_INIT { { NULL } }
179
+
180
+static int strset_item_hashcmp(const void *hash_data,
181
+ const struct hashmap_entry *entry,
182
+ const struct hashmap_entry *entry_or_key,
183
+ const void *keydata)
184
+{
185
+ const struct strset_item *a, *b;
186
+
187
+ a = container_of(entry, const struct strset_item, ent);
188
+ if (keydata)
189
+ return strcmp(a->value, keydata);
190
+
191
+ b = container_of(entry_or_key, const struct strset_item, ent);
192
+ return strcmp(a->value, b->value);
193
+}
194
+
195
+/*
196
+ * Adds "str" to the set if it was not already present; returns true if it was
197
+ * already there.
198
+ */
199
+static int strset_check_and_add(struct strset *ss, const char *str)
200
+{
201
+ unsigned int hash = strhash(str);
202
+ struct strset_item *item;
203
+
204
+ if (!ss->map.table)
205
+ hashmap_init(&ss->map, strset_item_hashcmp, NULL, 0);
206
+
207
+ if (hashmap_get_from_hash(&ss->map, hash, str))
208
+ return 1;
209
+
210
+ FLEX_ALLOC_STR(item, value, str);
211
+ hashmap_entry_init(&item->ent, hash);
212
+ hashmap_add(&ss->map, &item->ent);
213
+ return 0;
214
+}
215
+
216
+static void strset_clear(struct strset *ss)
217
+{
218
+ if (!ss->map.table)
219
+ return;
220
+ hashmap_free_entries(&ss->map, struct strset_item, ent);
221
+}
222
+
223
static void insert_records_from_trailers(struct shortlog *log,
224
struct commit *commit,
225
struct pretty_print_context *ctx,
@@ -173,6 +227,7 @@ static void insert_records_from_trailers(struct shortlog *log,
227
{
228
struct trailer_iterator iter;
229
const char *commit_buffer, *body;
230
+ struct strset dups = STRSET_INIT;
231
232
/*
233
* Using format_commit_message("%B") would be simpler here, but
@@ -190,10 +245,13 @@ static void insert_records_from_trailers(struct shortlog *log,
245
if (strcasecmp(iter.key.buf, log->trailer))
246
continue;
247
248
+ if (strset_check_and_add(&dups, value))
249
+ continue;
250
insert_one_record(log, value, oneline);
251
}
252
trailer_iterator_release(&iter);
253
254
+ strset_clear(&dups);
255
unuse_commit_buffer(commit, commit_buffer);
256
}
257
t/t4201-shortlog.sh
+28
@@ -234,4 +234,32 @@ test_expect_success 'shortlog --group=trailer:signed-off-by' '
234
test_cmp expect actual
235
'
236
237
+test_expect_success 'shortlog de-duplicates trailers in a single commit' '
238
+ git commit --allow-empty -F - <<-\EOF &&
239
+ subject one
240
+
241
+ this message has two distinct values, plus a repeat
242
+
243
+ Repeated-trailer: Foo
244
+ Repeated-trailer: Bar
245
+ Repeated-trailer: Foo
246
+ EOF
247
+
248
+ git commit --allow-empty -F - <<-\EOF &&
249
+ subject two
250
+
251
+ similar to the previous, but without the second distinct value
252
+
253
+ Repeated-trailer: Foo
254
+ Repeated-trailer: Foo
255
+ EOF
256
+
257
+ cat >expect <<-\EOF &&
258
+ 2 Foo
259
+ 1 Bar
260
+ EOF
261
+ git shortlog -ns --group=trailer:repeated-trailer -2 HEAD >actual &&
262
+ test_cmp expect actual
263
+'
264
+
265
test_done