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

Bump workspace to owo-colors 4.0 #167

Merged
merged 4 commits into from
Apr 25, 2024

Conversation

workingjubilee
Copy link
Contributor

Requires adding a path dependency in color-eyre on color-spantrace so that the versions agree. Fortunately we can do that now!

@@ -23,7 +23,7 @@ tracing-error = { version = "0.2.0", optional = true }
backtrace = { version = "0.3.48", features = ["gimli-symbolize"] }
indenter = { workspace = true }
owo-colors = { workspace = true }
color-spantrace = { version = "0.2", optional = true }
color-spantrace = { version = "0.2", path = "../color-spantrace", optional = true }
Copy link
Contributor

Choose a reason for hiding this comment

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

Unless I am mistaken, We don't need a path dependency to get the versions to agree. What error were you getting?

I am planning to make this a proper workspace once the crates are merged too, but I would prefer if we don't add path dependencies until that point so that I could use workspace path dependencies.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pksunkara You are indeed mistaken.

color-spantrace = { version = "0.2", path = "../color-spantrace", optional = true }

meant that it was resolving to the color-spantrace on crates.io, which depends on owo-colors 3.5, which is not compatible with 4.0, which meant that the types inside color-eyre were disagreeing on the version, which was preventing the conversion in the relevant From impl.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Specifically this code does not compile:

#[cfg(feature = "capture-spantrace")]
impl From<Theme> for color_spantrace::Theme {
fn from(src: Theme) -> color_spantrace::Theme {
color_spantrace::Theme::new()
.file(src.file)
.line_number(src.line_number)
.target(src.spantrace_target)
.fields(src.spantrace_fields)
.active_line(src.active_line)
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I forgot we haven't published this. Thanks.

@@ -1,4 +1,3 @@
/target
**/*.rs.bk
Cargo.lock
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the reason for committing the lockfile?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pksunkara
Copy link
Contributor

Windows tests are failing.

@workingjubilee
Copy link
Contributor Author

That's unfortunately because of upstream rustc changes.

Requires adding a path dependency in color-eyre on color-spantrace
so that the versions agree. Fortunately we can do that now!
Also, check in our lockfile, due to Cargo's change in guidance for libs.
@thenorili thenorili force-pushed the bump-owo-colors-for-everyone branch from db8c839 to b65fb59 Compare April 25, 2024 21:28
@pksunkara pksunkara merged commit aed87e4 into eyre-rs:master Apr 25, 2024
29 checks passed
workingjubilee pushed a commit to pgcentralfoundation/pgrx that referenced this pull request Jul 15, 2024
Removes one dependency on atty, which has a security vulnerability and
appears to be unmaintained.

Remaining dependencies to be fixed and/or released:

*   killercup/cargo-edit#900
*   eyre-rs/eyre#167
@workingjubilee workingjubilee deleted the bump-owo-colors-for-everyone branch July 15, 2024 23:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants