-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Copy published crossgen2 in artifacts/tests #80154
Copy published crossgen2 in artifacts/tests #80154
Conversation
Tagging subscribers to this area: @hoyosjs Issue DetailsFixes #80110
|
50a8fa9
to
5cb264a
Compare
300ce0d
to
25514ef
Compare
583f275
to
72fe8e5
Compare
931c0ee
to
db8b174
Compare
a7d30ec
to
79c6554
Compare
Overall looks good to me, thanks Adeel, in general I believe this to be heading in the right direction. We probably need to soften some rough edges though. It seems to me that quite a few R2R tests failed in your PR run, that will require investigation and fixing (theoretically these could be caused by the weird addition of ".." in populating |
In our today .NET core runtime sync several people emphasized that we should coordinate this work with the diagnostics team to make sure we don't paint ourselves in a corner due to the fact that today NativeAOT bugs are in general harder to analyze and investigate. /cc-ing @tommcdon for visibility. |
With this PR the repo no longer builds for me. I get this error:
Build command is |
Also, superpmi-collect fails during "Create Core_Root" step on linux-arm (and probably other platforms) with:
Edit: Looks like linux-arm and linux-arm64 are broken while other targets are ok. Edit 2: I'm guessing the problem is somewhere in the following that does not end up producing the right crossgen2 bits: runtime/eng/pipelines/coreclr/superpmi-collect.yml Lines 112 to 151 in 5fd3f22
|
This reverts commit 62835af.
https://dev.azure.com/dnceng-public/public/_build/results?buildId=787949&view=results also appears to have some failures that look related in the "Test crossgen2-comparison X to Y" jobs. |
Can you pinpoint the error? Those failures look to be same as #106948. |
The supported command is (and always have been): |
I have built this way for 3 years without issues. I do not see a mention that |
From
|
Yes, for debug libraries I understand that it needs to be specified. I never build debug libraries, so I never have needed to specify it -- section below what you quote mentions that it is only necessary if your libraries configuration is not release. |
The fix is simple (set LibrariesConfiguration = Release by default). We did ran couple of pipelines, including superpmi by making a dummy change. Apparently it wasn't enough.
We were using that as an ultimate test and didn't ran crossgen2 outerloop. |
👍
In this case superpmi-collect is the pipeline that substantially uses crossgen2, while superpmi-diffs/superpmi-replay do not use it at all. Those latter pipelines need only JIT + SPMI builds and then download the collections created by superpmi-collect. One thing that confuses me a bit is that it seems we are running ilc even when
To be fair I am not sure that the crossgen2 outerloop pipeline is checked much. I just noticed it while trying to find a publicly accessible pipeline run that hit the same issue as superpmi-collect (since it is only internally accessible). |
I think we can add a public test-only version of this pipeline that does everything except upload the collections. That should make the testing much more straightforward for contributors. |
If it is blocking, lets revert it. I think two of the fixes are striaghtforward (add a line in src/tests/Common/Directory.Build.props and update crosgen2 pipeline yamls to drop .dll). Yup, the superpmi-collect issue needs some investigation. It maybe as simple as passing --cross to: runtime/eng/pipelines/coreclr/superpmi-collect.yml Lines 124 to 129 in 5fd3f22
or something more involved. I'll take a closer look when I am off my day job. 😅 |
Thanks! I think we should revert it for now as otherwise we'll quickly end up with no SPMI collections for linux-arm/linux-arm64 while we investigate. |
…0154)" (dotnet#106956)" This reverts commit b579962.
)" This reverts commit b579962.
This reverts commit 62835af.
)" This reverts commit b579962.
)" (dotnet#106965) This reverts commit b579962.
Fixes #80110