search

Rahix / avr-hal-template

cargo-generate template for avr-hal projects
Apache License 2.0
130 stars 28 forks source link

Why does this template include custom spec files instead of using avr-unknown-gnu-atmega328? #6

Closed jeremysalwen closed 2 years ago

jeremysalwen commented 2 years ago

This is not a request, just a question because I am trying to understand how things work.

The nightly compiler has the avr-unknown-gnu-atmega328 target, but this template doesn't use that target. Is there something broken about it that the avr-specs/avr-atmega-328p.json target is working around?

Rahix commented 2 years ago

The specs are taken from avr-hal where they are generated here: https://github.com/Rahix/avr-hal/blob/main/avr-specs/sync-from-upstream.py

There are indeed slight changes we make in comparison to the target built into the compiler. But the primary reason here is consistency. As we have to use custom target-specs for all the other AVR chips, it wouldn't make sense to treat ATmega328P in a special way.

jeremysalwen commented 2 years ago

I see, if I try to build the example project using --target avr-unknown-gnu-atmega328, I hit linking errors, so the slight changes seem to actually be important.

I would also note that by using the name avr-atmega-328p.json, rather than avr-unknown-gnu-atmega328.json, the specs are incompatible with libraries like https://github.com/japaric/heapless that rely on the target name. Not sure if that is more their responsibility.

Rahix commented 2 years ago

Uff, I know heapless but I wasn't aware of the way they check for target feature support. Well, this mechanism isn't going to work for AVR anyway because there are so many chips which all need different target specs and thus will have unique names...

So, in true "not my fault" style I'd say this is something that needs to be solved on heapless side.

mutantbob commented 2 years ago

Yeah, I offered a patch to heapless, and japaric objects to the targets. https://github.com/japaric/heapless/pull/277 . I have no idea how to deal with this other than 

[patch.crates-io]
heapless = { git = "https://github.com/mutantbob/heapless.git" }

What does the unknown and gnu in the target even mean? I don't think we have a gnu environment on the avr.

Rahix commented 2 years ago

and japaric objects to the targets.

The objection is valid, the target name you proposed to use is the name of the custom targets we created. Those are not fit for usage in heapless and other "upstream" crates. And it does not scale as many of these names exist and more will come in the future.

Instead you need to go the route that was suggested of using #[cfg(target_arch = "avr")]. This will cover all AVR MCUs and thus is the best way forward. I think you can do something like

[target.'cfg(target_arch = "avr")'.dependencies]
atomic-polyfill = { version = "0.1.8" }

and for the build-script check the CARGO_CFG_TARGET_ARCH environment variable?

What does the unknown and gnu in the target even mean? I don't think we have a gnu environment on the avr.

unknown means unknown OS and gnu is there because the target actually does rely on the GNU libc for AVR.

Rahix commented 2 years ago

https://github.com/japaric/heapless/pull/277 is merged so I think we can close this issue. Let me know if there is anything else.