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

Jump to bottom

cargo: downgrade off of yanked version of textwrap #667

Closed
wants to merge 1 commit into from
Closed

cargo: downgrade off of yanked version of textwrap #667

wants to merge 1 commit into from

Conversation

martinvonz
Copy link
Member

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)

@martinvonz martinvonz enabled auto-merge (rebase) October 23, 2022 23:13
@martinvonz
Copy link
Member Author

@mgeisler, simply upgrading our own version of textwrap is not enough because we also depend on criterion 0.4.0, which depends on clap 3.1, which depends on the yanked textwrap 0.15.1. What should we do? Would it make sense to release a textwrap 0.15.2 that's the same as 0.15.0, so libraries like clap don't need a new release to work with an unyanked textwrap?

@mgeisler
Copy link

Would it make sense to release a textwrap 0.15.2 that's the same as 0.15.0, so libraries like clap don't need a new release to work with an unyanked textwrap?

Yeah, that might be the best solution... See also clap-rs/clap#4418 and mgeisler/textwrap#453.

@martinvonz
cargo: downgrade off of yanked version of textwrap
2fcb643 
I tried to upgrade off of the yanked version, i.e. 0.15.1 ->
0.16.0. However, we also have a transitive dependency on `textwrap`
via `criterion` -> `clap` -> `textwrap`. The `clap` version used by
`criterion` is `^3.1` (our own binary uses clap 4). The matching
version we currently have in our `Cargo.lock` is 3.2.22, which depends
on the yanked `textwrap` version with `^0.15.1`, so we can't upgrade
or downgrade it. This patch therefore also downgrades `clap` from
3.2.22 to 3.2.21, since that version depends on textwrap
`^0.15.0`. With that change made, we can downgrade `textwrap` to
`0.15.0`.
@martinvonz martinvonz force-pushed the push-ed86d6c135714d58a51d1e24304a0440 branch from d290a5d to 2fcb643 Compare October 24, 2022 15:57
@martinvonz martinvonz changed the title cargo: upgrade off of yanked version of textwrap cargo: downgrade off of yanked version of textwrap Oct 24, 2022
@martinvonz
Copy link
Member Author

@Ralith, I solved this by instead downgrading clap and textwrap. See the updated commit message for explanation. Can you take another look?

@martinvonz martinvonz disabled auto-merge October 24, 2022 15:58
@martinvonz
Copy link
Member Author

@Ralith, I solved this by instead downgrading clap and textwrap. See the updated commit message for explanation. Can you take another look?

There's actually a textwrap 0.15.2 available now (thanks, @mgeisler!), so we could instead upgrade to that. I don't think it really matters since our clap 3 dependency is just used by cargo bench (IIUC). Let me know if you prefer that I upgrade.

@martinvonz
Copy link
Member Author

Actually, with the new 0.15.2 out, the Dependabot upgrade of textwrap from 0.15.1 to 0.16.0 in #670 also included an upgrade from 0.15.2 for the transitive dependency, so I'll just drop this PR.

@martinvonz martinvonz closed this Oct 24, 2022
@martinvonz martinvonz deleted the push-ed86d6c135714d58a51d1e24304a0440 branch October 24, 2022 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ralith Ralith Ralith approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@martinvonz @mgeisler @Ralith