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.

Sign up for GitHub

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.56.1 #693

Merged
merged 2 commits into from
Mar 28, 2024
Merged

Bump MSRV to 1.56.1 #693

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a2b78f4
Bump MSRV to 1.56.1
tcharding Mar 26, 2024
2d0c783
Tighten the version grep in vendor script
tcharding Mar 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion secp256k1-sys/vendor-libsecp.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ else
SECP_VENDOR_GIT_ROOT="$(realpath "$SECP_VENDOR_GIT_ROOT")"
fi
SECP_SYS="$SECP_VENDOR_GIT_ROOT"/secp256k1-sys
DEFAULT_VERSION_CODE=$(grep version "$SECP_SYS/Cargo.toml" | sed 's/\./_/g' | sed 's/.*"\(.*\)".*/\1/')
DEFAULT_VERSION_CODE=$(grep "^version" "$SECP_SYS/Cargo.toml" | sed 's/\./_/g' | sed 's/.*"\(.*\)".*/\1/')
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is also wrong - toml allows leading whitespaces.

Just kidding. I mean it is strictly wrong but nobody cares.

Copy link
Member

Choose a reason for hiding this comment

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

Would be nice if there were a tool like jq but for TOML.

We could try using Nix, which has a fromTOML builtin function, but I'd guess that most people do not want to maintain complicated logic in the Nix language, which combines laziness and dynamic typing in a truly impossible-to-read-or-debug way.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I just realized, would it make sense to rewrite our scripts in Nushell? It can convert Toml and a bunch of other formats to its own structured data representation. So this line would be just let version = (open Cargo.toml | get package.version). Also it handles structured data and error much better than bash but it's not as annoying when launching external programs as Python is. Though one big downside is we'd need to keep tests which we want contributors to run in bash to not annoy them too much.

I tried it out a long time ago (and even contributed a bit) so don't remember it that much but I'd love to learn it properly. I also have a nushell script wrapping lncli and converting string-encoded integers to normal integers (yes, they do that 🤦 ) if anyone is interested and I wanted to have a bitcoin-cli wrapper too. (IDK if it encodes ints as strings but at least outputting structured data would be nice.)

Copy link
Member

Choose a reason for hiding this comment

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

Sounds very interesting. I think I'd probably go Nix first, despite its shortcomings as a language, since it's a litte more mainstream (at least, within this project it is much more well-known). In Nix it's easy to hand off to bash and share code with bash.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe Nushell is more appealing to learn than Nix based on your complaints? :) I find Nushell very readable. It does have dynamic typing but it's at least strong typing and some commands know their types upfront IIRC so they can report error messages nicely.

DEFAULT_DEPEND_DIR="$SECP_SYS/depend"
DEFAULT_SECP_REPO=https://github.com/bitcoin-core/secp256k1.git

Expand Down
Loading