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

xous-rs: use target_os and target_arch cfgs #252

Merged
merged 5 commits into from
Oct 25, 2022
Merged

Conversation

xobs
Copy link
Member

@xobs xobs commented Oct 25, 2022

Look at target_os to determine whether to enter "hosted" mode or not. If target_os is "xous", then the Xous standard library should default to using builtin Rust libstd features.

If it is "xous", then we should instead use our arch-specific implementations.

This enables third-party programs to directly use the xous crate without specifying feature flags.

Look at `target_os` to determine whether to enter "hosted" mode or not.
If `target_os` is "xous", then the Xous standard library should default
to using builtin Rust libstd features.

If it is "xous", then we should instead use our arch-specific
implementations.

This enables third-party programs to directly use the `xous` crate
without specifying feature flags.

Signed-off-by: Sean Cross <sean@xobs.io>
@xobs
Copy link
Member Author

xobs commented Oct 25, 2022

As an example of the sort of error this addresses:

image

By making this change, out-of-tree packages will be able to use xous without selecting a feature.

@xobs xobs requested a review from bunnie October 25, 2022 07:54
@bunnie
Copy link
Member

bunnie commented Oct 25, 2022

Hmm I guess I'm confused about the definitions of target vs host.

I thought that the "host" was what's running Xous, and so for hosted mode, the host OS would be eg windows/linux, but the target is still Xous.

But I guess you're saying in hosted mode, the target OS is no longer Xous...

Ah, I see. Checking the definitions, the "host" is where the compiler tools are located, and the "target" is where the program runs. So I guess even though you're 'running' Xous, the actual executable is running on windows/linux/etc. so the equivalent of hosted mode is not(target_os = "xous")

I think this patch needs to be expanded to normalize the method of determining if you are running in hosted mode.

Would you say that this is an "ironclad" method of determining if you're in hosted mode?

not(target_os = "xous")

There are no edge cases where this would fail?

@xobs
Copy link
Member Author

xobs commented Oct 25, 2022

The only edgecase where that would fail is if we somehow get Xous to the point where it's running its own programs under its own hosted mode. Aside from that, not(target_os = "xous") will always mean you're building for some other operating system such as Windows or Linux.

We build for "riscv32imac-unknown-xous-elf". The 3rd entry in the target triple is always the target_os. If you change the target triple, it will change the target_os.

@bunnie
Copy link
Member

bunnie commented Oct 25, 2022

Alright. I will add some more stuff to this PR then.

In general, hosted mode is assumed "by default" if the target
is not Xous. This allows packages to be built cleanly on command
lines that don't know to pass a --feature flag to specify a target.

However, an explicit "hosted" flag is still retained so that we
can confirm the build system intent. This saves us from the failure
mode that we thought we were building for target A, but accidentally
ommitted the flag, and instead we've been silently building for
"hosted" mode without any warning. The main consumer of the "hosted"
flag is now just the build.rs script inside of the utralib.
@bunnie
Copy link
Member

bunnie commented Oct 25, 2022

I did a search/replace normalization of references to "hosted" mode and replaced them with the target_os method.

Also I've gone and bumped the package version numbers, once those are all published to crates.io i'll merge the patch.

@bunnie bunnie merged commit 87a8838 into main Oct 25, 2022
@bunnie bunnie deleted the xous-rs-os-selection branch October 25, 2022 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants