* 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 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
* 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
* 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-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-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
* 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-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
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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).