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 differences between bash and cmd versions of crossgen test script #108500

Merged
merged 5 commits into from
Oct 4, 2024

Conversation

kg
Copy link
Contributor

@kg kg commented Oct 2, 2024

Thanks to @davmason for finding this.

@kg
Copy link
Contributor Author

kg commented Oct 2, 2024

/azp run runtime-coreclr crossgen2 outerloop

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@davmason davmason left a comment

Choose a reason for hiding this comment

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

It looks like it will solve the issue we found, I am not an expert on this part of the tests but if the outerloop is in a better place than we should ship it

@kg
Copy link
Contributor Author

kg commented Oct 3, 2024

Will address all the PR feedback tomorrow now that the outerloop run finished. It looks like this reduced the failure count on outerloop by 70, but it seems like the comparison lanes are broken and didn't run.

Copy link
Member

@ivdiazsa ivdiazsa left a comment

Choose a reason for hiding this comment

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

I left a Nit comment but besides that, @janvorli has pointed out all the observations I also had. So once those are addressed, this lgtm! Thank you so much for fixing this Katelyn!

Address PR feedback
@kg
Copy link
Contributor Author

kg commented Oct 3, 2024

One last PR comment to address. I ran shellcheck over the generated script and fixed a bunch of stuff it found (the rest of the warnings are spurious or are located outside of CrossGen.targets).

@kg
Copy link
Contributor Author

kg commented Oct 3, 2024

/azp run runtime-coreclr crossgen2 outerloop

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@kg
Copy link
Contributor Author

kg commented Oct 4, 2024

Got confused for a bit because Not-Int32 still fails with an invalid RVA even after this fix, but that's a separate problem.

@kg kg marked this pull request as ready for review October 4, 2024 02:27
@kg
Copy link
Contributor Author

kg commented Oct 4, 2024

Composite linux x64 checked was at 94 failures on my most recent run of it outside this PR, and this PR brings it down to 80 failures by fixing most (not all) of the invalid RVA errors that were caused by errors in the script. Merging.

@kg kg merged commit f05bfc4 into dotnet:main Oct 4, 2024
103 of 115 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants