-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Avoid special casing work items in sendtohelixhelp.proj
#46315
Conversation
I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label. |
</RunScriptCommand> | ||
</PropertyGroup> | ||
|
||
<MSBuild Projects ="$(RepoRoot)eng/testing/tests.targets" |
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.
Why not import the tests.targets
file and just set your target to depend on GenerateRunScript
? Would that work?
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.
If the "tests.targets" file is imported into the project, there will be such error during build:
error MSB4011: "/runtime/src/mono/wasm/build/WasmApp.targets" cannot be imported again. It was already imported at "/runtime/src/mono/netcore/sample/wasm/console/WasmSample.csproj (73,3)". This is most likely a build authoring error. This subsequent import will be ignored.
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 see... what you could do is remove the WasmApp.targets
import in WasmSample.csproj
and then that issue would be gone.
<Import Project="$(MonoProjectRoot)\wasm\build\WasmApp.targets" /> |
Co-authored-by: Santiago Fernandez Madero <safern@microsoft.com>
@@ -60,6 +60,19 @@ | |||
</Content> | |||
</ItemGroup> | |||
|
|||
<Target Name="PrepareRunScript" BeforeTargets="CopySampleAppToHelixTestDir" Condition="'$(ArchiveTests)' == 'true'" > |
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.
We can do the same as we did for the console sample, right?
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, I'll push that one up too.
…Lipnin/runtime into wasm_sample_use_run_script
After the merge with the latest master there is another error
|
It seems like that was caused because What we could do is condition that import to a property like |
# Conflicts: # src/mono/netcore/sample/wasm/browser/Wasm.Browser.Sample.csproj # src/mono/netcore/sample/wasm/console/Makefile # src/mono/netcore/sample/wasm/console/Wasm.Console.Sample.csproj
@MaximLipnin would you mind updating this PR and solving the merge conflicts? |
Tagging subscribers to this area: @directhex Issue DetailsAddresses #46156
|
gentle ping @MaximLipnin |
It will be addressed in other PR, closing it |
Addresses #46156