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

Fixing the sourcelink failure for embeded files #33521

Merged
merged 8 commits into from
Mar 15, 2020
Merged

Fixing the sourcelink failure for embeded files #33521

merged 8 commits into from
Mar 15, 2020

Conversation

Anipik
Copy link
Contributor

@Anipik Anipik commented Mar 12, 2020

Generating files before sourcelink calculates the embedded files
Fixes #33097

@danmoseley
Copy link
Member

Please port to preview 2 branch also. You can consider it approved if risk is acceptable to you and @ericstj

@safern
Copy link
Member

safern commented Mar 12, 2020

cc: @dotnet/runtime-infrastructure

Directory.Build.props Outdated Show resolved Hide resolved
Copy link
Member

@safern safern left a comment

Choose a reason for hiding this comment

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

Could you also run an official build with this changes and enabling the source link validation?

I think we should enable the source link validation as part of this change to catch these errors earlier.

@Anipik
Copy link
Contributor Author

Anipik commented Mar 12, 2020

Could you also run an official build with this changes and enabling the source link validation?

yeah i will do that.

@Anipik
Copy link
Contributor Author

Anipik commented Mar 12, 2020

Directory.Build.targets Outdated Show resolved Hide resolved
Copy link
Member

@hoyosjs hoyosjs left a comment

Choose a reason for hiding this comment

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

That one nit. Also, as long as Mono's SPC build also uses arcade defaults sourcelink should work.

eng/common/templates/post-build/post-build.yml Outdated Show resolved Hide resolved
@Anipik
Copy link
Contributor Author

Anipik commented Mar 13, 2020

sourcelink validation succeeded https://dev.azure.com/dnceng/internal/_build/results?buildId=558701

Copy link
Member

@tmat tmat left a comment

Choose a reason for hiding this comment

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

:shipit:

@hoyosjs
Copy link
Member

hoyosjs commented Mar 13, 2020

Please port to preview 2 branch also. You can consider it approved if risk is acceptable to you and @ericstj Eric St. John FTE

The prev2 branch wouldn't pass with this change as-is as the arcade version on the branch doesn't contain dotnet/arcade@527179a

@Anipik
Copy link
Contributor Author

Anipik commented Mar 13, 2020

I talked with @ericstj offline. we dont need to port this preview2 as the embedded files dont contain any actual code, these are just typeforwards or attributes. we can wait till preview 3

@Anipik
Copy link
Contributor Author

Anipik commented Mar 14, 2020

@ericstj @safern @ViktorHofer any insight about the failure.
I was not able to find anything good in the log https://helix.dot.net/api/2019-06-17/jobs/3b8f6fb9-6ab6-4ba6-bec5-c9f313c1dc0f/workitems/baseservices.threading/console

This doesnt seem to be related to this pr but i didnt have much info about the error to create an issue for this.

@danmoseley
Copy link
Member

@Anipik you might have to try to repro locally? 😕

@safern
Copy link
Member

safern commented Mar 15, 2020

I fixed this here: #33580 you just have to rerun the whole pipeline, or you can ignore and merge.

@Anipik Anipik merged commit 570fa1d into dotnet:master Mar 15, 2020
@Anipik Anipik deleted the sourcelink branch March 27, 2020 21:46
@ghost ghost locked as resolved and limited conversation to collaborators Dec 10, 2020
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.

For .net 5 preview1, the CoreFX symbols are missing for IL symbols.
6 participants