-
Notifications
You must be signed in to change notification settings - Fork 497
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 gnullvm targets to test.yml #2974
Conversation
One thing I know that is different with the llvm-mingw toolchain (and CLANG* toolchain in msys2, which is pretty similar) is that it does not embed a default manifest. The gcc toolchains do (https://sourceware.org/git/cygwin-apps/windows-default-manifest.git), and I'm sure msvc does. This seems like something winrt might require. |
All repo crates should depend on the current/latest version in the repo.
They probably need a Visual Studio dev command prompt, but only for running MIDLRT (for now).
MSVC does not embed a manifest by default, nor is this needed for COM or WinRT. |
It appears to me to be (at least): |
Locally (I'm manually running the tests on aarch64-pc-windows-gnullvm), I get a panic due to not finding MIDLRT. This is different than on the github runner, where MSVC is present, and it gives
|
on aarch64-pc-windows-gnullvm, the only failure that didn't seem to be due to not having msvc installed was |
I tested this with a rust that is hosted on x86_64-pc-windows-gnullvm, and it works. It fails if I add |
Finding the DLL is not easy because of this: rust-lang/cargo#9661 - that accounts for the "class not registered" errors. |
OK, so now I know why the tests fail. How would I make sure the tests that are expected to pass are passing, without messing things up too much (clearly, just "commenting" out some tests by turning them into strings in the yml file is not the answer) |
All the tests are expected to pass. 🙂 |
Let me rephrase then. What do I need to do to get this PR into an acceptable state, so that as much as possible is tested on gnullvm? Keep in mind I am not a rust developer, I basically just poke at it when it breaks in msys2's packaging of it or other packages that use it. |
/cc @mati865 who is moving -gnullvm forward in rust itself. |
It occurred to me last night that the dll ends up in |
There are no prebuilt std for these targets yet, so have to use nightly, install rust-src, and use build-std.
As a build dependency, it is being built for the "host", which then causes it to be placed in a different directory than the component_client test if an explicit target is specified to cargo.
Changing that to a "normal" dependency vs a build-dependency solves that "class not registered" error. So that just leaves the i686 error due to windows-targets 0.48 being pulled in (indirectly) by the dependency on jsonschema in the sample_component_json_validator* tests. I don't know if there is a way to force cargo to ignore the version specification and always just use the version of the crate in this repository. In my experience, cargo really wants to enforce semver and any method I've thought of to try to mess with that has ended in failure. |
When cross compiling (or using
Build dependencies are built only for the host so they are placed in
No, Cargo is very strict about it. Technically it might be possible using deprecated replace but that could be fragile. |
|
|
I looked at the docs for that, but they say:
I figured it wouldn't hurt to try it anyway, but as expected:
|
@@ -10,6 +10,7 @@ on: | |||
|
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.
All the stuff here to fix the environment for gnullvm should probably be in the fix-environment action...
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 was basing this on what was already done in cross.yml. The only "environment" thing this does that that does not is adding some extra to ~/.cargo/config. Would you like that moved to fix-environment, the whole setup of the llvm toolchain moved to fix-environment, or possibly the setup of the llvm toolchain in a new action (it seems like it should be possible to reuse that between here and cross.yml)?
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.
Chatting with @riverar the non-MSVC testing is a bit of muddle and needs to get cleaned up. Will get back to you on this.
@riverar will take a look at getting this working in the fix-environment action. Closing this for now. |
I am hoping after rust-lang/rust#121712 it will become possible to just |
There are no prebuilt std for these targets yet, so have to use nightly, install rust-src, and use build-std.
Known failures:
tool_yml
is out of date. I wanted to concentrate on the workflow itself, and then can update the generator there.sample_component_json_validator
andsample_component_json_validator_winrt
fail on i686-pc-windows-gnullvm, because they wind up pulling in windows-targets 0.48 and that version is known not to support i686-pc-windows-gnullvm. I don't know how to work around this.sample_component_json_validator_winrt_client
andtest_component_client
fail on both i686 and x86_64-pc-windows-gnullvm, due to "class not registered" error. This seems to be a side-effect of explicitly specifying a target tocargo
.