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 the mono runtime test config #958

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

lewing
Copy link
Contributor

@lewing lewing commented May 29, 2024

Update the build to get things working with the latest mono net9 targets

@mkhamoyan
Copy link

@lewing dotnet/runtime#103881 fixed the conflicting naming error, but now when running wit-bindgen with this PR changes I see this failure
csharp_mono_error
Because of this I couldn't validate if the fix is enough.

@lewing
Copy link
Contributor Author

lewing commented Jul 9, 2024

test result: ok. 27 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 965.25s

need to look at the performance here but csharp-mono tests are green with these changes

@lewing
Copy link
Contributor Author

lewing commented Aug 30, 2024

@dicej
with this version I'm seeing errors like

---- variants::run stdout ----
Error: failed to read component type file: VariantsWorld_component_type.o

Caused by:
    The system cannot find the file specified. (os error 2)

any thoughts?

@@ -190,9 +195,17 @@ impl CSProjectMonoBuilder {
</PropertyGroup>

<ItemGroup>
<NativeFileReference Include=\"{camel}_component_type.o\" Condition=\"Exists('{camel}_component_type.o')\"/>
<_WasiLinkStepArgs Include=\"-Wl,--component-type,{camel}_component_type.wit\" />
<!-- both versions of these seem to fail to find the .o
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wit-bindgen doesn't produce the object file directly anymore,

I've used:

<CustomLinkerArg Include="@(WitComponentImports->Replace('\', '/')->'-Wl,--component-type,&quot;%(Identity)&quot;')" />

In https://github.com/bytecodealliance/componentize-dotnet/pull/35/files#diff-7f4dfa8be7454aa047e35760bfbb0f95c0e1402fd34943606ccd9975e45b8b71

Not sure if _WasiLinkStepArgs is the same?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's another case of bad coevolution dotnet/runtime#107194 but they will be

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The resolution there is that everybody should use LinkerArg over all the other options

@lewing
Copy link
Contributor Author

lewing commented Aug 30, 2024

@dicej with this version I'm seeing errors like

---- variants::run stdout ----
Error: failed to read component type file: VariantsWorld_component_type.o

Caused by:
    The system cannot find the file specified. (os error 2)

any thoughts?

haha, I see the problem now. That is what I get for not looking again in the morning before.

@dicej
Copy link
Collaborator

dicej commented Aug 30, 2024

@dicej with this version I'm seeing errors like

---- variants::run stdout ----
Error: failed to read component type file: VariantsWorld_component_type.o

Caused by:
    The system cannot find the file specified. (os error 2)

any thoughts?

As of dotnet/runtime#103752, the *_component_type.o files have been replaced with *_component_type.wit files. Perhaps you're using an older runtime?

@lewing
Copy link
Contributor Author

lewing commented Aug 30, 2024

@dicej with this version I'm seeing errors like

---- variants::run stdout ----
Error: failed to read component type file: VariantsWorld_component_type.o

Caused by:
    The system cannot find the file specified. (os error 2)

any thoughts?

As of dotnet/runtime#103752, the *_component_type.o files have been replaced with *_component_type.wit files. Perhaps you're using an older runtime?

Sadly I was rushing to get it working last night before heading out, I should have looked again this morning. Sorry for the noise.

@lewing
Copy link
Contributor Author

lewing commented Aug 30, 2024

Ok, so it was also very confusing that the features were chained so the native aot test build was writing over the mono tests once the compilation issue was fixed.

the 'NuGet-Migrations failure is not related to these changes .

There do now appear to be a couple of failures on the mono side dotnet/runtime#107212

true => format!("<WasmBuildNative>{aot}</WasmBuildNative>"),
false => String::new(),
};
let tfm = "net9.0";

Choose a reason for hiding this comment

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

Should we move to Net10 ?
Because we are not going to backport fixes to Net9, right ?
One more below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there won't be a working net10 sdk for a while unfortunately

let out_wasm = out_dir
.join("bin")
.join(configuration)
.join("net9.0")

Choose a reason for hiding this comment

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

tmf

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that is only one of the issues here, build doesn't respect the project name, and the publish doesn't respect the output directory. @maraf we should discuss what options we have here

.arg(out_dir.join(format!("{camel}.csproj")))
.arg("-c")
.arg("Debug")
.arg(configuration)
.arg("/p:PlatformTarget=AnyCPU")
.arg("--self-contained")
Copy link

@pavelsavara pavelsavara Sep 2, 2024

Choose a reason for hiding this comment

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

for me --self-contained doesn't' work with 9.0.100-preview.6.24328.19 without workload. The publish doesn't fail but the file is located elsewhere and it's not single file.

After I install workload Microsoft.NET.Runtime.WebAssembly.Wasi.Sdk version 9.0.0-preview.6.24327.7 I get

FooWorld failed with 4 error(s) (11.4s) → xxx\
    wasm-ld : error : unknown argument: --component-type
    wasm-ld : error : unknown file type: FooWorld_component_type.wit

Which makes sense for preview6.

With preview7 I get

    1: error while executing at wasm backtrace:
           0: 0x296a7b - .tmpJCkP17!fseek
           1: 0x8efe - .tmpJCkP17!load_icu_data
           2: 0x8ff4 - .tmpJCkP17!mono_wasm_load_runtime
           3: 0x9299 - .tmpJCkP17!initialize_runtime
           4: 0x92b8 - .tmpJCkP17!main
           5: 0x28bc5c - .tmpJCkP17!__main_void
           6: 0x7e78 - .tmpJCkP17!_start
           7: 0xb40aff - wit-component:adapter:wasi_snapshot_preview1!wasi:cli/run@0.2.0#run

Which means <WasmSingleFileBundle>true</WasmSingleFileBundle> didn't work.

Copy link
Contributor Author

@lewing lewing Sep 6, 2024

Choose a reason for hiding this comment

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

nothing about this will work without the workload, I'm testing against daily builds of 9

using https://learn.microsoft.com/en-us/dotnet/core/tools/dotnet-install-script
and dotnet-install -c 9.0 -q daily

Copy link
Collaborator

Choose a reason for hiding this comment

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

I had to add <WasmBuildNative>true</WasmBuildNative> to get it to produce a single file

.arg(out_dir.join(format!("{camel}.csproj")))
.arg("-c")
.arg("Debug")
.arg(configuration)

Choose a reason for hiding this comment

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

Suggested change
.arg(configuration)
.arg(configuration)
.arg("-bl:publish-mono.binlog")

@lewing
Copy link
Contributor Author

lewing commented Sep 15, 2024

Several of the issues I hit should be fixed in dotnet/runtime main, but it testing that will be challenging until a proper net10 sdk is produced.

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.

5 participants