-
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
Fix differences between bash and cmd versions of crossgen test script #108500
Conversation
/azp run runtime-coreclr crossgen2 outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
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.
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
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. |
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 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
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). |
/azp run runtime-coreclr crossgen2 outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
Got confused for a bit because Not-Int32 still fails with an invalid RVA even after this fix, but that's a separate problem. |
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. |
…ipt (dotnet#108500) * Reduce differences between bash and cmd versions of crossgen test script * Fix bash crossgen runner script not passing full file list in composite mode * Clean up shellcheck warnings
Thanks to @davmason for finding this.