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

Add build script for Linux, update readme. #52

Merged
merged 8 commits into from
Oct 20, 2023
Merged

Conversation

GitGhillie
Copy link
Collaborator

@GitGhillie GitGhillie commented Sep 27, 2023

Closes #3. It may not be ideal (breaks Bevy dynamic_linking) but hopefully someone with more Linux experience comes along as we continue working on this crate. Edit: This is no longer an issue.

@GitGhillie GitGhillie added documentation Improvements or additions to documentation enhancement New feature or request labels Sep 27, 2023
@passivedragon
Copy link

This works fantastic for the project I tested this with.
I've simplified the buildscript for myself a little already. For testing I just placed the entire fmod thing in the root of my project as just fmod. My build script looks like this, for just linux:

    #[cfg(target_os = "linux")]
    {
        let target_arch = std::env::var("CARGO_CFG_TARGET_ARCH").unwrap();
        let api_loc = [
            format!("./fmod/api/core/lib/{target_arch}"),
            format!("./fmod/api/studio/lib/{target_arch}")
        ];
        for loc in api_loc {
            println!("cargo:rustc-link-search={loc}");
            println!("cargo:rustc-env=LD_LIBRARY_PATH={loc}");
        }
    }

the library structure fmod provides is already sorted by arch, so this seemed a bit nicer to me in my case.

Apologies if I'm jumping the gun a bit. I also don't think it makes sense to copy the files for debug builds, I don't see an advantage or difference, other than time spent copying files.

On another note, it does not break the bevy/dynamic-linking feature. The problem in that sense, is that you need to set the LD_LIBRARY_PATH when running it as well. By doing that, I've successfully had it complain about missing banks with the feature active.

@GitGhillie
Copy link
Collaborator Author

Glad to hear you got it working! I like this direction but I have two problems:

  1. The example doesn't work if I try your solution: error while loading shared libraries: libfmod.so.13: cannot open shared object file: No such file or directory. If I change
    format!("./fmod/api/core/lib/{target_arch}"),
    format!("./fmod/api/studio/lib/{target_arch}")

to something like

    format!("./fmod/api/combined/lib/{target_arch}")

and put all the SO files in there, then it does work. I wonder if I missed anything or if there is maybe a difference between distros on how this is handled?

  1. Afaik target_arch == aarch64 for arm64 users, while the folder inside the fmod download is just arm64. Can be fixed with a few lines of code but it won't be as pretty sadly.

Btw if you can expand a bit more on "you need to set the LD_LIBRARY_PATH when running it as well." I would highly appreciate it!

As for this:

Apologies if I'm jumping the gun a bit. I also don't think it makes sense to copy the files for debug builds, I don't see an advantage or difference, other than time spent copying files.

This was just an idea for a workaround to get dynamic-linking working, but it sounds like that won't be necessary anymore :)

@passivedragon
Copy link

No idea how to replicate the issue, could you tell me a bit more on how you got this? Perhaps which one wasn't found? Somehow I'm doubting that it's a distro issue.

Hmm, according to https://doc.rust-lang.org/reference/conditional-compilation.html#target_arch arm and aarch64 are different, in the case of arm everything is fine as is, from what I can see.

Regarding runtime linking, it's fairly trivial. If you're running the resulting binary directly, you have to specify the LD_LIBRARY_PATH anyway, so that it finds the required libs. In my case, I have it set to this:

target/debug/deps:~/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib:./fmod/api/core/lib/x86_64:./fmod/api/studio/lib/x86_64:$LD_LIBRARY_PATH

So depending on how you'd like run it, either place this in your IDEs env config for launch targets, or export LD_LIBRARY_PATH=... manually.

cargo run takes care of some aspects of setting up these things. Perhaps there is something to be set in the build script or otherwise in a project to make this work out of the box with just cargo run. Unfortunately I'm not aware of a way right now.

@GitGhillie
Copy link
Collaborator Author

What I did was clone the repo, add a build.rs with your solution, add the fmod libs as you described and then I ran cargo run --example minimal. So I didn't have any environment variables set. I was thinking maybe the second call of

    println!("cargo:rustc-link-search={loc}");
    println!("cargo:rustc-env=LD_LIBRARY_PATH={loc}");

was overriding the first one, but then I changed LD_LIBRARY_PATH to LD_RUN_PATH and the environment variable in my IDE config to LD_LIBRARY_PATH=./fmod/api/core/lib/x86_64:./fmod/api/studio/lib/x86_64 and everything started working, even with dynamic-linking enabled. If I don't change to LD_RUN_PATH it will fail to run again with error while loading shared libraries: libfmod.so.13: cannot open shared object file: No such file or directory

@GitGhillie
Copy link
Collaborator Author

I feel like instead of using the IDE it should also be possible to use a config.toml to set the env variable, but I haven't managed to make it work so far. It would be nice to have a solution that is IDE agnostic but also not just executing a raw executable. Anyways I have added the improvements we have so far, thank you for helping out!

