bouffalolab / bl808-pac

Embedded Rust's Peripheral Access Crate for BL808
Other
117 stars 6 forks source link

Proposal: Generate svd2rust files during build stage #3

Open EnzoAlbornoz opened 1 year ago

EnzoAlbornoz commented 1 year ago

After every change in bl808.svd, we need to regenerate the crate source code. We can avoid this by generating the code in build.rs and including it through include!(concat!(env!("OUT_DIR"), "/generated.rs"));. Only some tricks were needed in order to format and get the top-level docs.

Waiting for any comments or suggestions. 🙂

duskmoon314 commented 1 year ago

Why do you want to give up the benefit of the already-built crate and choose to spend more time building?

9names commented 1 year ago

It is also useful with a PAC to be able inspect the generated files (and the differences between them) during modifications to the SVD, as well as during review, or when updating svd2rust. While I think this PR has the best of intentions, I do not think this is a good change for users, developers or maintainers.

EnzoAlbornoz commented 1 year ago

Why do you want to give up the benefit of the already-built crate and choose to spend more time building?

Because we will have a single source of truth. Imagine that someone makes a PR modifying the SVD file. Can you guarantee the changes in the SVD are correctly reflected in the generated code? It will make accepting any PR more complex than generating the code during build time.

Also, when you are using this crate, you are building it from source anyway. This PR is just adding a small extra step (it takes less than 2 seconds to generate the code).

It is also useful with a PAC to be able inspect the generated files (and the differences between them) during modifications to the SVD, as well as during review, or when updating svd2rust. While I think this PR has the best of intentions, I do not think this is a good change for users, developers or maintainers.

We should not expect any differences from the generated code that aren't outlined by the changes on the SVD file. We are talking about generated code, so it should be like using a macro. The generated docs are still the same, and the user continues to be able to look at the generated source code when coding.

EnzoAlbornoz commented 1 year ago

Accepting a PR like #4 will break the code consistency because we'd need to regenerate the crate source code. It boils down to maintaining a couple of hundred lines instead of several thousand.

orangecms commented 1 year ago

Do I understand correctly that someone developing this PAC can run a command to see the generated code, and end consumers can look at the generated sources that have been published via docs.rs or otherwise generating the code locally themselves?

I'd be either for removing the generated code - which is an artifact, as @EnzoAlbornoz points out -, or set up a CI check to see that the source changes match with the changes in the generated code, giving the assurance of consistency.

EnzoAlbornoz commented 1 year ago

Do I understand correctly that someone developing this PAC can run a command to see the generated code, and end consumers can look at the generated sources that have been published via docs.rs or otherwise generating the code locally themselves?

I'd be either for removing the generated code - which is an artifact, as @EnzoAlbornoz points out -, or set up a CI check to see that the source changes match with the changes in the generated code, giving the assurance of consistency.

You can see the generated code in the build artifacts. E.g.: target/debug/build/bl808-pac-<hash>/out/generated.rs image image

I was able to generate the docs too.

image

9names commented 1 year ago

Accepting a PR like https://github.com/bouffalolab/bl808-pac/pull/4 will break the code consistency because we'd need to regenerate the crate source code.

That PR does not result in any change to the generated code. There is literally nothing to commit for such a PR. It just makes it more compatible with other tooling.

Also, when you are using this crate, you are building it from source anyway. This PR is just adding a small extra step (it takes less than 2 seconds to generate the code).

On your machine. With this presently incomplete SVD. It takes longer than that to download and build svd2rust and regex. And then you generate the files. It's hard to measure download time (and it's different for everyone) so lets take that out of the picture.

Here's the clean build time difference between this branch and upstream (on my fast desktop Linux PC):

real 0m2.759s user 0m8.994s

real 0m10.737s user 0m35.392s

Cool, only 8 seconds longer, approximately 3 times the build time

And on my much slower Windows laptop:

At least it's consistent? 3 times the build time. And a considerable amount for this machine. Rebuilds are sub-second in both cases, I'll consider that free.

set up a CI check to see that the source changes match with the changes in the generated code

Seems reasonable to me.