-
-
Notifications
You must be signed in to change notification settings - Fork 320
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
Remove platform rebuilding and generate_stub_lib
#6696
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should really change the ux here as part of this change.
I am also really not a fan of requiring platforms to include some sort of special libapp.roc
and dynhost
just to enable correctly linking. These are the types of confusing implicit deps that make it hard to use.
Like, probably can just be:
roc build --lib examples/simple.roc
cargo build
roc preprocess-host --host target/build/host --platform platform/main.roc
If we want/need to still depend on the dylib, lets make it explicit instead of locking in on libapp.so
that happens to be in the platform directory.
roc build --lib examples/simple.roc
cargo build
roc preprocess-host --host target/build/host --platform platform/main.roc --lib examples/simple.dylib
Fundamentally, the libapp
and dynhost
naming should die. It is just a weird constraint that will confuse users, especially if it is implicit like this.
I also am questioning if it would be possible to just use an example as the actual input. The example once parsed knows where the platform is. If the example is example/simple.roc
, the dylib should be example/simple.dylib
. I guess that still leave an open question about host binary though.
All this said, I think we can remove the dependency on an actual dylib and the stub symbols. So I think we should remove both of those. The only piece of information we actually need is what shared library the host thinks it linked to such that we can remove that dependency during preprocessing.
So I guess, I currently lean most towards this api if it works:
roc build --lib examples/simple.roc
cargo build
roc preprocess-host --host target/build/host --app examples/simple.roc
generate_stub_lib
from roc cligenerate_stub_lib
and platform rebuilding
generate_stub_lib
and platform rebuildinggenerate_stub_lib
Current progress for
|
14b7da7
to
3da4075
Compare
This looks good to me in general :) |
3da4075
to
5317347
Compare
examples/breakout/host/lib.rs
Outdated
pub extern "C" fn main() { | ||
// let bounds = roc::Bounds { | ||
// width: 1900.0, | ||
// height: 1000.0, | ||
// }; | ||
|
||
// gui::run_event_loop("RocOut!", bounds).expect("Error running event loop"); | ||
println!("TODO FIX PLATFORM IMPLEMENTATION"); | ||
std::process::exit(0); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Anton-4 -- do you think we should try and keep both breakout and the gui platform? They aren"t in a good state currently. I have made changes so they at least compile and are usable (though breakout I have basically broken entirely) for CI purposes. What is our plan/thoughts longer term?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, let"s take them out, I don"t think they"ve been a net positive overall.
interface Help | ||
exposes [ | ||
RocTarget, | ||
Arch, | ||
Os, | ||
archFromStr, | ||
osFromStr, | ||
rocTarget, | ||
prebuiltBinaryName, | ||
dynamicLibraryExtension, | ||
] | ||
imports [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO - move to a better location than in examples
160b2ea
to
aa009b7
Compare
generate_stub_lib
generate_stub_lib
Thank you for your contribution! Sometimes PRs end up staying open for a long time without activity, which can make the list of open PRs get long and time-consuming to review. To keep things manageable for reviewers, this bot automatically closes PRs that haven’t had activity in 60 days. This PR hasn’t had activity in 30 days, so it will be automatically closed if there is no more activity in the next 30 days. Keep in mind that PRs marked |
Good bot -- this PR is still active. Split off key parts of this into #6808 to ease the change. |
Status with this PR. Some of these commits are useful. My intent is to break this into smaller PRs. I plan on closing this when it is no longer useful. |
Closing as no longer required. |
zulip discussion
This PR
generate_stub_lib
roc preprocess-host
Roc platform hosts" are responsible for building using their native toolchain so they provide the binaries for legacy and surgical linking. The roc cli no longer has any dependency on a host toolchain.
The API for pre-processing the host has changed to the following: