Fix up to date check in old/new project system for non-absolute path rulesets

[panopticoncentral]

Customer scenario

A project with a ruleset (CodeAnalysisRuleSet) that is specified with a non-absolute path does not correctly resolve and so is always considered out of date. Besides being a bug in the new and old project system, this causes RPS regressions for the old project system.

A recent change (#2464) that added the support for considering ruleset files as part of uptodatecheck caused this regression for projects with non-absolute path.

Bugs this fixes:

https://devdiv.visualstudio.com/DevDiv/_workitems?id=455562

Workarounds, if any

Only workaround to avoid regression is to remove the ruleset from the project file (losing the ruleset functionality).

Risk

Low, this is removing an incorrect item from the up to date check.

Performance impact

Improves performance for projects that use rulesets by doing correct up to date check.

Is this a regression from a previous update?

No.

Root cause analysis:

We are adding integration tests to catch this kind of problem.

How was the bug found?

RPS tests.

** NOTES **

This looks like a big change, but most of the diff involves removing entries from files that are localized, so there are a lot of duplicates.

[panopticoncentral]

Notes on the fix:

We need to move from CodeAnalysisRuleSet, which is not resolved, to ResolvedCodeAnalysisRuleSet. This has to be done, obviously, after targets have been run, so our old strategy of picking up UpToDateBuildInput and UpToDateBuildOutput from the project file is not sufficient. So we move to picking up those items from a target (like we do resolved references).

[davkean]

This seems a very expensive way of resolving the ruleset - we do a design-time build to figure if we need to do a full build?

Is there a reason you went down this approach instead of just rooting UpToDateCheckInput/Output against the project file in the rule data?

[panopticoncentral]

I'm still learning the system, so it's possible that there's a better way of doing this and/or I've gotten confused on things.

My understanding & experience is that if we root the rules in the project file, then they come from the evaluation, before targets are run. In this specific case, that's a problem because we need to run the ResolveCodeAnalysisRuleSet target to get the actual ruleset path that we're going to need to look at. (As a side note, I guess we could just duplicate the logic of that target in the up to date check, but I'm trying to avoid doing too much input-specific logic if I can help it.) Otherwise ResolvedCodeAnalysisRuleSet is just empty at evaluation time and no item is created.

This speaks to a more general problem in that in the old system, targets cannot add up-to-date check inputs or outputs, which is a major limitation.

Regardless, my understanding of the system is that we're not going to get a design-time build specifically for up-to-date checks. We subscribe to the rule data source, so we'll get notified when the general design-time build finishes. Before that, if you try and build, we just throw up our hands and say we don't know. So my understanding is that we're not adding any expense to the system (well, maybe the amount of running an additional target), but we are potentially going to lose some early builds that might have been considered up to date in the old system.

Is all that correct, or am I missing things?

[davkean]

Design-Time builds are not free and add a non-trivial overhead. The first time you hook up the event, that is going to kick of a build just for you because we haven't yet got the data. I plan on making a change where we make sure by the very first design-time build we kick off, we've gather the first set of data of all the services in the system so that don't have them trickling in, kicking off extra underneeded builds. That isn't in, yet.

I've look at your code, I I see that you were already waiting on a design-time build (JointRuleSource) before your data was populated, it looks like a race:

• I make a change
• I build immediately before the design-time build has occurred
• The data you are looking at is out of date

Or is the design-time build a critical task that you bail on?

[davkean]

Ah, I also see that ResolvedCodeAnalysisRuleSet isn't just something you can root with the project, it's literally resolved like a reference.

[davkean]

Okay, based on that I can why you took this approach.

[davkean]

Filed: #2521 to reduce the number of design-time builds.

Depending on init, this change might cause an extra build during startup, can we quickly check to see if there anymore design-time builds with and without this change.?

[panopticoncentral]

I verified there are no additional design-time builds with this change.

[davkean]

Is "current directory" always set to the project file's directory?

[panopticoncentral]

Yes, relative paths in this situation are always relative to the project file's directory.

[srivatsn]

Please retarget this PR to the dev15.3.x branch once #2520 is merged.

[srivatsn]

Tagging @MattGertz for Preview4

[MattGertz]

Taking this for director approval because of the RPS regression.

[MattGertz]

Approved, but I got a frowny face on it, so passing it along :-|