Resolve log ambiguity

[nullstalgia]

Howdy. I've been using a lot from esp-rs's work recently, and I've been really impressed! The traits from embedded-hal let me move a device from native SPI to bitbanged SPI, and use the original esp-idf-hal impls as a reference, worked almost first try!.

Back on topic, I wanted to mention something that's disappointed me and I think needs addressed better than it has been before.

There's a module in here named log, and there's an external dependency named log.

This leads to confusion in cases like this one:

use crate::sys::*;

use log::warn;

Whenever the compiler can't fully reason out which log to use (::log: the crate, or sys::log: the module), it halts compilation and throws that up.

I'm writing this now because of this happening again in fatfs.rs.

While I could just throw in a PR that uses ::log in that file, that feels inadequate. I also don't want to alias the log macros in Cargo.toml with

logm = { package = "log", ...

or try to rename the log module without consulting with y'all, so here I am.

My naïve suggestion is to rename the module to perhaps logger, given that's how it's sometimes referred to in that small file.

What do you think is the best solution?

[ivmarkov]

Yes, naming the esp_idf_svc::log module log was a mistake we did early on.

However, now is too late - every single example starts with this line early-on in main and renaming esp_idf_svc::log to something else means we'll just break the world.

If there are resolution issues within some concrete module of the esp-idf-* crates - like as you say - fatfs, these need to be resolved within that module these days.

Btw, glob imports (use esp_idf_svc::sys::*) that we lazily use everywhere aren't helping either.

What compile error do you see in the fatfs module exactly?

[nullstalgia]

Fair enough about wanting to keep existing examples compatible.

Yeah, the globs definitely don't help, but I also don't know if the benefit of denying them is more than keeping them for dev UX. That's up to you guys.

As for the error I get:

(The netif one only is on master, not the rev I was working with)

error[E0659]: `log` is ambiguous
  --> /home/name/.cargo/git/checkouts/esp-idf-svc-a28457b0e32c6283/9e70526/src/fs/fatfs.rs:7:5
   |
7  | use log::warn;
   |     ^^^ ambiguous name
   |
   = note: ambiguous because of a conflict between a name from a glob import and an outer scope during import or macro resolution
   = note: `log` could refer to a crate passed with `--extern`
   = help: use `::log` to refer to this crate unambiguously
note: `log` could also refer to the struct imported here
  --> /home/name/.cargo/git/checkouts/esp-idf-svc-a28457b0e32c6283/9e70526/src/fs/fatfs.rs:10:5
   |
10 | use crate::sys::*;
   |     ^^^^^^^^^^^^^
   = help: consider adding an explicit import of `log` to disambiguate
   = help: or use `self::log` to refer to this struct unambiguously

error[E0659]: `log` is ambiguous
   --> /home/tony/.cargo/git/checkouts/esp-idf-svc-a28457b0e32c6283/9e70526/src/netif.rs:963:9
    |
963 |     use log::debug;
    |         ^^^ ambiguous name
    |
    = note: ambiguous because of a conflict between a name from a glob import and an outer scope during import or macro resolution
    = note: `log` could refer to a crate passed with `--extern`
    = help: use `::log` to refer to this crate unambiguously
note: `log` could also refer to the struct imported here
   --> /home/name/.cargo/git/checkouts/esp-idf-svc-a28457b0e32c6283/9e70526/src/netif.rs:966:9
    |
966 |     use crate::sys::*;
    |         ^^^^^^^^^^^^^
    = help: consider adding an explicit import of `log` to disambiguate
    = help: or use `self::log` to refer to this struct unambiguously

For more information about this error, try `rustc --explain E0659`.
error: could not compile `esp-idf-svc` (lib) due to 2 previous errors

[ivmarkov]

What is really weird is that neither I nor the CI is seeing these errors. Do you enable some extra components in ESP IDF? Or do you have some special sdkconfig.defaults?

Trying to understand what defines the esp_idf_sys::bindings::log symbol, which seems to be the conflict.

[nullstalgia]

It looks like the sdkconfig.defaults line CONFIG_BT_NIMBLE_ENABLED=y is what specifically is causing it, out of the lines I added:

CONFIG_BT_ENABLED=y
CONFIG_BT_BLE_ENABLED=y
CONFIG_BT_BLUEDROID_ENABLED=n
CONFIG_BT_NIMBLE_ENABLED=y # <- 

Plus I have the experimental feature on.

[ivmarkov]

CI does run with experimental enabled, but not with NimBLE enabled, so that's why we did not catch it.

Oh well, I think it would be easier if (once again) we search &replace use log::XXX with use ::log::XXX, rather than fixing the esp_idf_sys glob-imports everywhere.

Would you we willing to try it out by first addressing the compilation errors in the modules where you exhibit the problem (netif and fatfs)?

BTW: I assume all of that is with esp-idf-svc master?

[nullstalgia]

There is the possibility of aliasing the log package to help prevent this in the future. I'd rather have a smart Clippy lint, but that requires more nightly shenanigans that I'm unfamiliar with. My main goal with this issue is wanting to prevent more devs like myself to have to open another issue/PR in a year or so when someone else forgets two colons and it slips through CI.

I originally was running with this commit, but master is also affected (if not more since I didn't have the netif error on that commit.)

And I'm already trying it out, seems to work fine, but I'm having issues with other SPI devices/threads (you might catch me in the matrix channel 😅) so I haven't quite been able to do more than read a sub-kb .txt from the cards.

[ivmarkov]

In the meantime this is fixed with https://github.com/esp-rs/esp-idf-svc/commit/3f2ce04ecc8f076f69c56fe8fa8938294921e638

If you would like to work on a more elaborate solution, we can keep this issue open. But otherwise I would close it.