near / nearcore

Reference client for NEAR Protocol
https://near.org
GNU General Public License v3.0
2.31k stars 618 forks source link

near-o11y is not compilable with Rust 1.71 due to crate dependency and internal module having the same name (opentelemetry) #10477

Open frol opened 8 months ago

frol commented 8 months ago

Describe the bug

near-o11y is not compilable with Rust before 1.72:

image

https://github.com/near/nearcore/blob/55f5586e89708f0b23f6a57aae58eaef270e77af/core/o11y/Cargo.toml#L23

https://github.com/near/nearcore/blob/55f5586e89708f0b23f6a57aae58eaef270e77af/core/o11y/src/lib.rs#L22

There should not be the crate and internal module named the same regardless of the fact if Rust 1.72+ can handle it or not. It may confuse developers dealing with this code.

To Reproduce

Compile near-o11y with Rust 1.70.

Expected behavior

I expect the module to not raise compilation error due to the naming collision.

Version (please complete the following information):

nagisa commented 8 months ago

The ecosystem has generally converged to MSRV policy of N - 2 (current stable and 2 previous releases.) Anything more than that is going to be a very painful uphill battle for everybody involved, so I don’t really see any motivation for nearcore try and provide a guarantee greater than that.

The current stable rust toolchain is already up-to 1.75.0, with 1.73 being the oldest supported release within the N - 2 scheme. Although nearcore does not currently commit to supporting anything but the latest stable rust toolchain, supporting versions as old as 1.71 seems like it wouldn’t be in scope within even the N - 2 policy.

frol commented 8 months ago

@nagisa Regardless of MSRV, I would find it confusing as a contributor as it makes harder to reason about things where there is a naming collision

nagisa commented 8 months ago

I don’t disagree with that motivation, and I did not contend it :) Having a maintainable and surprise-free codebase is definitely a desirable property.

We can rename the module for sure, the current name is entirely internal to the implementation of the o11y crate.