Up-to-date checks: UpToDateCheckInput and UpToDateCheckOutput #2416
Conversation
|
@davkean If you could take a look and let me know what you think. I haven't added item types before, so may have done something wonky here. I'm seeing UpToDateCheckInput and UpToDateCheckOutput items show up in Solution Explorer, is this desired? This also removes hard-coded item types and just works off the schema that we define. |
|
@dotnet-bot retest this please |
| { | ||
| OnProjectChanged(e.Value.Item1); | ||
| OnProjectImportsChanged(e.Value.Item2); | ||
| OnSourceItemChanged(e.Value.Item3); | ||
| OnSourceItemChanged(e.Value.Item3, e.Value.Item5); |
There was a problem hiding this comment.
Could you add a comment saying what is Item3 and Item5?
There was a problem hiding this comment.
Is Item5 tracking the CustomInput?
There was a problem hiding this comment.
Sure, will do. Item5 is the project schema which we use to interrogate about what item types are used for up to date input.
| { | ||
| foreach (var itemType in e.ProjectChanges.Where(changes => changes.Value.Difference.AnyChanges)) | ||
| var itemTypes = new HashSet<string>(projectItemSchema.GetKnownItemTypes().Where(itemType => projectItemSchema.GetItemType(itemType).UpToDateCheckInput)); |
There was a problem hiding this comment.
Seems like we wont be hitting this path often, so this does not seem to be a perf hit.But would we be performant in the case of loading a project with a huge number of files?
There was a problem hiding this comment.
This particular line of code only iterates over the list of known item types, it isn't affected by the actual number of files. Or were you concerned about something further down?
There was a problem hiding this comment.
Thats true. Disregard my 'number of files' comment
| @@ -168,11 +182,11 @@ private void OnOutputGroupChanged(IImmutableDictionary<string, IOutputGroup> e) | |||
| } | |||
| } | |||
|
|
|||
| private void OnChanged(IProjectVersionedValue<Tuple<IProjectSubscriptionUpdate, IProjectImportTreeSnapshot, IProjectSubscriptionUpdate, IImmutableDictionary<string, IOutputGroup>>> e) | |||
| private void OnChanged(IProjectVersionedValue<Tuple<IProjectSubscriptionUpdate, IProjectImportTreeSnapshot, IProjectSubscriptionUpdate, IImmutableDictionary<string, IOutputGroup>, IProjectItemSchema>> e) | |||
There was a problem hiding this comment.
Tuple [](start = 54, length = 5)
Is there a reason this can't be a C#7 tuple? IProjectVersionedValue<(IProjectSubscriptionUpdate subscriptionUpdate, IProjectImportTreeSnapshot importTreeSnapshot, IProjectSubscriptionUpdate otherSubscriptionUpdate)>
There was a problem hiding this comment.
I tried but got type conversion errors. I could wrestle with the type system some more to see if there's a way around that, it's possible I missed something...
There was a problem hiding this comment.
hmm, that sounds like it might be a tuple bug. @jcouv is there a reason C# 7 tuples cannot be used above? Is this an issue that has been fixed in 7.1?
There was a problem hiding this comment.
Tuples should work, but the callers need to be updated to pass tuples/System.ValueTuple instead of System.Tuple. That may have been the problem.
I tried to stop by your office to look at the error. Ping me tomorrow when you have time to try again.
In reply to: 121548234 [](ancestors = 121548234)
|
Tagging @MattGertz to bring this for 15.3 - the prime reason for taking this would be that this offers an escape hatch to mark items to be considered for uptodate check for items that we don't know about (i.e when we have bugs). |
MattGertz
added
Pending director approval
Approved to merge
and removed
Pending director approval
labels
|
@panopticoncentral this has been approved to merge now. |
|
Thanks! |
|
Thanks for the clarification. I jumped the gun ;-) |
By default, VS will not consider None files that are not copied to output to have any effect on build, and therefore changing targets will result in VS saying there's nothing to build. In order for the Build/Pack to trigger anytime we make changes in any targets file, add the @(None) to the up-to-date check input files. Found via dotnet/project-system#2416
Customer scenario
In the old project system, a project file can specify UpToDateCheckInput and UpToDateCheckOutput items that add items into inputs or outputs that the fast up-to-date check uses. It's a kind of escape hatch for special project extensions that the project system is not aware of.
Bugs this fixes:
Fixes #2380
Workarounds, if any
In cases where these items would be used, the user would have to manually rebuild if their project was out of date but the project system didn't see it that way.
Risk
Low, it just adds two new item types that are only used in exotic situations.
Performance impact
Low, only impacts exotic situations.
Is this a regression from a previous update?
No.