gyscos
[BUG] Incompatible with bp3d crates
Describe the bug This crate causes compile errors when used with bp3d crates.
To Reproduce Add the following to a Cargo.toml:
bp3d-lua = { version = "1.0.0-rc.2.1.0", path = "../core", features = ["root-vm", "util-module", "libs", "dynamic", "interrupt"] }
bp3d-debug = "1.0.0-rc.6.2.0"
cursive = "0.21.1"
Expected behavior No build error.
Environment
- macOS
- Backend used: ncurses
- Current locale en_US.UTF-8
- Cursive version crate.io latest
Additional context The main problem is this crate requires cursive-core which requires log and time/local-time. The time/local-time feature is banned in any bp3d based tool as that:
- may throw errors in the presence of threads in older versions of time.
- depends on environment variables rather than directly using OS time zone.
- exposes functions which override/hide functions present in bp3d-os/time, obviously the functions in bp3d-os/time do not have the same signature and obviously NOT the same semantics.
The log crate should be avoided as much as possible in the presence of bp3d. When needed the log crate must never be attached to a backend as bp3d forces its own log backend as a redirect to bp3d-debug. If a backend must be provided, it should hook into bp3d-debog rather than log as that ultimately conflict with bp3d. Unfortunately your crate has a log backend which is not behind a feature.
I believe you could just put your logger behind a feature, of course disabled by default and that should fix build with bp3d crates.
Hi, and thanks for the report!
The logger backend is not attached by default. It's only attached in cursive::logger::init() (which itself is not called by any other code in cursive). So as long as you don't explicitly enable the cursive logger, it should be compatible with any other logger backend, including bp3d-debug.
I don't think the mere presence of a type that implements log::Log is enough to cause compilation issues?
Similarly, the time crate is only used in this logger implementation and in the related DebugView, so it shouldn't cause any risk as long as you're not using them.
I unfortunately cannot reproduce your build error (possibly in part because bp3d-lua doesn't seem to be a public crate?). I can compile a project with cursive and bp3d-debug as dependencies, so that doesn't seem to be the problem.
That being said I'm not against moving the logger and DebugView behind a feature flag.
The mere presence of log is not the one to cause issues. The log crate is not recommended but can be used thanks to a log redirect pump implemented in bp3d-tracing. The main difference is more of a performance related issue: bp3d-debug is designed to serve as profiler and logger at the same time while log can only serve as logger and the tracing crate is consuming too many microseconds per frame.
The main compilation problem as I explained happens with time/local-time feature. The time crate is allowed but the mere presence of the local-time feature will define functions which are already implemented by bp3d-os and this cases ambiguity errors. The time crate is not banned by itself; it's the combination time/local-time which is banned for the reasons I've already mentioned in my first message.
This is what I have explained in:
may throw errors in the presence of threads in older versions of time. depends on environment variables rather than directly using OS time zone. exposes functions which override/hide functions present in bp3d-os/time, obviously the functions in bp3d-os/time do not have the same signature and obviously NOT the same semantics.
I understand that reproducing with bp3d-lua may not be desirable, and so the following combination can be used in Cargo to produce issues:
[dependencies]
bp3d-os = { version = "1.0.1", features = [ "time" ] }
time = { version = "0.3.41", features = [ "formatting", "local-offset" ] }
And use the following code in your crate:
use bp3d_os::time::LocalOffsetDateTime;
let now = OffsetDateTime::now_local();
println!("{}", now);
Btw, bp3d-lua is not a private crate, it's just not yet complete and so is not yet on crates.io. You can however find the source code at https://github.com/BlockProject3D/tools.lua/tree/develop.