Fix CI #641

tcharding · 2023-08-08T02:03:21Z

CI has a bunch of things broken.

This is #640 followed by #639 followed by a few addition fixes to get CI green again. Includes clippy warnings in feature gated code which is not strictly necessary and a fix to a panic message, also not strictly necessary.

tcharding · 2023-08-08T02:55:32Z

I can't repro the ASAN memory sanitizer failure and have no clue what is causing it.

apoelstra · 2023-08-08T16:50:09Z

The ASAN thing is extremely weird. "Undefined reference to memmove" inside libcore itself. Let's wait a few days for a new nightly and see if it goes away.

tcharding · 2023-08-08T23:14:12Z

After writing rust-bitcoin/rust-bitcoin#1981 there is a lot to do and we need this in to get cracking.

Kick CI, no changes.

apoelstra · 2023-08-08T23:44:35Z

When I try to run this test locally I get the error error: the feature lang_items is internal to the compiler or standard library. AFAICT this error showed up months ago, so I don't know why it's not happening in CI (even though CI is using the same nightly compiler as me). If we remove feature(lang_items) from the nostd test (I don't know why it's even there) then things work.

tcharding · 2023-08-08T23:51:21Z

Strange you get the error still, it is allowed now (in a1005a6) I looked it up but didn't quite understand what it was for. I'll remove it.

tcharding · 2023-08-08T23:54:15Z

Removed, no other changes.

apoelstra · 2023-08-09T14:10:21Z

Ok I can reproduce locally now :)

apoelstra · 2023-08-09T14:24:19Z

Strange you get the error still, it is allowed now (in a1005a6) I looked it up but didn't quite understand what it was for. I'll remove it.

I don't know what that commit is from. I don't have it locally.

apoelstra · 2023-08-09T14:55:11Z

This is extremely weird. In my mastert/ directory things pass while in my pr-review/ directory they don't. Even running the exact same commit in both, even running cargo clean in both, even moving boths' target/ directories out of the way.

apoelstra · 2023-08-09T15:11:24Z

Ok, it's a problem in the cc crate that appeared between 1.79 (six months old) and 1.80 (first of several releases from the last week).

apoelstra · 2023-08-09T15:26:31Z

The old cc would provide -lc -lm -lrt -lpthread flags which the new one does not. I think -lc is the important one.

apoelstra · 2023-08-09T16:01:52Z

rust-lang/cc-rs#780 is the offending PR, though I haven't looked into how yet.

apoelstra · 2023-08-09T16:12:47Z

@tcharding let's just pin cc in CI to 1.0.79 for now.

Pinning is broken again, update the pins it CI so that the following sequence of commands would work ```bash rm Cargo.lock cargo +1.48 update -p wasm-bindgen-test --precise 0.3.34 cargo +1.48 update -p serde_test --precise 1.0.175 cargo +1.48 test --all-features ``` Note, solely out of interest, `cargo +1.48 build` does not need pinning (at the moment :)

`cargo +nightly` output of panic recently changed breaking our grep statement by adding the code line and a newline. Grep for the exact second line of the error message.

Remove the `internal_features` attribute, not sure what it was supposed to be doing but the crate works without it.

We have an unconditional panic for some combination of features, this causes clippy to give a bunch of useless warnings. Add allow attributes to quieten down clippy.

Currently the panic message refers to stuff related to development of the library, this is meaningless for users of the lib. Target panic message at secp users instead.

In preparation for using `pushd`/`popd` use `bash` to run the CI script instead of `sh`.

Looks like a recent version of `cc` breaks our ASAN job. Pin to the previous version.

tcharding · 2023-08-10T00:15:52Z

Took me a while, but oh yeah it works!

apoelstra · 2023-08-10T14:57:54Z

Ok, testing locally, thanks so much!

BTW I think the lang_items stuff was obsoleted by #205. I also think the doccomment in no_std which talks about eh_personality is also out of date since that PR. But we can fix it in a followup. Basically, what happened was:

We originally were redefining internal std stuff as a way to trigger errors in the case that we accidentally pulled std into a nostd build.
The compiler took away the internal std stuff or otherwise somehow broke this, so elichai removed the redefinitions but didn't update the comment explaining what we were doing. He also didn't remove feature(lang_items) which became unnecessary.
The compiler later made feature(lang_items) deny-by-default to warn us about the kind of breakage in point (2).

It would be nice to test somehow the "we aren't pulling in std", but I think this is a weird enough failure mode that we shouldn't bend over backward to do it.

apoelstra

ACK 92778ef

