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

[mono][tvos] Do not treat assembly.pdb/xml files as native files to bundle when AOTing on Helix #107079

Merged
merged 2 commits into from
Sep 2, 2024

Conversation

ivanpovazan
Copy link
Member

@ivanpovazan ivanpovazan commented Aug 28, 2024

Description

In #106737 we switched from using package to project references in libraries tests for a number of dependencies. This affected iOS/tvOS CI testing, more specifically AOTing and test app bundling on Helix, causing builds to fail due to missing files. The root cause of failure were assumptions in MSBuild targets that perform collection and packaging of build artifacts to be transferred to Helix machines which assumed that files like: System.Runtime.Serialization.Formatters.pdb can only appear in the runtime pack output folder. This is not true anymore as this file will appear in the output folder of the test being built since it is now a project reference. (details described in: #107029 (comment)).

Changes

The issue has been fixed by excluding all assembly.dll/.pdb/.xml from AppleNativeFilesToBundle to correct the assumption and slightly clean-up the implementation. Pdbs are properly included only if DebuggerSupport feature switch is enabled for a particular test and are transferred to Helix for AOTing accordingly.
As a side note, builds on Helix always used assemblies from the current build of the runtime pack.


Fixes #107029

@ivanpovazan ivanpovazan added NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) NO-REVIEW Experimental/testing PR, do NOT review it labels Aug 28, 2024
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Aug 28, 2024
@ivanpovazan
Copy link
Member Author

/azp run runtime-extra-platforms

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ivanpovazan
Copy link
Member Author

/azp run runtime-extra-platforms

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ivanpovazan ivanpovazan changed the title WIP: [mono][tvos] Do not treat assembly.pdb/xml files as native files to bundle when AOTing on Helix [mono][tvos] Do not treat assembly.pdb/xml files as native files to bundle when AOTing on Helix Aug 29, 2024
@ivanpovazan ivanpovazan marked this pull request as ready for review August 29, 2024 14:29
@ivanpovazan ivanpovazan added area-Infrastructure-mono os-tvos Apple tvOS and removed NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) NO-REVIEW Experimental/testing PR, do NOT review it needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Aug 29, 2024
@ivanpovazan
Copy link
Member Author

The failing tests reported in: #107029 are now passing.
Other CI failures don't seem to be related.

Copy link
Member

@matouskozak matouskozak left a comment

Choose a reason for hiding this comment

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

LGTM!

@ivanpovazan ivanpovazan merged commit dd03a13 into dotnet:main Sep 2, 2024
179 of 196 checks passed
@ivanpovazan ivanpovazan deleted the fix-helix-builds branch September 2, 2024 07:03
ericstj pushed a commit to adamsitnik/runtime that referenced this pull request Sep 17, 2024
…undle when AOTing on Helix (dotnet#107079)

* Do not treat assembly.pdb/xml files as native files to bundle

* Bundle satellite assemblies as well
github-actions bot pushed a commit that referenced this pull request Sep 17, 2024
…undle when AOTing on Helix (#107079)

* Do not treat assembly.pdb/xml files as native files to bundle

* Bundle satellite assemblies as well
carlossanlop pushed a commit that referenced this pull request Sep 17, 2024
* Remove package references from library tests (#106737)

* Remove package references from library tests

These tests should be referencing the product assemblies so that they
test latest and not old bits.

* Reference the OOB version of SRSF and make sure it's copied

* BinaryFormatter tests should be skipped only on AOT, WASM and Mobile (#106858)

* respect AppContext switch (which is currently enabled for all projects in the root Directory.Build.props file)

* add project reference to all test projects that need working BF (and were being skipped for a while)

* adjust to changes from #104202: EqualityComparer<string>.Default is mapped to StringEqualityComparer, but serialized as GenericEqualityComparer<string>

* Don't use WeakReferences in the round trip test, as the target may get freed in the meantime, fixes #104905 (#106967)

* Enable more BinaryFormatter tests (#107408)

* enable the BinaryFormatter tests in System.Runtime.Serialization.Formatters.Tests

* add new test project, where the flag is disabled and it runs only 3 tests in total that ensure that

* The SerializationGuard is no longer activated since BF was moved to the OOB package, the tests need to reflect that.

* Disable binary formatter tests when DotNetBuildSourceOnly. (#107549)

* [mono][tvos] Do not treat assembly.pdb/xml files as native files to bundle when AOTing on Helix (#107079)

* Do not treat assembly.pdb/xml files as native files to bundle

* Bundle satellite assemblies as well

* [mono][ci] Include PDBs from runtime pack when building on Helix if required (#107348)

---------

Co-authored-by: Eric StJohn <[email protected]>
Co-authored-by: Tom Deseyn <[email protected]>
Co-authored-by: Ivan Povazan <[email protected]>
carlossanlop pushed a commit that referenced this pull request Sep 17, 2024
…107903)

* Remove package references from library tests (#106737)

* Remove package references from library tests

These tests should be referencing the product assemblies so that they
test latest and not old bits.

* Reference the OOB version of SRSF and make sure it's copied

* BinaryFormatter tests should be skipped only on AOT, WASM and Mobile (#106858)

* respect AppContext switch (which is currently enabled for all projects in the root Directory.Build.props file)

* add project reference to all test projects that need working BF (and were being skipped for a while)

* adjust to changes from #104202: EqualityComparer<string>.Default is mapped to StringEqualityComparer, but serialized as GenericEqualityComparer<string>

* Don't use WeakReferences in the round trip test, as the target may get freed in the meantime, fixes #104905 (#106967)

* Enable more BinaryFormatter tests (#107408)

* enable the BinaryFormatter tests in System.Runtime.Serialization.Formatters.Tests

* add new test project, where the flag is disabled and it runs only 3 tests in total that ensure that

* The SerializationGuard is no longer activated since BF was moved to the OOB package, the tests need to reflect that.

* Disable binary formatter tests when DotNetBuildSourceOnly. (#107549)

* [mono][tvos] Do not treat assembly.pdb/xml files as native files to bundle when AOTing on Helix (#107079)

* Do not treat assembly.pdb/xml files as native files to bundle

* Bundle satellite assemblies as well

* [mono][ci] Include PDBs from runtime pack when building on Helix if required (#107348)

---------

Co-authored-by: Eric StJohn <[email protected]>
Co-authored-by: Adam Sitnik <[email protected]>
Co-authored-by: Tom Deseyn <[email protected]>
Co-authored-by: Ivan Povazan <[email protected]>
jtschuster pushed a commit to jtschuster/runtime that referenced this pull request Sep 17, 2024
…undle when AOTing on Helix (dotnet#107079)

* Do not treat assembly.pdb/xml files as native files to bundle

* Bundle satellite assemblies as well
@github-actions github-actions bot locked and limited conversation to collaborators Oct 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
2 participants