-
Notifications
You must be signed in to change notification settings - Fork 10.1k
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
Quick infrastructure fix-ups #25545
Quick infrastructure fix-ups #25545
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -329,9 +329,9 @@ | |
<Uri>https://github.com/dotnet/arcade</Uri> | ||
<Sha>4be47e467013f8a07a1ed7b6e49e39c8150bde54</Sha> | ||
</Dependency> | ||
<Dependency Name="Microsoft.Net.Compilers.Toolset" Version="3.8.0-2.20407.3"> | ||
<Dependency Name="Microsoft.Net.Compilers.Toolset" Version="3.8.0-2.20403.2" Pinned="true"> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why pin this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @TanayParikh this won't protect us from downgrades in the 'master' branch 'til it's merged forward and also won't impact the aspnetcore ➡️ aspnetcore-tooling dependency updates 'til then. Suggest we trigger our Roslyn subscription to get back to being ahead of Arcade in this version on our 'master' branch. Do you know how to do that❔ If not, ping me in Teams. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This is for safety. We've seen runtime changes break roslyn and then roslyn break runtime and extensions too late in the game. Had an offline conversation about this w/ @Anipik @ericstj @jaredpar @mmitche @safern and we landed on this version. (Might move forward slightly later in RC2 but that'll be deliberate and coordinated.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. /fyi all. This version has been discussed in a few different places, the latest being dotnet/roslyn#46575. In RC2, runtime, aspnetcore, and extensions will use The plan is different in our master branches. At least arcade, aspnetcore and aspnetcore-tooling will follow the 'VS 16.8' channel to get newer Microsoft.Net.Compilers.Toolset versions. We will likely move to use the 'VS Master' channel later in 6.0. |
||
<Uri>https://github.com/dotnet/roslyn</Uri> | ||
<Sha>dba2fa57432b4bd9cc7880e2c6fe3acdd01bba3c</Sha> | ||
<Sha>b6a07e61473ed8a804de465f7c1cb5c17f80732d</Sha> | ||
</Dependency> | ||
</ToolsetDependencies> | ||
</Dependencies> |
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.
/fyi @JunTaoLuo @TanayParikh
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.
Ah, crap, I forgot to fix this in the regular job. Will go do that now
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.
That job hasn't been timing out has it i.e. weren't the timeouts specific to the quarantined pipeline❔
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.
And, oops, forgot you were doing this part
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.
Not sure, I thought you meant it was both - I can close #25546 if it's just this one
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.
Hmm, ci.yml doesn't specify timeouts (except for the Helix job) which means its macOS test job gets the default-build.yml default of 180 minutes. Since that's so close to when https://dev.azure.com/dnceng/public/_build/results?buildId=795980&view=results passed, I'll use 240 minutes there.
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.
Let me clarify, the aspnetcore-ci run completed after 90 minutes so it still passed without timing out. I think a default timeout of 180 minutes is still fine. I just wanted to mention that build because 90 minutes is more than 3x the usual time of about 25 mins. It's slow, but I don't think we need to adjust our timeout for it.
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 pushed a change to err on the safe side. The extra half hour timeout is less likely to cause problems elsewhere than having to retry a 1.5 hour job.
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.
(Note I didn't double the timeout there😃)
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.
About the same for me as well. Seen occasional slowness for macos specifically.