@passivedragon
Copy link

passivedragon commented Sep 30, 2023

It would be possible to make an ide independent solution with cargo-make for example, but that would be yet another dependency.

It should be possible to set run time env in the config, or .cargo/ at least, not sure if that's ideal.
I think for the purposes that dynamic linking is used, mainly faster development builds, this seems sufficient to me.

Copy link
Owner

@Salzian Salzian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this logic work in a built game? Once built, the files are in target/release. When a developer ships his game, only that folder will be shipped. How do we bundle the libraries?

Btw, this might be also an issue for windows, needs investigation.

Comment on lines +47 to +48
- Create a new folder `fmod` in the root of your project.
- Extract the `api` folder into it.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we replicate the behavior of windows of putting these files into the root?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO that will get real ugly real soon, as that's 12 files you would be adding to your project root. And if you are shipping the game for different target architectures on linux you need the folders anyways. And if we add the Steam Audio plugin later it's going to be even more lib files in the root.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's also less work for the user this way

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, we should streamline this for all platforms, including Windows. Not sure what convention to use. I've seen in other projects written in different languages the use of a ./vendor folder.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we use a different folder than root for Windows then Windows users will also need to use a build.rs... Maybe let's decide on this in a different PR/issue?

Comment on lines +15 to +29
let mut target_arch = std::env::var("CARGO_CFG_TARGET_ARCH").unwrap();

// Account for the naming mismatch between FMOD library folders and the target architecture
if target_arch == "aarch64" {
target_arch = "arm64".into();
}

let api_loc = [
format!("./fmod/api/core/lib/{target_arch}"),
format!("./fmod/api/studio/lib/{target_arch}"),
];
for loc in api_loc {
println!("cargo:rustc-link-search={loc}");
println!("cargo:rustc-env=LD_RUN_PATH={loc}");
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we wrap this logic into a function of the library? That way, a developer doesn't have to copy all of this code, but only call a function in his build.rs file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's possible, but the user will have to add a separate dependency under [build-dependencies] in their Cargo.toml. It cannot be combined afaik. I can look into it if you think that's the way to go.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really annoying how much of a hassle it is to link to dynamic libraries in Rust.

Would love to find a simple and consistent across platforms solution.

@GitGhillie
Copy link
Collaborator Author

For shipping a game, I added this line in the build.rs comments which should give enough direction hopefully:
"If you are running your executable directly (no IDE, no cargo) see https://www.hpc.dtu.dk/?page_id=1180"
It's then up the user to choose a method and package it accordingly.

I think on Windows you can just put the dll's in the same folder as the exe, but I haven't tried it out yet.

@Salzian
Copy link
Owner

Salzian commented Oct 1, 2023

A comment in build.rs has no visibility to the developer unless included in the documentation.

I think on Windows you can just put the dll's in the same folder as the exe, but I haven't tried it out yet.

If you mean the target/release or target/debug folder, we already discussed that it is suboptimal, since these folders disappear with cargo clean.

Question:

On Windows (maybe) and macOS, it works like that. Can you put the FMOD *.so files next to the binary, and it automatically recognizes them? In that case, it might make more sense to replace the proposed solution here with a bundling mechanism. It will copy the dynamic libraries from some folder in the repository to the output folder.

@passivedragon
Copy link

A comment in build.rs has no visibility to the developer unless included in the documentation.

I think on Windows you can just put the dll's in the same folder as the exe, but I haven't tried it out yet.

If you mean the target/release or target/debug folder, we already discussed that it is suboptimal, since these folders disappear with cargo clean.

Question:

On Windows (maybe) and macOS, it works like that. Can you put the FMOD *.so files next to the binary, and it automatically recognizes them? In that case, it might make more sense to replace the proposed solution here with a bundling mechanism. It will copy the dynamic libraries from some folder in the repository to the output folder.

Bundling and shipping the game/app is up to the user, this is a library, it cannot decide how the app is going to be built. All that can be done is guide users towards potential solutions that might fit their use cases. For dev builds, copying files has no benefit, using dynamic linking the way it is intended makes much more sense to me.

Ultimately you don't have to dynamically link at all if you don't want to, if you'd rather avoid the clutter. For a release build that's likely preferable anyway.

Regarding the visibility of build.rs, it's highly visible to anyone looking at the included examples and wanting to figure out how to use this library. It should also be mentioned in documentation/readme, but saying it has no visibility seems rather odd.

Salzian
Salzian previously approved these changes Oct 20, 2023
README.md Outdated Show resolved Hide resolved
Co-authored-by: Fabian Fritzsche <salzian.dev@gmail.com>
@GitGhillie GitGhillie merged commit 2a654cd into main Oct 20, 2023
@GitGhillie GitGhillie deleted the linux-support branch October 20, 2023 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support Linux
3 participants