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

Use OutDirName to determine the SymStore subdirectory name #5174

Merged
merged 1 commit into from
Apr 13, 2020

Conversation

dougbu
Copy link
Member

@dougbu dougbu commented Apr 1, 2020

Description

Because $(MSBuildProjectName) is not necessarily unique within a repo e.g. matching ref/ and src/ projects, we can end up overwriting symbols. This one word change ensures symbols always go to unique directories.

Customer Impact

Without this, customers may be unable to download symbols while debugging.

The issue has recently led to problems pushing new Razor bits into Visual Studio, That could mean we delay getting other fixes to customers.

Regression

No, this problem has been around for a long time. Was previously missed because symptoms involve a build race and tend not to break every assembly : symbol relationship.

Risk

Very low.

Workarounds

No known customer workarounds.

It may be possible to change enough in individual repos to avoid problems. But, something like renaming the non-unique projects is likely much riskier.

- the name has to match the output directory binaries are built to. This directory is constructed in
  https://github.com/dotnet/arcade/blob/master/src/Microsoft.DotNet.Arcade.Sdk/tools/ProjectLayout.props#L13 based on `OutDirName`
- cherry-picked from 559d684 (#5150) then fixed up
@dougbu dougbu requested review from tmat and markwilkie April 1, 2020 02:44
@dougbu
Copy link
Member Author

dougbu commented Apr 1, 2020

@markwilkie where is the template I should add to this 'release/3.x' PR?

@markwilkie
Copy link
Member

@dougbu dougbu added the tell-mode Servicing tell-mode label Apr 1, 2020
@dougbu
Copy link
Member Author

dougbu commented Apr 1, 2020

🆗 I followed the process for an Arcade tell mode change

@markwilkie
Copy link
Member

Not sure what dependencies (if any) 3x takes on the symstore directory. @tmat and @JohnTortugo ?

@JohnTortugo
Copy link
Contributor

From the publishing side we only reference the ArtifactsSymStoreDirectory directory. It's fine if things change under that.

@dougbu
Copy link
Member Author

dougbu commented Apr 2, 2020

@markwilkie w/ the tell-mode info and label, am I waiting on anything else before merging? (No, I'm not authorized to merge into this branch.)

@markwilkie
Copy link
Member

What would be the downside/s of waiting for a bit for this to "bake"? I'd prefer this to be vetted for a while in master before porting. Thoughts?

@dougbu
Copy link
Member Author

dougbu commented Apr 2, 2020

@markwilkie my concern is the potential to ship unaligned symbols again in 3.1.4. Suggest we let it bake some but make sure it gets into 'release/3.x' with time for dotnet/extensions to update Arcade bits before that release locks down.

@markwilkie
Copy link
Member

Sounds good. When's a good target date do you think @dougbu ?

@dougbu
Copy link
Member Author

dougbu commented Apr 3, 2020

Let's say 12 April.

@markwilkie
Copy link
Member

Sounds good. If you could ping me again middle of the month and let's do this. :)

cc/ @riarenas @ilyas1974 @mmitche for visibility

@dougbu
Copy link
Member Author

dougbu commented Apr 13, 2020

@markwilkie the time is ripe to get this in. Again, we'd like to avoid mismatched symbols in 3.1.4.

The dotnet/runtime, dotnet/extensions, and dotnet/aspnetcore repos are all at Arcade SDK v5.0.0-beta.20201.2 and that contains @tmat's 559d684 fix. I have not heard of any issues since getting the new Arcade bits.

@markwilkie
Copy link
Member

I'm good with this. Thanks!

@dougbu
Copy link
Member Author

dougbu commented Apr 13, 2020

@markwilkie you're welcome. But, I can't merge into this branch (hint, hint)

@markwilkie markwilkie merged commit 1a55276 into release/3.x Apr 13, 2020
@dougbu dougbu deleted the dougbu/cherry-pick/fix-symstore-output-dir branch April 13, 2020 18:40
@dougbu
Copy link
Member Author

dougbu commented Apr 13, 2020

Thanks @markwilkie❕ What's the usual time between a 'release/3.x' merge and the new build showing up in Maestro++?

In addition, what is your perspective on updating Arcade in the three repos with matching src/ and ref/ project names versus doing it across every repo building for 3.1.4?

/cc @wtgodbe @mmitche
/fyi @ericstj this fix should improve assembly / symbol matchups in dotnet/corefx if that repo uses the Arcade SDK's _InnerDeployToSymStore target in 3.1 builds.

@riarenas
Copy link
Member

Once this PR merges, and an arcade-validation official build with the changes finishes, it will be available in the channel:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tell-mode Servicing tell-mode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants