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

Use workspace versioning and relax hashbrown/indexmap deps #956

Merged
merged 3 commits into from
Aug 7, 2023

Conversation

jakelishman
Copy link
Member

@jakelishman jakelishman commented Aug 1, 2023

This commit does two things: first, switch a lot of metadata-definition to use the workspace-package inheritance allowed since Rust 1.64 (including several shared dependencies); second, relax the installation requirements on indexmap and hashbrown to allow both to use their most recent two majors. Dependents can then install rustworkx-core using either (say) hashbrown 0.13.2 or hashbrown 0.14.0 as desired, and similar between indexmap 1.9.3 and indexmap 2.0.0.

This commit leaves the lock file in the hashbrown 0.14.0, indexmap 2.0.0 state, although note that some other crate dependents still use hashbrown 0.12.3 internally, but this doesn't matter for the public API surface. I tested it manually in the hashbrown 0.13.2, indexmap 1.9.3 state by updating those two dependencies with:

cargo update -p 'indexmap@2.0.0' --precise 1.9.3
cargo update -p 'hashmap@0.14.0' --precise 0.13.2
cargo update -p 'hashmap@0.12.3'

This commit is kind of a proof-of-concept. I'm happy to split the different parts of it into a couple of different commits / PRs if preferred. I only did all the workspace stuff because it was more convenient for me when playing with the versions of packages - I'm totally happy to close this PR and reopen it in a couple of different bits.

Using this, I was able to build Qiskit against rustworkx-core =0.14.0 using both hashbrown 0.13.2 and hashbrown 0.14.0 with a couple of suitable cargo update commands.

Taken verbatim, this PR fixes #911 - it doesn't replace hashbrown, but it does relax the version requirements.

This commit does two things: first, switch a lot of metadata-definition
to use the workspace-package inheritance allowed since Rust 1.64
(including several shared dependencies); second, relax the installation
requirements on `indexmap` and `hashbrown` to allow both to use their
most recent two majors.  Dependents can then install `rustworkx-core`
using either (say) `hashbrown 0.13.2` or `hashbrown 0.14.0` as desired,
and similar between `indexmap 1.9.3` and `indexmap 2.0.0`.

This commit leaves the lock file in the `hashbrown 0.14.0`, `indexmap
2.0.0` state, although note that some other crate dependents still use
`hashbrown 0.12.3` internally, but this doesn't matter for the public
API surface.  I tested it manually in the `hashbrown 0.13.2`, `indexmap
1.9.3` state by updating those two dependencies with:

    cargo update -p 'indexmap@2.0.0' --precise 1.9.3
    cargo update -p 'hashmap@0.14.0' --precise 0.13.2
    cargo update -p 'hashmap@0.12.3'
@jakelishman
Copy link
Member Author

It's probably possible to avoid allowing multiple versions of indexmap, but when I first set out, I falsely had it in my head that indexmap 1.9.3 exposed hashbrown types and so would need the flexibility, but I think I was just mistaken about that.

@jakelishman
Copy link
Member Author

Last comment: I left the hashbrown requirement at >=0.13, but I didn't actually bother to try and see if it'd build at 0.12 as well. I can if there's interest.

@mtreinish
Copy link
Member

This is a good reminder that I forgot to forward port #929 to main. I did that on stable first for the 0.13.0 bugfix release and the msrv version constraint is lower on 0.13.x so we needed a wider hashmap range.

@jakelishman
Copy link
Member Author

Hahahaha, that's funny - I guess you can just close this PR if it's already done, then.

@mtreinish
Copy link
Member

Nah this pr is good, we need the fix on main too and this does it combined with more modern workspace usage.

@coveralls
Copy link

coveralls commented Aug 1, 2023

Pull Request Test Coverage Report for Build 5787373908

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 96.46%

Totals Coverage Status
Change from base Build 5787279388: 0.0%
Covered Lines: 15313
Relevant Lines: 15875

💛 - Coveralls

Copy link
Collaborator

@IvanIsCoding IvanIsCoding left a comment

Choose a reason for hiding this comment

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

LGTM, this will simplify keeping the dependencies between rustworkx and rustworkx-core consistent in the futre

@IvanIsCoding IvanIsCoding added the automerge Queue a approved PR for merging label Aug 7, 2023
@mergify mergify bot merged commit 34b7ec0 into Qiskit:main Aug 7, 2023
27 checks passed
raynelfss pushed a commit to raynelfss/rustworkx that referenced this pull request Aug 10, 2023
…t#956)

This commit does two things: first, switch a lot of metadata-definition
to use the workspace-package inheritance allowed since Rust 1.64
(including several shared dependencies); second, relax the installation
requirements on `indexmap` and `hashbrown` to allow both to use their
most recent two majors.  Dependents can then install `rustworkx-core`
using either (say) `hashbrown 0.13.2` or `hashbrown 0.14.0` as desired,
and similar between `indexmap 1.9.3` and `indexmap 2.0.0`.

This commit leaves the lock file in the `hashbrown 0.14.0`, `indexmap
2.0.0` state, although note that some other crate dependents still use
`hashbrown 0.12.3` internally, but this doesn't matter for the public
API surface.  I tested it manually in the `hashbrown 0.13.2`, `indexmap
1.9.3` state by updating those two dependencies with:

    cargo update -p 'indexmap@2.0.0' --precise 1.9.3
    cargo update -p 'hashmap@0.14.0' --precise 0.13.2
    cargo update -p 'hashmap@0.12.3'

Co-authored-by: Ivan Carvalho <8753214+IvanIsCoding@users.noreply.github.com>
@jakelishman jakelishman deleted the workspace-relax-indexmap-hashbrown branch August 26, 2023 23:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Queue a approved PR for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider replacing hashbrown use in core API
4 participants