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

add output paths as one of causes to reanalyze solution cralwer. #31181

Merged
merged 1 commit into from
Nov 15, 2018

Conversation

heejaechang
Copy link
Contributor

output paths are not specifically affecting semantics of code but some analyzer such as source based test discovery requires it to generate correct data. so they want to be re-analyzed when those are changed as well.

since output path are rarely get changed. decide to add it as one of cause to reanalyze projects.

output paths are not specifically affecting semantics of code but some analyzer such as source based test discovery requires it to generate correct data. so they want to be re-analyzed when those are changed as well.

since output path are rarely get changed. decide to add it as one of cause to reanalyze projects.
@heejaechang heejaechang requested a review from a team as a code owner November 14, 2018 22:32
@heejaechang
Copy link
Contributor Author

tagging @shyamnamboodiripad can you take a look?

tagging @jinujoseph for preview 2

!object.Equals(oldProject.DefaultNamespace, newProject.DefaultNamespace))
!object.Equals(oldProject.DefaultNamespace, newProject.DefaultNamespace) ||
!object.Equals(oldProject.OutputFilePath, newProject.OutputFilePath) ||
!object.Equals(oldProject.OutputRefFilePath, newProject.OutputRefFilePath))
Copy link
Contributor

Choose a reason for hiding this comment

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

We may need to change this again if it turns out that we need to change OutputFilePath back to the obj path and add an additional property for the 'bin' path... cc @jasonmalinowski

Copy link
Member

Choose a reason for hiding this comment

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

Can we write this differently somehow so we don't have to keep updating this list? :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure.

Copy link
Member

@genlu genlu left a comment

Choose a reason for hiding this comment

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

Unrelated to this change, but it might be useful to have additional info on what have changed in project config, so individual analyzer can decide if it's OK to ignore the change. For example, source-based discovery won't be affected by changing analyzer options.

@heejaechang
Copy link
Contributor Author

@dotnet/roslyn-infrastructure is this a known issue?

https://dnceng.visualstudio.com/public/_build/results?buildId=44526&view=ms.vss-test-web.test-result-details

...

AddResourceToModule
Failed an hour ago
Duration0:00:00.054
Failing buildCurrent build
Ownernot available

Debug

Work items

Attachments

History
Error message
System.NullReferenceException : Object reference not set to an instance of an object
Stack trace
at System.Reflection.Assembly.add_ModuleResolve (System.Reflection.ModuleResolveEventHandler value) [0x00000] in <3fbd5de7321a46409ce8ab20db4b2557>:0
at Microsoft.CodeAnalysis.CSharp.UnitTests.Emit.ResourceTests.AddResourceToModule () [0x00367] in <3395e108cbdf4ec4b9dd847a7cdee5cb>:0
at (wrapper managed-to-native) System.Reflection.MonoMethod.InternalInvoke(System.Reflection.MonoMethod,object,object[],System.Exception&)
at System.Reflection.MonoMethod.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x0006a] in <3fbd5de7321a46409ce8ab20db4b2557>:0

@heejaechang
Copy link
Contributor Author

retest roslyn-CI please

@sharwell
Copy link
Member

@heejaechang ADO rebuild works the way Jenkins was supposed to work, not using GitHub comments: 😄

image

@heejaechang heejaechang merged commit d5f7119 into dotnet:dev16.0-preview2 Nov 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants