Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Linker-Based Asset Propogation #30

Merged
merged 43 commits into from
Jul 1, 2024

Conversation

DogeDark
Copy link
Member

@DogeDark DogeDark commented Jun 26, 2024

This PR is built off of the great work done in #26. Instead of searching the compiled binary for link sections, we create helper functions for CLIs to act as a linker "intercept" to grab assets directly from unlinked object files.

While being a more ideal solution, it does complicate things:

  • CLIs need special handling possibly separate from normal command handling when used as a linker.
  • There is currently no sure-fire way to know the CLI is being called as a linker.
  • The CLI is run twice meaning that collected asset information is not directly available to long-running tasks such as a serve system. The asset information needs to be provided through temp files, sockets, or other methods of propagation.
  • We currently call cargo twice. First for the regular build, second for the linker intercept. This can be omitted with once-stabilized nightly features or more difficult linker-detection systems. If rust-lld becomes the default everywhere, this could become much simpler.
  • In my testing, the working directory was changed while cargo and rustc did it's thing. To solve this, we pass the current working directory as a linker arg that is collected when the CLI is called as a linker.

What Is Added:

// A linker_intercept function that returns a `Some()` if it detects linker args. 
// A list of user-provided args are passed through linker args and paths to every object file (rlib, o) is provided.
// Option<(linker_args, object_files)>
manganis_cli_support::linker_intercept(env::args()) -> Option<(Vec<String>, Vec<PathBuf>)>; 

// A helper function to call the current running exec as a linker by calling `cargo rustc` with the passed `args`. 
// Subcommand is the subcommand the current executable should be called with.
manganis_cli_support::start_linker_intercept(subcommand: ToString, args: IntoIterator, linker_args: IntoIterator);

// A helper function to get manganis json strings from a list of paths to object files.
get_json_from_object_files(object_paths: Vec<PathBuf>) -> Vec<String>;

// AssetManifest load function has changed to accept a list of manganis json strings.
AssetManifest::load(json: Vec<String>) -> Self;

// AssetManifest has a new `load_from_objects` function. 
// This just calls `get_json_from_object_files` and `AssetManifest::load()`.
AssetManifest::load_from_objects(object_files: Vec<PathBuf>) -> Self;

Testing
The way a linker is called seems fairly consistent across platforms, but that doesn't mean we missed something!

  • Windows (link)
  • Linux (cc)
  • wasm32-unknown-unknown (rust-lld)
  • Mac (cc)

Other platforms?

rambip and others added 30 commits April 17, 2024 16:36
* improve error message when asset does not exist

* change fs error message for images too

* manifest path hint when path is not a file

* typos in error message

* store IO error
@ealmloff ealmloff added the bug Something isn't working label Jun 26, 2024
@DogeDark
Copy link
Member Author

I made some changes, can we have Mac tested again?

cd test-package
cargo build -p manganis-cli-support --example cli
../target/debug/examples/cli build

Testing if the shell command executes successfully. If the example exits without error, it works.

@jkelleyrtp
Copy link
Member

Fixes look great - tests pass for me on mac!

MASSIVE THANK YOU! @rambip @DogeDark @ealmloff

This is amazing - a huge win for the Rust asset system. I'm very excited to build more tooling around this, both for dioxus and for other projects also looking to support assets.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants