Make MSRV test green again

[elsirion]

The MSRV test on master was failing for a while. Since MSRV is the reason to keep unicode-normalization pinned, I fixed it to demonstrate that even without pinning downstream consumers can maintain the current MSRV.

So this PR is also an alternative to #85 and #28. The approach taken here is making the downstream consumer responsible for pinning to old versions if they require the MSRV support. After talking to @TheBlueMatt this seems to be an idiomatic way to go about it in the rust-bitcoin ecosystem.

Currently failing MSRV test on master:

I tested locally with a `rust 1.41.1` nix dev shell

{
  description = "Rust devShell with MSRV 1.41.1 using Crane and oxalica/rust-overlay";

  inputs = {
    nixpkgs.url = "github:NixOS/nixpkgs/nixpkgs-unstable";
    flake-utils.url = "github:numtide/flake-utils";
    crane.url = "github:ipetkov/crane";
    rust-overlay = {
      url = "github:oxalica/rust-overlay";
      inputs.nixpkgs.follows = "nixpkgs";
    };
  };

  outputs = { self, nixpkgs, flake-utils, crane, rust-overlay, ... }:
    flake-utils.lib.eachDefaultSystem (system:
      let
        overlays = [ (import rust-overlay) ];
        pkgs = import nixpkgs { inherit system overlays; };
        rustToolchain = pkgs.rust-bin.stable."1.41.1".default.override {
          extensions = [ "clippy" "rustfmt" ];
        };
        craneLib = (crane.mkLib pkgs).overrideToolchain rustToolchain;
      in {
        devShells.default = craneLib.devShell {
          # Add extra dev tools here if needed, e.g.:
          # packages = [ pkgs.ripgrep pkgs.cargo-audit ];
        };
      }
    );
}

[dpc]

Why not use and put Cargo.lock under revision control?

[elsirion]

It's a library, a Cargo.lock would do more harm than good imo since you wouldn't know how you got to it, i.e. which dependencies need pinning. This way, every time a upstream dependency breaks MSRV CI will fail and it can be tracked as "needs pinning". The goal isn't to have the perfect Cargo.lock in this repo, but to make it easy for consumers to get their Cargo.lock in shape.

[dpc]

If some downstream project needs pinning, they probably will not even know that they should look for pinning instruction in some dependency between what failed and their own code (and which one exactly). And figuring it out failing pinning manually is not a rocket science, probably faster than checking transitive deps for help.

I don't know where this idea that bip39 is somehow MSRV coordinator of rust-bitcoin ecosystem. If anything it's own responsibility should be just ensuring it compiles at the minimum desired MSRV, and that's about it. Cargo.lock would achieve that.

[elsirion]

I don't mind too much either way, but if you have a weird MSRV you probably have to add every dependency very carefully anyway and the deps telling you which of their transitive dependencies are problematic is useful. I was tinkering around for quite a bit till I got it to work.

Note that even before at least bitcoin_hashes was already being pinned like that, so I'm not introducing this as a new concept but rather extending it to the crates that became problematic since the test last worked.

Maybe @apoelstra or @tcharding have an opinion on this one.

[apoelstra]

Agreed. It's true that downstream projects will have a hard time finding instructions in the READMEs of their deps, but they'll have an even harder time finding anything in the Cargo.lock files of their deps.

It's probably useful to commit a Cargo-minimal.lock file here for use in CI the way we do in rust-bitcoin, but IMO committing a file called Cargo.lock would cause more confusion than help.

[dpc]

IMO committing a file called Cargo.lock would cause more confusion than help.

I have Cargo.lock checked in all projects, apps and libraries alike. Nothing confusing about its existence itself. It is ignored in downstream projects specifically so developers of libraries have a liberty of using it for whatever they want. But what exactly is locked is entirely up to developers, and typically means just "these are the deps versions that developers used/tested so probably work". By itself it does not solve e.g. checking if project builds with with minimum dependency version and/or latest deps versions, etc. So Cargo-minimal.lock makes sense.

[apoelstra]

Yeah, that's fair. So I'd be fine if there were a committed Cargo.lock file, and I suppose it ought to be one that works with the project's MSRV.

[elsirion]

I added a Cargo-minimal.lock and a test that it's still correct for MSRV.

[tcharding]

ACK 7d1c0cf

[elsirion]

@tcharding do you think we could cut a patch release for this? It would unblock us on the Fedimint side (using a fork isn't ideal since LDK and BDK depend on this crate too and we'd have to fork half the universe).

[apoelstra]

I don't know who maintains this crate.

[tcharding]

I'm not a maintainer here man. This is @stevenroose's crate, I'll ping him on signal for you.

[stevenroose]

tACK, sorry, I can't keep up with github notifications anymore. I should spend some time setting up special ones for important projects like these. But changes look good. Will