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

Set props & envs to diagnose null ref issue #42320

Closed
wants to merge 4 commits into from

Conversation

ellahathaway
Copy link
Member

@ellahathaway ellahathaway commented Jul 23, 2024

Related to dotnet/dnceng#3305

This PR sets the Features prop to debug-analyzers and 3 envs to help debug the null ref issue linked above.

The dumps will be published as build artifacts as part of

find artifacts/log/ -exec rsync -R {} -t ${targetFolder} \;

@ellahathaway ellahathaway requested review from a team as code owners July 23, 2024 22:03
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Infrastructure untriaged Request triage from a team member labels Jul 23, 2024
@ellahathaway ellahathaway requested review from jaredpar and mmitche July 23, 2024 22:20
@@ -84,6 +84,8 @@
<!-- Pass locations for assets -->
<BuildArgs>$(BuildArgs) /p:SourceBuiltAssetsDir=$(ArtifactsAssetsDir)</BuildArgs>
<BuildArgs>$(BuildArgs) /p:SourceBuiltAssetManifestsDir=$(RepoAssetManifestsDir)</BuildArgs>
<!-- TODO: Needed to debug null ref issue. Remove once https://github.com/dotnet/dnceng/issues/3305 is resolved -->
<BuildArgs>$(BuildArgs) /p:Features=debug-analyzers</BuildArgs>
Copy link
Member

Choose a reason for hiding this comment

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

Is this going to flow everywhere are needed or should this be set as an env?

Copy link
Member

Choose a reason for hiding this comment

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

This flows all the way into the inner builds.

Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately /p: in MSBuild is a global final setting. So this would override anyone that is using <Features> in their project / props files. That is mostly esoteric copmiler flags so unlikley it's being used actively right now. For the moment think we should go forward with this as is to track down this problem.

@baronfel, @rainersigwald another case where a version of -p: which was initial value not final value would be useful.

@ellahathaway ellahathaway requested a review from mmitche July 23, 2024 22:53
@ellahathaway ellahathaway enabled auto-merge (squash) July 23, 2024 23:16
@@ -84,6 +84,8 @@
<!-- Pass locations for assets -->
<BuildArgs>$(BuildArgs) /p:SourceBuiltAssetsDir=$(ArtifactsAssetsDir)</BuildArgs>
<BuildArgs>$(BuildArgs) /p:SourceBuiltAssetManifestsDir=$(RepoAssetManifestsDir)</BuildArgs>
<!-- TODO: Needed to debug null ref issue. Remove once https://github.com/dotnet/dnceng/issues/3305 is resolved -->
<BuildArgs>$(BuildArgs) /p:Features=debug-analyzers</BuildArgs>
Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately /p: in MSBuild is a global final setting. So this would override anyone that is using <Features> in their project / props files. That is mostly esoteric copmiler flags so unlikley it's being used actively right now. For the moment think we should go forward with this as is to track down this problem.

@baronfel, @rainersigwald another case where a version of -p: which was initial value not final value would be useful.

@ellahathaway
Copy link
Member Author

Build failures in fsharp are related to the envs. I’m currently investigating.

@ellahathaway ellahathaway marked this pull request as draft July 24, 2024 01:11
auto-merge was automatically disabled July 24, 2024 01:11

Pull request was converted to draft

@ellahathaway
Copy link
Member Author

Going to trigger the builds on an internal branch instead of merging these changes into main. I'm keeping the PR open until we can finish investigating the build failures that occurred with these changes.

@ellahathaway
Copy link
Member Author

Closing this PR since we found another way to repro.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Infrastructure untriaged Request triage from a team member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants