search

flatpak / flatpak-builder

Tool to build flatpaks from source
GNU Lesser General Public License v2.1
141 stars 93 forks source link

[WIP] Add Cargo buildsystem #564

Open chriskilding opened 1 year ago

chriskilding commented 1 year ago

Adds a Flatpak buildsystem for Cargo.

Implements #15

Note: I am opening this PR at an early stage to gather initial feedback and requirements for what needs to be done. I'm not familiar with the internals of Flatpak, so will gratefully accept any help.

chriskilding commented 1 year ago

I'm also a bit stuck on how to add the new test cases to the script / entry point that runs the test suite - could someone advise where this is done, so that they can be run in the build?

TingPing commented 1 year ago

The entry point is https://github.com/flatpak/flatpak-builder/blob/main/tests/test-builder.sh

They are just simple shell scripts.

TingPing commented 1 year ago

So we need to make this have value beyond what buidlsystem: simple just calling cargo does. That means that we have something that actually expands Cargo sources or similar.

chriskilding commented 1 year ago

@TingPing agreed - that was always the intention. I just wanted to get the test harness set up around it first to TDD my changes.

Would you still recommend tomlc99, or are there any better (perhaps Cargo-specific) parser options? Ideally we'd want a C TOML library which is readily installable from the OS package manager.

TingPing commented 1 year ago

I have no opinion at all of TOML libraries.

On Fedora these are packaged:

TingPing commented 1 year ago

Not packaged but maybe interesting:

TingPing commented 1 year ago

I think I'd lean towards tomlc99 as a Meson subproject.

chriskilding commented 1 year ago

Another thought about parsing the Cargo / Cargo.lock files....

If we take the Toml parser route with tomlc99 or similar, effectively we would have to reimplement all of Cargo's parse+fetch logic in C. Furthermore the Cargo project occasionally changes the Cargo.lock file format (as indicated by its version number), so each time this changes we'd have to update the flatpak-builder reimplementation. This will likely be non-trivial.

I am therefore thinking that either of the following strategies would be better than reimplementing the Cargo dependency fetch logic ourselves:

  1. Use the cargo crate and some kind of Rust-C FFI bindings to call the fetch Rust function from C directly. (This presumes that this function exists and is publicly available within Cargo's internal library. I imagine the FFI bindings would make this option more difficult to implement.)
  2. Do a shell call within flatpak-builder to effectively exec cargo fetch. (I think this will be simpler to implement, and also make it easier to accommodate newer versions of Cargo and the Cargo.lock file format. We would however have to take care to show any errors from the delegated cargo fetch call to the user.)

Thoughts?

hfiguiere commented 1 year ago

Also one key thing here is that cargo isn't part of the base sdk. So the failure mode might not be ideal.

chriskilding commented 1 year ago

@hfiguiere yes, also a good point. I suppose this prompts the question of how flatpak-builder should provide buildsystems for any language/toolchain that's not in the base SDK - not just Rust/Cargo.

chriskilding commented 1 year ago

One possibility is that the buildsystem API could provide a feature where you can declare the sdk-extensions that the buildsystem depends on.

There would then be a couple of options for how to resolve those sdk-extensions when needed. A 'soft' resolution option would be to use the extension if it's installed, but if it's not, flatpak-builder prompts the user to add them to the Flatpak manifest with a message like:

"The buildsystem depends on the following sdk-extensions [ foo, bar, baz ], but these were not present. Please declare them in your Flatpak manifest."

Meanwhile a more automated option would be for flatpak-builder to just go ahead and resolve all the buildsystem's sdk-extension dependencies by itself. This would, however, mean that those sdk-extensions are not declared in the Flatpak manifest. There might be unintended consequences to that, but I don't have a view yet on what those could be.

alufers commented 10 months ago

"The buildsystem depends on the following sdk-extensions [ foo, bar, baz ], but these were not present. Please declare them in your Flatpak manifest."

Meanwhile a more automated option would be for flatpak-builder to just go ahead and resolve all the buildsystem's sdk-extension dependencies by itself. This would, however, mean that those sdk-extensions are not declared in the Flatpak manifest. There might be unintended consequences to that, but I don't have a view yet on what those could be.

The problem I see with this is that you can choose multiple different rust SDK extensions (stable, nightly etc.):

https://github.com/flathub/org.freedesktop.Sdk.Extension.rust-stable https://github.com/flathub/org.freedesktop.Sdk.Extension.rust-nightly

To add insult to injury, those SDK extensions put cargo in different paths, and they have to be appended to PATH manually in the manifest.