Add initial optional Rust support #5427
Conversation
There was a problem hiding this comment.
Something I genuinely don't understand about rust is with distro packages how do you add extra crates? buildds aren't supposed to have access to the web. So I feel like we should probably be cautious which crates are used and if at all possible configure the build process for distro packages in CI to fail if a crate isn't already installed rather than try to install it.
| !crc | ||
| } | ||
|
|
||
| fn crc32_full(buf: &[u8], crc_init: u32, polynomial: u32) -> u32 { |
There was a problem hiding this comment.
this is probably missing comments about the function documentation and parameters. Not sure if the documentation generator will parse the rust files, but another thing worth checking.
Answered my own question at least for Debian. I think we're still going to have some pain though in getting people setup depending upon what crates we use/depend upon. |
At the moment I'm not planning to use any crates -- all the stuff here is from std. |
hughsie
force-pushed
the
wip/rust
branch
2 times, most recently
from
00e0fab
to
70ea154
Compare
Right; but it's somethign we need to think about. Once the cat is out of the bag, it's a matter of time until there are rust based plugins and those might need to take crates. |
| @@ -37,6 +38,7 @@ fu_crc8_full(const guint8 *buf, gsize bufsz, guint8 crc_init, guint8 polynomial) | |||
| } | |||
| return ~((guint8)(crc >> 8)); | |||
| } | |||
| #endif | |||
There was a problem hiding this comment.
Couldn't you just have this one ifndef wrap all 3? Or is the concern the documentation?
There was a problem hiding this comment.
I was worried about the docs. I'll play -- I didn't like the idea of duplicating them in two source files.
There was a problem hiding this comment.
If possible I would suggest moving them to the rust files, and then making sure the job that builds docs does it from the rust files too. Then that makes sure that we can scale better too as more rust stuff is added.
|
What the status on this? |
It's work in progress. It's something I still want to do, but I don't want to make it more difficult to build fwupd.Being super-honest, I'm also somewhat unimpressed with both the binary size and the compile speed when enabling the rust support. You might have also noticed that slowly other parts of the codebase are actually using rust structures and enums, although we transpile them to C before compiling. e.g. see https://fwupd.github.io/libfwupdplugin/tutorial.html#the-rustgen-helper |
|
@hughsie Huh, that isn't normal usually; Rust is pretty fast and can provide super small binary sizes. Maybe ask people in the Rust community to help in optimization? |
hughsie
force-pushed
the
wip/rust
branch
2 times, most recently
from
24750e8
to
dc253df
Compare
|
@superm1 can you explain what's wrong with |
superm1
force-pushed
the
wip/rust
branch
2 times, most recently
from
68ba254
to
7a7ed99
Compare
|
I've got the meson stuff sorted now at least between this and #6271 |
|
I'm obviously not experienced in rust but given the error the compiler is throwing, it's just saying there is no point to dropping the ref because it's not allocating the memory, right? so just dropping |
Nah, you have to tell rust explicitly -- otherwise it leaks. I'll have a look after the weekend -- thanks for the buildsystem stuff, it's much appreciated. |
Do not too excited, this is only to see what breaks.
Turns out I was 100% wrong; fixed. |
|
Note: this raises the meson dep to 1.0 -- this might be a problem for ChromeOS. |
|
@superm1 lets branch first -- I don't want to open the gates just yet. |
|
Sure no problem. |
Do not get too excited, this is only to see what breaks.
Type of pull request: