Skip to content

Bump env_logger to 0.10. - #1012

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

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

Conversation

@monaka

@monaka monaka commented Jun 21, 2023

Copy link
Copy Markdown

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

tnull commented Jun 21, 2023

Copy link
Copy Markdown
Contributor

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

notmandatory commented Aug 8, 2023

Copy link
Copy Markdown
Member

@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

evanlinjin commented Aug 8, 2023

Copy link
Copy Markdown
Member

@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