Closed Anders429 closed 1 year ago
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.
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.
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.
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.
I've updated our release notes and will leave this issue open for a while for others using affected targets.
Great, thanks for the quick response!
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?
@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 https://github.com/rust-lang/rust/pull/98333 is relevant?
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!
I have a library,
mgba_log
, which provides a logging implementation for use in Game Boy Advance development, building for the targetthumbv4t-none-eabi
. Back on version0.4.18
, I was usingset_logger()
andset_max_level()
, which seemed to work fine when compiling for this target. However, with version0.4.19
, the project no longer compiles, with the following error messages: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 forthumbv6
targets, but says nothing aboutthumbv4
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)?