* What's cooking (draft for #4's issue this month) @ 2021-04-14 1:11 Junio C Hamano 2021-04-14 9:43 ` Patrick Steinhardt 2021-04-14 23:22 ` Junio C Hamano 0 siblings, 2 replies; 14+ messages in thread From: Junio C Hamano @ 2021-04-14 1:11 UTC (permalink / raw) To: git A handful of topics have been merged to 'master' as the 9th batch of this cycle. 54a3917115 The ninth batch e0d4a63c09 Merge branch 'vs/completion-with-set-u' e6545201ad Merge branch 'ab/detox-config-gettext' a9414b86ac Merge branch 'gk/gitweb-redacted-email' 8446b388b1 Merge branch 'cc/test-helper-bloom-usage-fix' 2279289e95 Merge branch 'ab/send-email-validate-errors' 4c6ac2da2c Merge branch 'tb/precompose-prefix-simplify' 1d5fbd45c4 Merge branch 'fm/user-manual-use-preface' 7b55441db1 Merge branch 'ab/perl-do-not-abuse-map' 0623669fc6 Merge branch 'tb/pack-preferred-tips-to-give-bitmap' f63add4aa8 Merge branch 'jk/ref-filter-segfault-fix' And a few topics have been merged to 'next'. cdadb2c621 Merge branch 'hn/reftable-tables-doc-update' into next bbe18a7b3a Merge branch 'jk/pack-objects-bitmap-progress-fix' into next 41713a32bd Merge branch 'ah/merge-ort-ubsan-fix' into next 35fb0e853d Merge branch 'ab/userdiff-tests' into next eb80d55a8c Merge branch 'ar/userdiff-scheme' into next On 'seen' there are many topics that have not seen adequately reviews; some tests are broken near its tip (I think they pass the selftests by themselves). 0e76df05ca Merge branch 'ps/rev-list-object-type-filter' into seen 956fbceb2e ### breaks 6112 6113 c007303ad4 Merge branch 'bc/hash-transition-interop-part-1' into seen 4813f16161 ### breaks 0031 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: What's cooking (draft for #4's issue this month) 2021-04-14 1:11 What's cooking (draft for #4's issue this month) Junio C Hamano @ 2021-04-14 9:43 ` Patrick Steinhardt 2021-04-14 21:20 ` Junio C Hamano 2021-04-14 21:31 ` Junio C Hamano 2021-04-14 23:22 ` Junio C Hamano 1 sibling, 2 replies; 14+ messages in thread From: Patrick Steinhardt @ 2021-04-14 9:43 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Han-Wen Nienhuys [-- Attachment #1: Type: text/plain, Size: 4540 bytes --] On Tue, Apr 13, 2021 at 06:11:39PM -0700, Junio C Hamano wrote: > On 'seen' there are many topics that have not seen adequately > reviews; some tests are broken near its tip (I think they pass the > selftests by themselves). > > 0e76df05ca Merge branch 'ps/rev-list-object-type-filter' into seen > 956fbceb2e ### breaks 6112 6113 > c007303ad4 Merge branch 'bc/hash-transition-interop-part-1' into seen > 4813f16161 ### breaks 0031 Test breakage for the rev-list filter series has been a bad interaction of d8883ed540 (object.c: stop supporting len == -1 in type_from_string_gently(), 2021-03-28) and ps/rev-list-object-type-filter. The following patch fixes this, which I'll include in my next reroll of this series. diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c index 19e128ef11..33c7718a7a 100644 --- a/list-objects-filter-options.c +++ b/list-objects-filter-options.c @@ -100,7 +100,7 @@ static int gently_parse_list_objects_filter( return 1; } else if (skip_prefix(arg, "object:type=", &v0)) { - int type = type_from_string_gently(v0, -1); + int type = type_from_string_gently(v0, strlen(v0)); if (type < 0) { strbuf_addstr(errbuf, _("expected 'object:type=<type>'")); return 1; The other breakages I see are caused by hn/reftable, where all tests in t0031-reftables.sh cause segfaults. The root cause seems to be that reading refs via the reftable backend doesn't initialize the `algo` field of the OID, which is fixed via the following patch. diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index 130fd90e45..35fb7dd0a2 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -251,10 +251,10 @@ static int reftable_ref_iterator_advance(struct ref_iterator *ref_iterator) ri->base.flags = 0; switch (ri->ref.value_type) { case REFTABLE_REF_VAL1: - hashcpy(ri->oid.hash, ri->ref.value.val1); + oidread(&ri->oid, ri->ref.value.val1); break; case REFTABLE_REF_VAL2: - hashcpy(ri->oid.hash, ri->ref.value.val2.value); + oidread(&ri->oid, ri->ref.value.val2.value); break; case REFTABLE_REF_SYMREF: { int out_flags = 0; @@ -299,7 +299,7 @@ static int reftable_ref_iterator_peel(struct ref_iterator *ref_iterator, struct git_reftable_iterator *ri = (struct git_reftable_iterator *)ref_iterator; if (ri->ref.value_type == REFTABLE_REF_VAL2) { - hashcpy(peeled->hash, ri->ref.value.val2.target_value); + oidread(peeled, ri->ref.value.val2.target_value); return 0; } @@ -977,7 +977,7 @@ git_reftable_reflog_ref_iterator_advance(struct ref_iterator *ref_iterator) free(ri->last_name); ri->last_name = xstrdup(ri->log.refname); - hashcpy(ri->oid.hash, ri->log.update.new_hash); + oidread(&ri->oid, ri->log.update.new_hash); return ITER_OK; } } @@ -1090,8 +1090,8 @@ static int git_reftable_for_each_reflog_ent_newest_first( break; } - hashcpy(old_oid.hash, log.update.old_hash); - hashcpy(new_oid.hash, log.update.new_hash); + oidread(&old_oid, log.update.old_hash); + oidread(&new_oid, log.update.new_hash); full_committer = fmt_ident(log.update.name, log.update.email, WANT_COMMITTER_IDENT, @@ -1157,8 +1157,8 @@ static int git_reftable_for_each_reflog_ent_oldest_first( struct object_id new_oid; const char *full_committer = ""; - hashcpy(old_oid.hash, log->update.old_hash); - hashcpy(new_oid.hash, log->update.new_hash); + oidread(&old_oid, log->update.old_hash); + oidread(&new_oid, log->update.new_hash); full_committer = fmt_ident(log->update.name, log->update.email, WANT_COMMITTER_IDENT, NULL, @@ -1330,8 +1330,8 @@ git_reftable_reflog_expire(struct ref_store *ref_store, const char *refname, if (err > 0 || strcmp(log.refname, refname)) { break; } - hashcpy(ooid.hash, log.update.old_hash); - hashcpy(noid.hash, log.update.new_hash); + oidread(&ooid, log.update.old_hash); + oidread(&noid, log.update.new_hash); if (should_prune_fn(&ooid, &noid, log.update.email, (timestamp_t)log.update.time, @@ -1410,7 +1410,7 @@ static int git_reftable_read_raw_ref(struct ref_store *ref_store, strbuf_addstr(referent, ref.value.symref); *type |= REF_ISSYMREF; } else if (reftable_ref_record_val1(&ref) != NULL) { - hashcpy(oid->hash, reftable_ref_record_val1(&ref)); + oidread(oid, reftable_ref_record_val1(&ref)); } else { *type |= REF_ISBROKEN; errno = EINVAL; Patrick [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: What's cooking (draft for #4's issue this month) 2021-04-14 9:43 ` Patrick Steinhardt @ 2021-04-14 21:20 ` Junio C Hamano 2021-04-14 21:53 ` Junio C Hamano 2021-04-14 21:31 ` Junio C Hamano 1 sibling, 1 reply; 14+ messages in thread From: Junio C Hamano @ 2021-04-14 21:20 UTC (permalink / raw) To: Patrick Steinhardt, Ævar Arnfjörð Bjarmason Cc: git, Han-Wen Nienhuys Patrick Steinhardt <ps@pks.im> writes: > On Tue, Apr 13, 2021 at 06:11:39PM -0700, Junio C Hamano wrote: >> On 'seen' there are many topics that have not seen adequately >> reviews; some tests are broken near its tip (I think they pass the >> selftests by themselves). >> >> 0e76df05ca Merge branch 'ps/rev-list-object-type-filter' into seen >> 956fbceb2e ### breaks 6112 6113 >> c007303ad4 Merge branch 'bc/hash-transition-interop-part-1' into seen >> 4813f16161 ### breaks 0031 > > Test breakage for the rev-list filter series has been a bad interaction > of d8883ed540 (object.c: stop supporting len == -1 in > type_from_string_gently(), 2021-03-28) and > ps/rev-list-object-type-filter. Yuck. The commit d8883ed5 (object.c: stop supporting len == -1 in type_from_string_gently(), 2021-03-28) by itself may have meant well, but in an environment where concurrent group development is the norm, it is a change that can unnecessarily break other topics easily, like it just did. Perhaps a band-aid like this may be necessary. diff --git i/object.c w/object.c index 68de229f1a..b4bde9e444 100644 --- i/object.c +++ w/object.c @@ -39,6 +39,9 @@ int type_from_string_gently(const char *str, ssize_t len, int gentle) { int i; + if (len < 0) + BUG("type-from-string-gently no longer allows unspecified length"); + for (i = 1; i < ARRAY_SIZE(object_type_strings); i++) if (!strncmp(str, object_type_strings[i], len) && object_type_strings[i][len] == '\0') > The following patch fixes this, which > I'll include in my next reroll of this series. Thanks. > diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c > index 19e128ef11..33c7718a7a 100644 > --- a/list-objects-filter-options.c > +++ b/list-objects-filter-options.c > @@ -100,7 +100,7 @@ static int gently_parse_list_objects_filter( > return 1; > > } else if (skip_prefix(arg, "object:type=", &v0)) { > - int type = type_from_string_gently(v0, -1); > + int type = type_from_string_gently(v0, strlen(v0)); > if (type < 0) { > strbuf_addstr(errbuf, _("expected 'object:type=<type>'")); > return 1; ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: What's cooking (draft for #4's issue this month) 2021-04-14 21:20 ` Junio C Hamano @ 2021-04-14 21:53 ` Junio C Hamano 2021-04-15 9:37 ` Jeff King 0 siblings, 1 reply; 14+ messages in thread From: Junio C Hamano @ 2021-04-14 21:53 UTC (permalink / raw) To: Patrick Steinhardt Cc: Ævar Arnfjörð Bjarmason, git, Han-Wen Nienhuys Junio C Hamano <gitster@pobox.com> writes: >> The following patch fixes this, which >> I'll include in my next reroll of this series. Hmph, I am not sure if that is wise. The offending "-1 no longer is accepted" comes from a topic that is not even in 'next', so you may not want to depend on it. It turns out that the two-argument form that passes -1 to the function we see below as the preimage in your fix-up patch comes from my conflict resolution. Your original has type_from_string_gently(v0, -1, 1); and the other topic wants to make two unrelated changes to the API, i.e. making -1 no longer a valid "please count, as it is pointless to force callers to always count" option, and drops "is this asking to be gentle?" parameter. It may be better to just update what is recorded in my conflict resolution machinery, without making it type_from_string_gently(v0, strlen(v0), 1); as it would be necessary to adjust when both topics are merged anyway. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: What's cooking (draft for #4's issue this month) 2021-04-14 21:53 ` Junio C Hamano @ 2021-04-15 9:37 ` Jeff King 2021-04-15 18:02 ` Junio C Hamano 0 siblings, 1 reply; 14+ messages in thread From: Jeff King @ 2021-04-15 9:37 UTC (permalink / raw) To: Junio C Hamano Cc: Patrick Steinhardt, Ævar Arnfjörð Bjarmason, git, Han-Wen Nienhuys On Wed, Apr 14, 2021 at 02:53:19PM -0700, Junio C Hamano wrote: > Junio C Hamano <gitster@pobox.com> writes: > > >> The following patch fixes this, which > >> I'll include in my next reroll of this series. > > Hmph, I am not sure if that is wise. The offending "-1 no longer is > accepted" comes from a topic that is not even in 'next', so you may > not want to depend on it. > > It turns out that the two-argument form that passes -1 to the > function we see below as the preimage in your fix-up patch comes > from my conflict resolution. Your original has > > type_from_string_gently(v0, -1, 1); > > and the other topic wants to make two unrelated changes to the API, > i.e. making -1 no longer a valid "please count, as it is pointless > to force callers to always count" option, and drops "is this asking > to be gentle?" parameter. It may be better to just update what is > recorded in my conflict resolution machinery, without making it > > type_from_string_gently(v0, strlen(v0), 1); > > as it would be necessary to adjust when both topics are merged > anyway. Heh. I wonder if I was too optimistic in my review at: https://lore.kernel.org/git/YHCYfeRsHU34ZF%2Fl@coredump.intra.peff.net/ -Peff ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: What's cooking (draft for #4's issue this month) 2021-04-15 9:37 ` Jeff King @ 2021-04-15 18:02 ` Junio C Hamano 0 siblings, 0 replies; 14+ messages in thread From: Junio C Hamano @ 2021-04-15 18:02 UTC (permalink / raw) To: Jeff King Cc: Patrick Steinhardt, Ævar Arnfjörð Bjarmason, git, Han-Wen Nienhuys Jeff King <peff@peff.net> writes: > Heh. I wonder if I was too optimistic in my review at: > > https://lore.kernel.org/git/YHCYfeRsHU34ZF%2Fl@coredump.intra.peff.net/ Yes, merging is hard. When merging a topic X into 'seen', I can review "log -p ..MERGE_HEAD" and keep the core changes the topic wants to do in my head while reviewing the conflicting parts, but this is the other "log -p MERGE_HEAD.." direction where all the little cuts _other_ topics inflicted on the codebase before an attempted merge, and something seemingly benign and well intended change like "well nobody passes -1 to ask us to count _now_, so let's drop the feature without even bothering to check nobody does so in the future" can easily be missed. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: What's cooking (draft for #4's issue this month) 2021-04-14 9:43 ` Patrick Steinhardt 2021-04-14 21:20 ` Junio C Hamano @ 2021-04-14 21:31 ` Junio C Hamano 1 sibling, 0 replies; 14+ messages in thread From: Junio C Hamano @ 2021-04-14 21:31 UTC (permalink / raw) To: Patrick Steinhardt; +Cc: git, Han-Wen Nienhuys Patrick Steinhardt <ps@pks.im> writes: > The other breakages I see are caused by hn/reftable, where all tests in > t0031-reftables.sh cause segfaults. The root cause seems to be that > reading refs via the reftable backend doesn't initialize the `algo` > field of the OID, which is fixed via the following patch. OK. On 'master' without bc/hash-transition-interop-part-1, hashcpy() and oidread() both use the size from the_hash_algo but with the topic, oidread() becomes the way to declare that the given oid uses the_hash_algo. static inline void hashcpy(unsigned char *sha_dst, const unsigned char *sha_src) { memcpy(sha_dst, sha_src, the_hash_algo->rawsz); } static inline void oidread(struct object_id *oid, const unsigned char *hash) { size_t rawsz = the_hash_algo->rawsz; memcpy(oid->hash, hash, rawsz); memset(oid->hash + rawsz, 0, GIT_MAX_RAWSZ - rawsz); oid->algo = hash_algo_by_ptr(the_hash_algo); } So the patch makes sense to directly squashed into hn/reftable topic to (1) be a benign no-op in the master and to (2) fix an expected semantic conflict with bc/hash-transition-interop-part-1 topic. Thanks. > diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c > index 130fd90e45..35fb7dd0a2 100644 > --- a/refs/reftable-backend.c > +++ b/refs/reftable-backend.c > @@ -251,10 +251,10 @@ static int reftable_ref_iterator_advance(struct ref_iterator *ref_iterator) > ri->base.flags = 0; > switch (ri->ref.value_type) { > case REFTABLE_REF_VAL1: > - hashcpy(ri->oid.hash, ri->ref.value.val1); > + oidread(&ri->oid, ri->ref.value.val1); > break; > case REFTABLE_REF_VAL2: > - hashcpy(ri->oid.hash, ri->ref.value.val2.value); > + oidread(&ri->oid, ri->ref.value.val2.value); > break; > case REFTABLE_REF_SYMREF: { > int out_flags = 0; > @@ -299,7 +299,7 @@ static int reftable_ref_iterator_peel(struct ref_iterator *ref_iterator, > struct git_reftable_iterator *ri = > (struct git_reftable_iterator *)ref_iterator; > if (ri->ref.value_type == REFTABLE_REF_VAL2) { > - hashcpy(peeled->hash, ri->ref.value.val2.target_value); > + oidread(peeled, ri->ref.value.val2.target_value); > return 0; > } > > @@ -977,7 +977,7 @@ git_reftable_reflog_ref_iterator_advance(struct ref_iterator *ref_iterator) > > free(ri->last_name); > ri->last_name = xstrdup(ri->log.refname); > - hashcpy(ri->oid.hash, ri->log.update.new_hash); > + oidread(&ri->oid, ri->log.update.new_hash); > return ITER_OK; > } > } > @@ -1090,8 +1090,8 @@ static int git_reftable_for_each_reflog_ent_newest_first( > break; > } > > - hashcpy(old_oid.hash, log.update.old_hash); > - hashcpy(new_oid.hash, log.update.new_hash); > + oidread(&old_oid, log.update.old_hash); > + oidread(&new_oid, log.update.new_hash); > > full_committer = fmt_ident(log.update.name, log.update.email, > WANT_COMMITTER_IDENT, > @@ -1157,8 +1157,8 @@ static int git_reftable_for_each_reflog_ent_oldest_first( > struct object_id new_oid; > const char *full_committer = ""; > > - hashcpy(old_oid.hash, log->update.old_hash); > - hashcpy(new_oid.hash, log->update.new_hash); > + oidread(&old_oid, log->update.old_hash); > + oidread(&new_oid, log->update.new_hash); > > full_committer = fmt_ident(log->update.name, log->update.email, > WANT_COMMITTER_IDENT, NULL, > @@ -1330,8 +1330,8 @@ git_reftable_reflog_expire(struct ref_store *ref_store, const char *refname, > if (err > 0 || strcmp(log.refname, refname)) { > break; > } > - hashcpy(ooid.hash, log.update.old_hash); > - hashcpy(noid.hash, log.update.new_hash); > + oidread(&ooid, log.update.old_hash); > + oidread(&noid, log.update.new_hash); > > if (should_prune_fn(&ooid, &noid, log.update.email, > (timestamp_t)log.update.time, > @@ -1410,7 +1410,7 @@ static int git_reftable_read_raw_ref(struct ref_store *ref_store, > strbuf_addstr(referent, ref.value.symref); > *type |= REF_ISSYMREF; > } else if (reftable_ref_record_val1(&ref) != NULL) { > - hashcpy(oid->hash, reftable_ref_record_val1(&ref)); > + oidread(oid, reftable_ref_record_val1(&ref)); > } else { > *type |= REF_ISBROKEN; > errno = EINVAL; > > > Patrick ^ permalink raw reply [flat|nested] 14+ messages in thread
* What's cooking (draft for #4's issue this month) 2021-04-14 1:11 What's cooking (draft for #4's issue this month) Junio C Hamano 2021-04-14 9:43 ` Patrick Steinhardt @ 2021-04-14 23:22 ` Junio C Hamano 2021-04-14 23:26 ` Junio C Hamano ` (2 more replies) 1 sibling, 3 replies; 14+ messages in thread From: Junio C Hamano @ 2021-04-14 23:22 UTC (permalink / raw) To: git, Han-Wen Nienhuys, Ævar Arnfjörð Bjarmason, brian m. carlson, Patrick Steinhardt Here is the (local) test status near the tip of 'seen', relative to the integration result last night. * hn/reftable has a preparatory change to use oidread() instead of hashcpy() in places queued at its tip. This is essentially a no-op in the codebase without bc/hash-transition-interop-part-1 and would be a bugfix with that topic. Please squash it into an appropriate step in the series when updating the topic in the future. * ab/unexpected-object-type topic has an assertion to catch semantic conflicts with topics in-flight queued at its tip. It would probably be safe to carry it until the topioc is merged to 'master' and then remove it after the dust settles. Please squash it into an appropriate step in the series when updating the topic in the future. * The tip of 'seen' passes all the tests locally, except that t5540 fails when compiled with CC=clang (http-push exits with signal 11). bc/hash-transition-interop-part-1, which is at the tip of 'seen', seems to have this issue standalone. FYI, here is what "clang --version" gives me: Debian clang version 11.0.1-2 Target: x86_64-pc-linux-gnu Thread model: posix InstalledDir: /usr/bin Credits for the corrections cited above goes to Patrick. The attached is a summary of various topics. Their status may be a bit stale (I haven't re-reviewed them with respect to the latest discussion on the list). Thanks. ----- >8 ---------- >8 ---------- >8 ---------- >8 ---------- >8 ----- Expecting a (hopefully final) reroll. - ag/merge-strategies-in-c 03-17 #15 Expecting a reroll. - sg/bugreport-fixes 04-08 #1 - ds/maintenance-prefetch-fix 04-10 #3 - ma/t0091-bugreport-fix 04-12 #1 On hold, waiting for the "protections" topic to stablize. + ds/sparse-index 03-30/04-07 #21 On hold. Probably needs either a reroll or incremental refinements. + dl/complete-stash 03-24/03-24 #3 Stalled - es/config-hooks 03-10 #36 - ab/describe-tests-fix 04-12 #5 - ab/pickaxe-pcre2 04-12 #22 - ab/unexpected-object-type 04-14 #11 Undecided - mt/add-rm-in-sparse-checkout 04-08 #7 - jh/rfc-builtin-fsmonitor 04-08 #23 - mt/parallel-checkout-part-2 04-08 #5 - bc/hash-transition-interop-part-1 04-10 #16 - mr/bisect-in-c-4 04-11 #4 - ps/rev-list-object-type-filter 04-12 #8 - ab/fsck-unexpected-type 04-13 #6 - rs/repack-without-loosening-promised-objects 04-14 #2 - ds/sparse-index-protections 04-14 #26 Waiting for reroll. - ah/plugleaks 04-10 #9 Waiting for reviews to conclude. - ab/doc-lint 04-10 #7 - ab/rebase-no-reschedule-failed-exec 04-10 #2 - hn/refs-trace-errno 04-12 #1 Waiting for reviews. - jt/push-negotiation 04-08 #6 - tb/multi-pack-bitmaps 04-10 #23 - ab/svn-tests-set-e-fix 04-12 #2 - ab/test-lib-updates 04-12 #16 - ao/p4-avoid-decoding 04-12 #2 - zh/trailer-cmd 04-12 #2 - hn/reftable 04-14 #21 Will merge to 'master'. + en/ort-perf-batch-10 03-18/04-07 #8 + en/ort-readiness 03-20/04-08 #13 + jz/apply-run-3way-first 04-06/04-08 #1 + ab/complete-cherry-pick-head 04-07/04-09 #1 + jz/apply-3way-cached 04-07/04-09 #1 + ab/userdiff-tests 04-08/04-13 #9 + ar/userdiff-scheme 04-08/04-13 #1 + ah/merge-ort-ubsan-fix 04-12/04-13 #1 + hn/reftable-tables-doc-update 04-12/04-13 #1 + jk/pack-objects-bitmap-progress-fix 04-12/04-13 #1 Will merge to 'next'. - jt/fetch-pack-request-fix 04-08 #1 - ab/detox-gettext-tests 04-13 #1 - ab/usage-error-docs 04-13 #3 - jk/promisor-optim 04-13 #3 - so/log-diff-merge 04-13 #5 - ps/config-global-override 04-13 #3 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: What's cooking (draft for #4's issue this month) 2021-04-14 23:22 ` Junio C Hamano @ 2021-04-14 23:26 ` Junio C Hamano 2021-04-15 0:34 ` brian m. carlson 2021-04-15 12:58 ` Han-Wen Nienhuys 2 siblings, 0 replies; 14+ messages in thread From: Junio C Hamano @ 2021-04-14 23:26 UTC (permalink / raw) To: git; +Cc: brian m. carlson Junio C Hamano <gitster@pobox.com> writes: > * The tip of 'seen' passes all the tests locally, except that t5540 > fails when compiled with CC=clang (http-push exits with signal > 11). bc/hash-transition-interop-part-1, which is at the tip of > 'seen', seems to have this issue standalone. BTW https://github.com/git/git/runs/2347905320?check_suite_focus=true#step:5:203 shows this. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: What's cooking (draft for #4's issue this month) 2021-04-14 23:22 ` Junio C Hamano 2021-04-14 23:26 ` Junio C Hamano @ 2021-04-15 0:34 ` brian m. carlson 2021-04-15 6:37 ` Junio C Hamano 2021-04-15 12:58 ` Han-Wen Nienhuys 2 siblings, 1 reply; 14+ messages in thread From: brian m. carlson @ 2021-04-15 0:34 UTC (permalink / raw) To: Junio C Hamano Cc: git, Han-Wen Nienhuys, Ævar Arnfjörð Bjarmason, Patrick Steinhardt [-- Attachment #1: Type: text/plain, Size: 1553 bytes --] On 2021-04-14 at 23:22:19, Junio C Hamano wrote: > Here is the (local) test status near the tip of 'seen', relative to > the integration result last night. > > * hn/reftable has a preparatory change to use oidread() instead of > hashcpy() in places queued at its tip. This is essentially a > no-op in the codebase without bc/hash-transition-interop-part-1 > and would be a bugfix with that topic. Please squash it into an > appropriate step in the series when updating the topic in the > future. > > * ab/unexpected-object-type topic has an assertion to catch > semantic conflicts with topics in-flight queued at its tip. It > would probably be safe to carry it until the topioc is merged to > 'master' and then remove it after the dust settles. Please > squash it into an appropriate step in the series when updating > the topic in the future. > > * The tip of 'seen' passes all the tests locally, except that t5540 > fails when compiled with CC=clang (http-push exits with signal > 11). bc/hash-transition-interop-part-1, which is at the tip of > 'seen', seems to have this issue standalone. FYI, here is what > "clang --version" gives me: > > Debian clang version 11.0.1-2 > Target: x86_64-pc-linux-gnu > Thread model: posix > InstalledDir: /usr/bin You should expect a reroll, so feel free to drop this if it breaks things for now and I'll figure out where things are going wrong. -- brian m. carlson (he/him or they/them) Houston, Texas, US [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 263 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: What's cooking (draft for #4's issue this month) 2021-04-15 0:34 ` brian m. carlson @ 2021-04-15 6:37 ` Junio C Hamano 2021-04-19 2:10 ` brian m. carlson 0 siblings, 1 reply; 14+ messages in thread From: Junio C Hamano @ 2021-04-15 6:37 UTC (permalink / raw) To: brian m. carlson Cc: git, Han-Wen Nienhuys, Ævar Arnfjörð Bjarmason, Patrick Steinhardt "brian m. carlson" <sandals@crustytoothpaste.net> writes: > On 2021-04-14 at 23:22:19, Junio C Hamano wrote: >> Here is the (local) test status near the tip of 'seen', relative to >> the integration result last night. >> >> * hn/reftable has a preparatory change to use oidread() instead of >> hashcpy() in places queued at its tip. This is essentially a >> no-op in the codebase without bc/hash-transition-interop-part-1 >> and would be a bugfix with that topic. Please squash it into an >> appropriate step in the series when updating the topic in the >> future. >> >> * ab/unexpected-object-type topic has an assertion to catch >> semantic conflicts with topics in-flight queued at its tip. It >> would probably be safe to carry it until the topioc is merged to >> 'master' and then remove it after the dust settles. Please >> squash it into an appropriate step in the series when updating >> the topic in the future. >> >> * The tip of 'seen' passes all the tests locally, except that t5540 >> fails when compiled with CC=clang (http-push exits with signal >> 11). bc/hash-transition-interop-part-1, which is at the tip of >> 'seen', seems to have this issue standalone. FYI, here is what >> "clang --version" gives me: >> >> Debian clang version 11.0.1-2 >> Target: x86_64-pc-linux-gnu >> Thread model: posix >> InstalledDir: /usr/bin > > You should expect a reroll, so feel free to drop this if it breaks > things for now and I'll figure out where things are going wrong. I actually do appreciate the topic to be in 'seen', as these integration exercises tend to serve as an early warning for impending messy conflicts I'll need to be worried about. I do worry about the memory requirement bloat of the object_id structure, as we do need to keep one instance per object in-core, but the squashable fix for the reftable topic given by Patrick to replace use of hashcpy() with oidread() is still a good idea even if we are going to use a different mechanism to keep track of which object_id instance uses what hash algorithm, so again I am happy to have seen your bc/hash-transition-interop-part-1 topic and had an early chance to make it collide with others ;-) Thanks. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: What's cooking (draft for #4's issue this month) 2021-04-15 6:37 ` Junio C Hamano @ 2021-04-19 2:10 ` brian m. carlson 2021-04-19 23:14 ` Junio C Hamano 0 siblings, 1 reply; 14+ messages in thread From: brian m. carlson @ 2021-04-19 2:10 UTC (permalink / raw) To: Junio C Hamano Cc: git, Han-Wen Nienhuys, Ævar Arnfjörð Bjarmason, Patrick Steinhardt [-- Attachment #1: Type: text/plain, Size: 2419 bytes --] On 2021-04-15 at 06:37:11, Junio C Hamano wrote: > I actually do appreciate the topic to be in 'seen', as these > integration exercises tend to serve as an early warning for > impending messy conflicts I'll need to be worried about. > > I do worry about the memory requirement bloat of the object_id > structure, as we do need to keep one instance per object in-core, > but the squashable fix for the reftable topic given by Patrick > to replace use of hashcpy() with oidread() is still a good idea even > if we are going to use a different mechanism to keep track of which > object_id instance uses what hash algorithm, so again I am happy to > have seen your bc/hash-transition-interop-part-1 topic and had an > early chance to make it collide with others ;-) I'm still working on a full reroll for the series including performance measurements (since this took me much longer than I expected it would), but I wanted to include a patch for the segfault below to keep things tidy in the mean time. I should point out that this doesn't appear to crash when running the testsuite in SHA-256 mode for reasons I'm not sure about, which explains why I didn't see it originally. ---- %< ---- From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: "brian m. carlson" <sandals@crustytoothpaste.net> Date: Sun, 18 Apr 2021 22:56:04 +0000 Subject: [PATCH] http-push: set algorithm when reading object ID In most places in the codebase, we use oidread to properly read an object ID into a struct object_id. However, in the HTTP code, we end up needing to parse a loose object path with a slash in it, so we can't do that. Let's instead explicitly set the algorithm in this function so we can rely on it in the future. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> --- http-push.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/http-push.c b/http-push.c index b60d5fcc85..5675cd7708 100644 --- a/http-push.c +++ b/http-push.c @@ -1022,6 +1022,8 @@ static void remote_ls(const char *path, int flags, /* extract hex from sharded "xx/x{38}" filename */ static int get_oid_hex_from_objpath(const char *path, struct object_id *oid) { + oid->algo = hash_algo_by_ptr(the_hash_algo); + if (strlen(path) != the_hash_algo->hexsz + 1) return -1; ---- %< ---- -- brian m. carlson (he/him or they/them) Houston, Texas, US [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 263 bytes --] ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: What's cooking (draft for #4's issue this month) 2021-04-19 2:10 ` brian m. carlson @ 2021-04-19 23:14 ` Junio C Hamano 0 siblings, 0 replies; 14+ messages in thread From: Junio C Hamano @ 2021-04-19 23:14 UTC (permalink / raw) To: brian m. carlson Cc: git, Han-Wen Nienhuys, Ævar Arnfjörð Bjarmason, Patrick Steinhardt "brian m. carlson" <sandals@crustytoothpaste.net> writes: > I'm still working on a full reroll for the series including performance > measurements (since this took me much longer than I expected it would), > but I wanted to include a patch for the segfault below to keep things > tidy in the mean time. I should point out that this doesn't appear to > crash when running the testsuite in SHA-256 mode for reasons I'm not > sure about, which explains why I didn't see it originally. Thanks; will apply and keep it with the latest round for now. > > ---- %< ---- > From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 > From: "brian m. carlson" <sandals@crustytoothpaste.net> > Date: Sun, 18 Apr 2021 22:56:04 +0000 > Subject: [PATCH] http-push: set algorithm when reading object ID > > In most places in the codebase, we use oidread to properly read an > object ID into a struct object_id. However, in the HTTP code, we end up > needing to parse a loose object path with a slash in it, so we can't do > that. Let's instead explicitly set the algorithm in this function so we > can rely on it in the future. > > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> > --- > http-push.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/http-push.c b/http-push.c > index b60d5fcc85..5675cd7708 100644 > --- a/http-push.c > +++ b/http-push.c > @@ -1022,6 +1022,8 @@ static void remote_ls(const char *path, int flags, > /* extract hex from sharded "xx/x{38}" filename */ > static int get_oid_hex_from_objpath(const char *path, struct object_id *oid) > { > + oid->algo = hash_algo_by_ptr(the_hash_algo); > + > if (strlen(path) != the_hash_algo->hexsz + 1) > return -1; > > ---- %< ---- ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: What's cooking (draft for #4's issue this month) 2021-04-14 23:22 ` Junio C Hamano 2021-04-14 23:26 ` Junio C Hamano 2021-04-15 0:34 ` brian m. carlson @ 2021-04-15 12:58 ` Han-Wen Nienhuys 2 siblings, 0 replies; 14+ messages in thread From: Han-Wen Nienhuys @ 2021-04-15 12:58 UTC (permalink / raw) To: Junio C Hamano Cc: git, Ævar Arnfjörð Bjarmason, brian m. carlson, Patrick Steinhardt On Thu, Apr 15, 2021 at 1:22 AM Junio C Hamano <gitster@pobox.com> wrote: > * hn/reftable has a preparatory change to use oidread() instead of > hashcpy() in places queued at its tip. This is essentially a > no-op in the codebase without bc/hash-transition-interop-part-1 > and would be a bugfix with that topic. Please squash it into an > appropriate step in the series when updating the topic in the > future. Done. Thanks for the fix. -- Han-Wen Nienhuys - Google Munich I work 80%. Don't expect answers from me on Fridays. -- Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Paul Manicle, Halimah DeLaine Prado ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-04-19 23:14 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-04-14 1:11 What's cooking (draft for #4's issue this month) Junio C Hamano 2021-04-14 9:43 ` Patrick Steinhardt 2021-04-14 21:20 ` Junio C Hamano 2021-04-14 21:53 ` Junio C Hamano 2021-04-15 9:37 ` Jeff King 2021-04-15 18:02 ` Junio C Hamano 2021-04-14 21:31 ` Junio C Hamano 2021-04-14 23:22 ` Junio C Hamano 2021-04-14 23:26 ` Junio C Hamano 2021-04-15 0:34 ` brian m. carlson 2021-04-15 6:37 ` Junio C Hamano 2021-04-19 2:10 ` brian m. carlson 2021-04-19 23:14 ` Junio C Hamano 2021-04-15 12:58 ` Han-Wen Nienhuys
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.