92778efe926b673f04e2935af50314db99cf8244 CI: Pin cc in ASAN no_std_test crate (Tobin C. Harding) 5e6dd8a46783c8ab0aaf6a057cf6942ffc72fb2a CI: Use bash instead of sh (Tobin C. Harding) cf5f1034cad0fbeae5e995442f7fd80e8eaa3edf Target panic message at lib users (Tobin C. Harding) ec9c9643d794b7c5b2c4bbe4aeee8b70bf46ff1f Allow stuff after unconditional panic (Tobin C. Harding) 3bbf08348e9b11427ac8f1d13f1a975b3feee7ff no_std_test: Remove internal_features (Tobin C. Harding) cff7a3dae79ab416a1de106c5736ae1bfb69503f CI: Grep for exact error message (Tobin C. Harding) 79e184f08af0aff0df87b911b4aae61d474e962b CI: Update MSRV pins (Tobin C. Harding) Pull request description: CI has a bunch of things broken. This is #640 followed by #639 followed by a few addition fixes to get CI green again. Includes clippy warnings in feature gated code which is not strictly necessary and a fix to a panic message, also not strictly necessary. ACKs for top commit: apoelstra: ACK 92778efe926b673f04e2935af50314db99cf8244 Tree-SHA512: f99b01e17fade7df394299bdb6bf385bec3f88d6568d43962238049b33a94c364d48c266acb358e72a48dd55a4aac6300ace6478b0821275b89cb86eba639d8b

This was referenced Aug 8, 2023

CI: Update pins #640

Closed

CI: Grep for exact error message #639

Closed

Introduce new error handling convention #635

Draft

tcharding force-pushed the 08-08-ci branch from 5ad5ed9 to 24c88fe Compare August 8, 2023 02:31

tcharding force-pushed the 08-08-ci branch from 24c88fe to d395e95 Compare August 8, 2023 23:14

tcharding force-pushed the 08-08-ci branch from d395e95 to 253ebe8 Compare August 8, 2023 23:54

This was referenced Aug 8, 2023

CI: test with commited lock files #632

Merged

Clean up the hashes dependency/feature #628

Merged

Accidentally closed: ~Release v0.28.0~ #621

Closed

tcharding force-pushed the 08-08-ci branch from 253ebe8 to 6248570 Compare August 9, 2023 22:43

tcharding force-pushed the 08-08-ci branch from 6248570 to 5a1840c Compare August 9, 2023 22:53

tcharding added 4 commits August 10, 2023 09:16

CI: Grep for exact error message

cff7a3d

`cargo +nightly` output of panic recently changed breaking our grep statement by adding the code line and a newline. Grep for the exact second line of the error message.

no_std_test: Remove internal_features

3bbf083

Remove the `internal_features` attribute, not sure what it was supposed to be doing but the crate works without it.

Allow stuff after unconditional panic

ec9c964

We have an unconditional panic for some combination of features, this causes clippy to give a bunch of useless warnings. Add allow attributes to quieten down clippy.

Target panic message at lib users

cf5f103

Currently the panic message refers to stuff related to development of the library, this is meaningless for users of the lib. Target panic message at secp users instead.

tcharding force-pushed the 08-08-ci branch from 5a1840c to e418ee9 Compare August 9, 2023 23:17

CI: Use bash instead of sh

5e6dd8a

In preparation for using `pushd`/`popd` use `bash` to run the CI script instead of `sh`.

tcharding force-pushed the 08-08-ci branch from e418ee9 to e50f06b Compare August 9, 2023 23:48

CI: Pin cc in ASAN no_std_test crate

92778ef

Looks like a recent version of `cc` breaks our ASAN job. Pin to the previous version.

tcharding force-pushed the 08-08-ci branch from e50f06b to 92778ef Compare August 9, 2023 23:54

apoelstra approved these changes Aug 10, 2023

View reviewed changes

apoelstra merged commit 090f073 into rust-bitcoin:master Aug 10, 2023

tcharding deleted the 08-08-ci branch August 10, 2023 23:30

Fix CI #641

Fix CI #641

Uh oh!

Conversation

tcharding commented Aug 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tcharding commented Aug 8, 2023

Uh oh!

apoelstra commented Aug 8, 2023

Uh oh!

tcharding commented Aug 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

apoelstra commented Aug 8, 2023

Uh oh!

tcharding commented Aug 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tcharding commented Aug 8, 2023

Uh oh!

apoelstra commented Aug 9, 2023

Uh oh!

apoelstra commented Aug 9, 2023

Uh oh!

apoelstra commented Aug 9, 2023

Uh oh!

apoelstra commented Aug 9, 2023

Uh oh!

apoelstra commented Aug 9, 2023

Uh oh!

apoelstra commented Aug 9, 2023

Uh oh!

apoelstra commented Aug 9, 2023

Uh oh!

tcharding commented Aug 10, 2023

Uh oh!

apoelstra commented Aug 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

apoelstra left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tcharding commented Aug 8, 2023 •

edited

Loading

tcharding commented Aug 8, 2023 •

edited

Loading

tcharding commented Aug 8, 2023 •

edited

Loading

apoelstra commented Aug 10, 2023 •

edited

Loading