Skip to content

Bump env_logger to 0.10.#1012

Closed
monaka wants to merge 1 commit intobitcoindevkit:masterfrom
diamondhands-dev:pr-bump-env_logger-to-0.10
Closed

Bump env_logger to 0.10.#1012
monaka wants to merge 1 commit intobitcoindevkit:masterfrom
diamondhands-dev:pr-bump-env_logger-to-0.10

Conversation

@monaka
Copy link
Copy Markdown

@monaka monaka commented Jun 21, 2023

Description

Notes to the reviewers

This will reduce using atty that is unmainted.
See also: https://rustsec.org/advisories/RUSTSEC-2021-0145

Changelog notice

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

New Features:

  • I've added tests for the new feature
  • I've added docs for the new feature

Bugfixes:

  • This pull request breaks the existing API
  • I've added tests to reproduce the issue which are now passing
  • I'm linking the issue being fixed by this PR

This will reduce using `atty`.
See also: https://rustsec.org/advisories/RUSTSEC-2021-0145

Signed-off-by: Masaki Muranaka <monaka@monami-ya.com>
@tnull
Copy link
Copy Markdown
Contributor

tnull commented Jun 21, 2023

Unfortunately, env_logger 0.10 seems to require rustc 1.60 which doesn't meet MSRV policy. Up to 0.9.3, env_logger only required rustc 1.41.0, which may therefore be an option. However, this version is still dependent on atty, so at least for the given reason it doesn't make much sense to bump it to 0.9.3 either.

@evanlinjin evanlinjin marked this pull request as ready for review June 21, 2023 10:51
@evanlinjin
Copy link
Copy Markdown
Member

Thank you for this PR, unfortunately it doesn't seem possible due to MSRV requirements (thanks to @tnull for pointing this out). Closing for now.

@evanlinjin evanlinjin closed this Aug 7, 2023
@apoelstra
Copy link
Copy Markdown

If the dep can really be bumped with no changes to the codebase, it may be possible to provide a ranged version that covers multiple major versions.

@monaka monaka deleted the pr-bump-env_logger-to-0.10 branch August 7, 2023 20:04
@notmandatory
Copy link
Copy Markdown
Member

notmandatory commented Aug 8, 2023

@evanlinjin we should be able to bump env_logger to 0.10 and just cargo update --precise it back to 0.7 in CI for verifying build/tests with our MSRV (1.57), as in #1046.

@notmandatory notmandatory added new feature New feature or request ci labels Aug 8, 2023
@evanlinjin
Copy link
Copy Markdown
Member

evanlinjin commented Aug 8, 2023

@evanlinjin we should be able to bump env_logger to 0.10 and just cargo update --precise it back to 0.7 in CI for verifying build/tests with our MSRV (1.57), as in #1046.

If we set env_logger = "0.10", version 0.7 would not be compatible: https://doc.rust-lang.org/cargo/reference/resolver.html#semver-compatibility

We need to see whether setting a range (from 0.7 to 0.10) does not break builds at any version of env_logger (as @apoelstra suggested).

@monaka Sorry I closed the PR too early. Seems like there is a lot more options to explore. I'll create a ticket.

Edit: Ticket created (#1067).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new feature New feature or request

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

5 participants