Skip to content

[PERF] Fix crossgen zip missing error #87662

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

Merged
merged 18 commits into from
Jun 19, 2023

Conversation

LoopedBard3
Copy link
Member

Fixes: dotnet/performance#2972

Since the archive files are only used by mobile scenarios, gate the task to only run for android and ios scenarios.
Test run showing ios still succeeds with this change in place: https://dev.azure.com/dnceng/internal/_build/results?buildId=2202069&view=results
General test run showing we get further for the crossgen linux run without breaking the other two more.

@ghost
Copy link

ghost commented Jun 16, 2023

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes: dotnet/performance#2972

Since the archive files are only used by mobile scenarios, gate the task to only run for android and ios scenarios.
Test run showing ios still succeeds with this change in place: https://dev.azure.com/dnceng/internal/_build/results?buildId=2202069&view=results
General test run showing we get further for the crossgen linux run without breaking the other two more.

Author: LoopedBard3
Assignees: LoopedBard3
Labels:

area-Infrastructure

Milestone: -

Copy link
Member

@DrewScoggins DrewScoggins left a comment

Choose a reason for hiding this comment

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

From looking at the commits it looks like you started by trying to add the zip tool to the AzDO image. Did that not work?

Other than that, this change seems fine. Does it actually fix the failing leg? I couldn't find a run that went against that leg with this change in our CI.

@LoopedBard3
Copy link
Member Author

Yea, attempting to add the zip tool failed due to permissions when testing locally in the docker. The other part of installing the zip is that Mariner does not seem to have apk as a package manager, meaning we would have to special case the installation whenever an image comes up that doesn't have zip by default.

I did miss running the azdo run with the very latest, I started a run for it here: https://dev.azure.com/dnceng/internal/_build/results?buildId=2204513&view=results

@jkoritzinsky
Copy link
Member

@LoopedBard3 we can update the docker images used for perf testing to include the zip tool. We were trying to limit the packages we install in the images, but we can add more if needed (or define a new image for perf testing)

@LoopedBard3
Copy link
Member Author

@jkoritzinsky This PR should go in regardless as this step does not need to run other than for the runs specified. As such, we don't need the zip tool added into the images right now.

@LoopedBard3 LoopedBard3 merged commit 903eef7 into dotnet:main Jun 19, 2023
@LoopedBard3 LoopedBard3 deleted the PerfFixCrossgenZipMissing branch June 19, 2023 21:09
@ghost ghost locked as resolved and limited conversation to collaborators Jul 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Linux x64 Crossgen runs failing due to zip issue
4 participants