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

Fix osx m1 target path for .gdextension files #238

Merged
merged 1 commit into from
May 8, 2023

Conversation

jrockett6
Copy link
Contributor

My dylib path for M1 is different. This may be something specific to my system configuration, but I wanted to open this just in case something has changed since the example was made. Feel free to ignore.

@Bromeon Bromeon added bug c: tooling CI, automation, tools labels Apr 19, 2023
@Bromeon
Copy link
Member

Bromeon commented Apr 19, 2023

Thanks! This PR essentially reverts part of #215 and thus makes the original issue #208 reappear.
Instead, we should find a way to support both 🙂

Could you try to find out what's different in your case from #208, and how that could be expressed inside .gdextension files?

@jrockett6
Copy link
Contributor Author

Herm... ok so in #208 their compiliation --out-dir (for godot and dodge-the-creeps) is

.../gdext/target/aarch64-apple-darwin/debug/deps

mine is:

.../gdext/target/debug/deps

Why this is the case... I'm not sure. I'm very unfamiliar with rust compilation. I do have rosetta installed, but I'm not sure if that changes things. Per this s/o answer, my default build target looks to be aarch64-apple-darwin (arm64), so I don't think rosetta is at play here.

My knowledge is very limited here, so I don't know what system configuration would change the default target path, maybe the easiest thing here would be to wait until another M1 user comes along and breaks the tie 😅 or if there are other current M1 users that haven't run into this issue, then I guess my configuration is non-default.

As far a checking for the system configuration so both paths could work successfully, I'm unsure how to go about that..

@jrockett6
Copy link
Contributor Author

jrockett6 commented Apr 20, 2023

Ok, I believe the output dir path changes based on whether or not you specify the --target flag in cargo build.

cargo build --target=aarch64-apple-darwin places results in the longer path.

cargo build is the shorter path.

So I believe this PR includes the correct (default) path.

Unfortunately #208 did not post their full cargo build command - just the verbose output.

This should be testable real quick on a different OS/architecture as well to confirm whether the path changes with the --target flag.

@jrockett6 jrockett6 force-pushed the fix_m1_target_path branch from 3c2d611 to 416b1db Compare April 20, 2023 18:32
@jrockett6 jrockett6 changed the title Fix osx m1 target path for dodge the creeps Fix osx m1 target path for .gdextension files Apr 20, 2023
@Bromeon
Copy link
Member

Bromeon commented Apr 20, 2023

Ok, I believe the output dir path changes based on whether or not you specify the --target flag in cargo build.

I don't think the original author explicitly called cargo build like that (why?), but you're right that the verbose log they posted contains --target aarch64-apple-darwin.

What do you get if you run cargo build -v?
What is your precise architecture?

@jrockett6
Copy link
Contributor Author

jrockett6 commented Apr 20, 2023

For godot and dodge-the-creeps crate, cargo build -v output is:

   Compiling godot v0.1.0 (/Users/jrockett/workspace/3rd-party/gdext/godot)
     Running `rustc --crate-name godot --edition=2021 godot/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --crate-type lib --emit=dep-info,metadata,link -C embed-bitcode=no -C split-debuginfo=unpacked -C debuginfo=2 --cfg 'feature="formatted"' -C metadata=47c2b4578159b45e -C extra-filename=-47c2b4578159b45e --out-dir /Users/jrockett/workspace/3rd-party/gdext/target/debug/deps -C incremental=/Users/jrockett/workspace/3rd-party/gdext/target/debug/incremental -L dependency=/Users/jrockett/workspace/3rd-party/gdext/target/debug/deps --extern godot_core=/Users/jrockett/workspace/3rd-party/gdext/target/debug/deps/libgodot_core-14da519a66996f16.rmeta --extern godot_macros=/Users/jrockett/workspace/3rd-party/gdext/target/debug/deps/libgodot_macros-2ae845635546f01a.dylib`
   Compiling dodge-the-creeps v0.1.0 (/Users/jrockett/workspace/3rd-party/gdext/examples/dodge-the-creeps/rust)
     Running `rustc --crate-name dodge_the_creeps --edition=2021 examples/dodge-the-creeps/rust/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --crate-type cdylib --emit=dep-info,link -C embed-bitcode=no -C split-debuginfo=unpacked -C debuginfo=2 -C metadata=2338a6acca1bbf64 --out-dir /Users/jrockett/workspace/3rd-party/gdext/target/debug/deps -C incremental=/Users/jrockett/workspace/3rd-party/gdext/target/debug/incremental -L dependency=/Users/jrockett/workspace/3rd-party/gdext/target/debug/deps --extern godot=/Users/jrockett/workspace/3rd-party/gdext/target/debug/deps/libgodot-47c2b4578159b45e.rlib --extern rand=/Users/jrockett/workspace/3rd-party/gdext/target/debug/deps/librand-818e535b31322abe.rlib`

