Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump MSRV to 1.48 and replace most of CI with a bash script #2107

Merged
merged 6 commits into from
Mar 17, 2023

Conversation

TheBlueMatt
Copy link
Collaborator

No description provided.

1.48.0 was released at the end of 2020, nearly 2.5 years ago. It
has been the rustc available on Debian stable since bullseye,
released in 2021. supporting Debian oldstable for more than a year
seems more than sufficient time to give Debian folks to upgrade,
and bullseye is set to become `oldstable` later this year with the
release of `bookworm`, likely this summer.

This also allows us to clean up our MSRV substantially, having a
single MSRV across our crates rather than a number of separate
ones. Sadly, windows already requires 1.49.
@codecov-commenter
Copy link

codecov-commenter commented Mar 15, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.49 🎉

Comparison is base (2d213a4) 91.15% compared to head (6558b51) 91.65%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2107      +/-   ##
==========================================
+ Coverage   91.15%   91.65%   +0.49%     
==========================================
  Files         101      101              
  Lines       48866    50766    +1900     
  Branches    48866    50766    +1900     
==========================================
+ Hits        44544    46529    +1985     
+ Misses       4322     4237      -85     
Impacted Files Coverage Δ
lightning/src/onion_message/functional_tests.rs 94.92% <100.00%> (-0.04%) ⬇️

... and 36 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@tnull tnull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Will see if we can eventually get testing MSRV of lightning-transaction-sync also down to 1.48.

I ran shellcheck on the bash script and it yielded some errors and warnings. The following diff fixes them for me:

diff --git a/ci/ci-tests.sh b/ci/ci-tests.sh
index 03750dd6..3b686e38 100755
--- a/ci/ci-tests.sh
+++ b/ci/ci-tests.sh
@@ -5,7 +5,7 @@ RUSTC_MINOR_VERSION=$(rustc --version | awk '{ split($2,a,"."); print a[2] }')
 HOST_PLATFORM="$(rustc --version --verbose | grep "host:" | awk '{ print $2 }')"

 # Tokio MSRV on versions newer than 1.14 is rustc 1.49
-[ $RUSTC_MINOR_VERSION -lt 49 ] && cargo update -p tokio --precise "1.14.0" --verbose
+[ "$RUSTC_MINOR_VERSION" -lt 49 ] && cargo update -p tokio --precise "1.14.0" --verbose
 [ "$LDK_COVERAGE_BUILD" != "" ] && export RUSTFLAGS="-C link-dead-code"

 export RUST_BACKTRACE=1
@@ -16,12 +16,14 @@ cargo test --verbose --color always

 echo -e "\n\nBuilding with all Log-Limiting features"
 pushd lightning
-for FEATURE in $(cat Cargo.toml | grep '^max_level_' | awk '{ print $1 }'); do
-       cargo build --verbose --color always --features $FEATURE
+
+grep '^max_level_' < Cargo.toml | awk '{ print $1 }' | while IFS= read -r FEATURE
+do
+       cargo build --verbose --color always --features "$FEATURE"
 done
 popd

-if [ $RUSTC_MINOR_VERSION -gt 51 ]; then # Current `object` MSRV, subject to change
+if [ "$RUSTC_MINOR_VERSION" -gt 51 ]; then # Current `object` MSRV, subject to change
        echo -e "\n\nTest backtrace-debug builds"
        pushd lightning
        cargo test --verbose --color always --features backtrace
@@ -66,7 +68,7 @@ cargo build --verbose --color always --features rpc-client,rest-client,tokio
 cargo test --verbose --color always --features rpc-client,rest-client,tokio
 popd

-if [ $RUSTC_MINOR_VERSION -gt 67 -a [[ "$HOST_PLATFORM" != *windows* ]] ]; then
+if [[ ( "$RUSTC_MINOR_VERSION" -gt 67 ) && ( "$HOST_PLATFORM" != *windows* ) ]]; then
        echo -e "\n\nBuilding and testing Transaction Sync Clients with features"
        pushd lightning-transaction-sync
        cargo build --verbose --color always --features esplora-blocking
@@ -83,7 +85,7 @@ pushd lightning-background-processor
 cargo test --verbose --color always --no-default-features --features futures
 popd

-if [ $RUSTC_MINOR_VERSION -gt 55 ]; then
+if [ "$RUSTC_MINOR_VERSION" -gt 55 ]; then
        echo -e "\n\nTest Custom Message Macros"
        pushd lightning-custom-message
        cargo test --verbose --color always

ci/ci-tests.sh Outdated Show resolved Hide resolved
@TheBlueMatt
Copy link
Collaborator Author

Fixed the shellcheck warnings, it gets a bit excited with useless warnings, but better than not, so added that to CI as well.

Copy link
Contributor

@tnull tnull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI is unhappy due to sudo not being available on Windows and no apt on macos. LGTM otherwise though.

@TheBlueMatt
Copy link
Collaborator Author

Oops, lol, duh.

@wpaulino
Copy link
Contributor

Somehow CI is still showing some previous build checks as pending...

@TheBlueMatt
Copy link
Collaborator Author

Yea, the "pending" jobs were just old jobs we were requiring, I've removed them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants