Version 0.4.19 breaks build for target thumbv4t-none-eabi

[Anders429]

I have a library, mgba_log , which provides a logging implementation for use in Game Boy Advance development, building for the target thumbv4t-none-eabi. Back on version 0.4.18, I was using set_logger() and set_max_level(), which seemed to work fine when compiling for this target. However, with version 0.4.19, the project no longer compiles, with the following error messages:

error[E0425]: cannot find function `set_logger` in crate `log`
   --> /home/runner/work/mgba_log/mgba_log/src/lib.rs:309:10
    |
309 |     log::set_logger(&LOGGER)
    |          ^^^^^^^^^^ not found in `log`

error[E0425]: cannot find function `set_max_level` in crate `log`
    --> /home/runner/work/mgba_log/mgba_log/src/lib.rs:311:24
     |
311  |         .map(|()| log::set_max_level(LevelFilter::Debug))
     |                        ^^^^^^^^^^^^^ help: a function with a similar name exists: `max_level`
     |
    ::: /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/log-0.4.19/src/lib.rs:1268:1
     |
1268 | pub fn max_level() -> LevelFilter {
     | --------------------------------- similarly named function `max_level` defined here

You can see these error messages on my CI run, which is what brought my attention to it initially.

Is this an intentional breakage? The documentation for set_logger() mentions a lack of availability for thumbv6 targets, but says nothing about thumbv4 targets. I also checked the changelog and didn't see anything calling this out specifically. Of course, thumbv4t-none-eabi is a tier 3 target, so perhaps it is not guaranteed by this crate anyway (which is totally understandable)?

I assume this is related to the change in #555. For this target, should I just be using the "racy" versions of these functions anyway (meaning it was an error at the non-racy versions worked at all before)?

[KodrAus]

Thanks for the report @Anders429. It’s possible this is just a bug after I needed to apply my clumsy git-fu to split our release into two. I’ll check it out.

[KodrAus]

I do think you should probably use the racy versions of those functions, it is possible the allow-list in the old build script might have been missing this target. If this was actually intentional then we should at least call it out in our release notes.

[Anders429]

Looks like the old build.rs file previously explicitly listed this target in the target_has_atomics() function. This was originally added in #488.

I'm not entirely sure on the atomic story for this target. Should thumbv4t-none-eabi not have been included in this list originally? The target spec does seem to indicate it should have some atomic support, but I don't know to how that fits in with the atomic support needed for log. I'm certainly not an expert on this by any means.

I'm definitely fine with just using the racy versions if that's the actual proper thing to do going forward.

[KodrAus]

Ok, it looks like this was intentional, and we were incorrectly enabling the atomic versions on thumbv4t-none-eabi, and probably others too. Our old build script was just thoroughly insufficient for target detection. Sorry about the mixup @Anders429! I'll update the release notes to call this out.

[KodrAus]

I've updated our release notes and will leave this issue open for a while for others using affected targets.

[Anders429]

Great, thanks for the quick response!

[guncha]

Same issue with riscv32imc-unknown-none-elf. The target doesn't have atomic instructions, but according to the RISC-V spec for RV32I:

naturally aligned loads and stores are guaranteed to execute atomically

Which should be enough to safely update the max log level, no?

[KodrAus]

@guncha I think log is doing the right thing now by not concerning itself with that level of platform specific behaviour. It follows what the target_has_atomic config tells it.

It seems like there might be ongoing work in Rust to enable atomics for RISCV but I’m not across it myself. Maybe rust-lang/rust#98333 is relevant?

[KodrAus]

This issue has been open for a few months and we've just queued up an 0.4.20 release, so I think it's a reasonable to point to close this one now. Thanks everyone for all the input!