-
Notifications
You must be signed in to change notification settings - Fork 78
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
Replace milksnake with maturin #2393
Conversation
Codecov Report
@@ Coverage Diff @@
## latest #2393 +/- ##
==========================================
- Coverage 84.09% 84.08% -0.01%
==========================================
Files 130 130
Lines 15048 15044 -4
Branches 2208 2403 +195
==========================================
- Hits 12654 12650 -4
Misses 2098 2098
Partials 296 296
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
0002a04
to
64afa31
Compare
FYI, something like this will also work as long as you release from CI: |
Feel free to open an issue in maturin if it's better to be solved in upstream. |
855b191
to
1505f9d
Compare
1362: Add workspace lock file to sdist as a fallback r=messense a=messense See sourmash-bio/sourmash#2393 (comment) > Figure out how to pull `Cargo.lock` from workspace root. cc `@luizirber` Co-authored-by: messense <[email protected]>
1362: Add workspace lock file to sdist as a fallback r=messense a=messense See sourmash-bio/sourmash#2393 (comment) > Figure out how to pull `Cargo.lock` from workspace root. cc `@luizirber` Co-authored-by: messense <[email protected]>
5ced5af
to
1722e21
Compare
The versioning is still pending, but I propose doing it in another PR because this one already have many moving parts... ready for review @sourmash-bio/devs |
.github/workflows/python.yml
Outdated
- name: tox cache | ||
uses: actions/cache@v3 | ||
with: | ||
path: .tox/ | ||
key: ${{ runner.os }}-tox-v3-${{ hashFiles('**/setup.cfg') }} | ||
restore-keys: | | ||
${{ runner.os }}-tox-v3- | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the tox cache due to tox 4
complaining about already existing env
@@ -268,26 +268,17 @@ jobs: | |||
|
|||
- uses: actions-rs/toolchain@v1 | |||
with: | |||
toolchain: 1.48.0 | |||
toolchain: 1.56.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rayon-core
requires 1.56.1, so that's our MSRV now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
general query: is there a reason not to update the MSRV regularly anyway? It shouldn't matter for most things, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
general query: is there a reason not to update the MSRV regularly anyway? It shouldn't matter for most things, right?
Initially I was trying to follow LTS releases or what Debian supported, thinking that it would make it easier to package there.
For example, current Debian stable (bullseye
) is 1.48: https://tracker.debian.org/pkg/rustc
(Ubuntu releases it in the security channel, so even 18.04 has Rust 1.61: https://packages.ubuntu.com/bionic/rustc)
But... there are no sourmash packages in either of them.
I don't think we should go updating MSRV every release, but 1.56.1 isa good one because it is the introduction of the 2021 edition.
|
||
- name: Check if it builds properly | ||
uses: actions-rs/cargo@v1 | ||
with: | ||
command: build | ||
args: --all-features |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make sure that we track optional features too, so dependabot won't leave us in a broken situation.
@@ -11,8 +11,7 @@ | |||
"html_dir": ".asv/html", | |||
"build_cache_size": 8, | |||
"build_command": [ | |||
"python -m pip install 'setuptools_scm[toml]>=4,<6' milksnake", | |||
"python setup.py build", | |||
"python -m pip install 'setuptools_scm[toml]>=4,<6' milksnake maturin", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
still keeping setuptools
and milksnake
around for comparisons with older versions
Looks good! When you merge, can you create a new issue about updating the release process? You could review & merge #2386 at the same time :) |
f368684
to
cfc8b82
Compare
cfc8b82
to
a8e1445
Compare
-> #2425 |
# sourmash release 4.7.0 Major new features: * provide an initial plugin architecture for sourmash that supports new signature saving & loading mechanisms (#2428) * add plugin support for new command-line subcommands (#2438) * debias all containment values (#2243) Minor new features: * Use RankLineageInfo to simplify reading lineages (#2467) * store taxids in lineageDB (#2466) * Use new tax classes for taxonomic summarization (#2443) * add tax summarization dataclasses for safety and flexibility (#2439) * add `--scaled` to sourmash compare (#2414) * replace `lca_utils.LineagePair` with `tax_utils.LineagePair` (#2441) * Add new classes for lineage manipulation (#2437) Cleanup and documentation updates: * ReadTheDocs updates (#2445) * update `sourmash compare` command-line docs (#2400) Developer updates: * fix python tests by bumping tox and pip cache versions (#2424) * Update sphinx requirement from <6,>=4.4.0 to >=4.4.0,<7 (#2430) * Build: replace milksnake with maturin (#2393) * importlib_metadata is a dependency on old Python versions (#2484) * Release docs: use two separate sed commands (#2483) * minor fixes to release behavior (#2479) * Use screed and maturin from nixpkgs in `flake.nix` (#2481) * update release procedure after v4.6.0 and v4.6.1 (#2386) * Update makefile and docs (#2432) Dependabot updates: * Bump once_cell from 1.17.0 to 1.17.1 (#2488) * Bump ouroboros from 0.15.5 to 0.15.6 (#2487) * Bump memmap2 from 0.5.8 to 0.5.9 (#2486) * Bump supercharge/redis-github-action from 1.4.0 to 1.5.0 (#2485) * Bump proptest from 1.0.0 to 1.1.0 (#2460) * Bump web-sys from 0.3.60 to 0.3.61 (#2461) * Bump serde_json from 1.0.91 to 1.0.93 (#2471) * Bump wasm-bindgen-test from 0.3.33 to 0.3.34 (#2463) * Bump cachix/install-nix-action from 18 to 19 (#2459) * Bump wasm-bindgen from 0.2.83 to 0.2.84 (#2464) * Bump typed-builder from 0.11.0 to 0.12.0 (#2451) * Bump bumpalo from 3.9.1 to 3.12.0 (#2450) * Bump pypa/cibuildwheel from 2.11.4 to 2.12.0 (#2447) * Bump bzip2 from 0.4.3 to 0.4.4 (#2444) * Bump once_cell from 1.14.0 to 1.17.0 (#2429) * Bump serde from 1.0.151 to 1.0.152 (#2423) * Bump pypa/cibuildwheel from 2.11.3 to 2.11.4 (#2422) * Bump serde_json from 1.0.89 to 1.0.91 (#2418) * Bump serde from 1.0.150 to 1.0.151 (#2419) * Bump thiserror from 1.0.37 to 1.0.38 (#2417) * Bump finch from 0.4.3 to 0.5.0 (#2416) * Bump rayon from 1.6.0 to 1.6.1 (#2404) * Bump serde from 1.0.149 to 1.0.150 (#2403) * Bump pypa/cibuildwheel from 2.11.2 to 2.11.3 (#2402) * Bump serde from 1.0.148 to 1.0.149 (#2397) * Bump capnp from 0.14.5 to 0.14.11 (#2396)
While investigating why the Python 3.11 PR failed in conda-forge I found out that there is an error in milksnake. Main issue is that milksnake seems to be abandoned, without recent releases, so submitting a PR to fix there might be unfruitful.
maturin is a PEP517-compatible backend for building packages and aimed at Rust packages (pure or mixed with more Python code, like sourmash). It is actively maintained, and supports the
cffi
bindings case that we use too.This PR is based on pyocd/cmsis-pack-manager#195
Closes #1564 (previous try with
setuptools-rust
)TODO
there is no support for setuptools_scm with maturin. Need to replace/version properly.turns out thatsetuptools_scm
is doing something that PyPA doesn't recommend anymore. From core metadata and PEP 643:maturin develop
/tox -e dev
, currently failing.Cargo.lock
from workspace root. Maturin support for workspace is lacking, so the way I found to solve it (use thecore
crate Cargo.toml directly) expect the lock file to besrc/core/Cargo.lock
, but that doesn't exist.setuptools_scm
does), so we need to change the release process =/