so no --target it looks like.

My architecture is arm64 (aarch64-apple-darwin), though I may be misunderstanding the question about "precise"

@jrockett6
Copy link
Contributor Author

jrockett6 commented Apr 20, 2023

Here is the output from uname -a

Darwin xxxxxxxxx 21.6.0 Darwin Kernel Version 21.6.0: Sat Jun 18 17:05:47 PDT 2022; root:xnu-8020.140.41~1/RELEASE_ARM64_T8101 arm64

@Bromeon
Copy link
Member

Bromeon commented Apr 20, 2023

@mareksubocz in case you see this:

As you encountered the original issue #208, could you maybe comment on your architecture as well?
Do you see what might be different in this case?

@jrockett6
Copy link
Contributor Author

jrockett6 commented Apr 20, 2023

If I'm not mistaken, wasn't 208 largely caused by the lack of

macos.debug.arm64 = "path"

because just having

macos.debug = "path"

didn't work. As opposed to an incorrect path?

In any case the path doesn't appear to be consistent depending on build environment/command, and it would seem odd to me if the default path for arm64 was different than other architectures, apart from the fact that it's Tier 2 my googling hasn't yielded any hints as to why it would be different.

I am curious if this is it's possible to get the longer path on other machines e.g. target/x86_64-unknown-linux-gnu/debug/deps with specifying the --target flag - just wondering if this is an arm64 specific discussion.

@TitanNano
Copy link
Contributor

If you set the default target inisde the .cargo/config.toml you will always get the long path. This is what I do in my project.

@Bromeon
Copy link
Member

Bromeon commented Apr 30, 2023

Any more ideas on how to proceed here?

We need a way to support both setups. Changing it back will likely just break things again for other users.

@TitanNano
Copy link
Contributor

Any more ideas on how to proceed here?

We need a way to support both setups. Changing it back will likely just break things again for other users.

@Bromeon include the target triplet in all paths and add a README that instructs people to always build with a target specified. Either via .cargo/config.toml or CLI arg. As far as I know, there is no way to get consistent behavior for all targets without users applying some config.

@jrockett6
Copy link
Contributor Author

jrockett6 commented May 1, 2023

It may be hard to set a definitive target triplet in the configuration file though because I don't think the rust target triples map 1:1 with the gdext "xxx.release =" configurations. At least, there are much more potential hosts than godot configuration options - but I'm not full aware of how this is handled on the gdext side.

Using default path without the target triple specified seems fine here imo. Could add something to the readme if needed.

I believe we have confirmed here that this is not a mac specific issue though - it's just that the fix that came from the #208 mac issue was from a user with a non-default configuration. So at a minimum here the m1 path should be made consistent with other architectures.

@Bromeon
Copy link
Member

Bromeon commented May 2, 2023

Makes sense 👍

@jrockett6 could you add a note/comment in the ReadMe that mentions the aarch64 config, so that this information is not lost?

The .gdextension file format doesn't allow comments, so maybe open a separate small code block with these two lines:

macos.debug.arm64 = "res://../../target/aarch64-apple-darwin/debug/libitest.dylib"
macos.release.arm64 = "res://../../target/aarch64-apple-darwin/release/libitest.dylib"

@jrockett6 jrockett6 force-pushed the fix_m1_target_path branch from 416b1db to f1a7376 Compare May 3, 2023 19:34
@jrockett6
Copy link
Contributor Author

Ok I updated the readme, but now I realize that the change may be redundant with the paragraph above:

The [libraries] section should be updated to match the paths of your dynamic Rust libraries. {my-ext} can be replaced with the name of your crate.

So let me know what you think, I can revert the readme change and just add a code block example to the dodge-the-creeps/godot/README.md if that's preferred.

@Bromeon
Copy link
Member

Bromeon commented May 8, 2023

Should be good as-is. Thanks!

bors r+

@bors
Copy link
Contributor

bors bot commented May 8, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

  • full-ci

@bors bors bot merged commit 1f3e46d into godot-rust:master May 8, 2023
@jrockett6 jrockett6 deleted the fix_m1_target_path branch May 8, 2023 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug c: tooling CI, automation, tools
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants