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

Use Dataflow and snapshots for dependencies node data processing #6183

Closed
drewnoakes opened this issue May 12, 2020 · 0 comments · Fixed by #9008
Closed

Use Dataflow and snapshots for dependencies node data processing #6183

drewnoakes opened this issue May 12, 2020 · 0 comments · Fixed by #9008
Labels
Feature-Dependency-Node "Dependencies" node in Solution Explorer that display project, binary & package references Resolution-Fixed The bug has been fixed, refer to the milestone to see in which release it was fixed. Tracking Tracking a bug against another repo or a larger thematic issue tracking a group of work. Triage-Approved Reviewed and prioritized
Milestone

Comments

@drewnoakes
Copy link
Member

drewnoakes commented May 12, 2020

The processing of data through the Dependencies node code has a few issues that I'd like to address in a batch, potentially in 16.8:

  1. We have bugs around project configurations (e.g. multi-targeting bugs)
  2. Data providers currently notify of updates provide via .NET events
  3. Data providers are stateless, and publish deltas
  4. There are two ways to submit tree updates (public IProjectDependenciesSubTreeProvider and internal IDependencyCrossTargetSubscriber) where there should only be one
  5. The concept of 'filters' should be removed
    • They attempt to work around the statelessness of dependency providers but distribute logic in multiple places, which makes design and debugging difficult
    • They require passing forward lots of state for later steps to use, which could be avoided if the providers were more self-contained and did that work themselves
    • Fixed in Remove concept of snapshot filters from dependency tree data handling #7992
  6. Debugging Dependencies node issues is harder than it should be
  7. By using Dataflow we could have fewer concepts and less code
    • Remove e.g. IDependencyCrossTargetSubscriber, AggregateCrossTargetProjectContext, AggregateCrossTargetProjectContextProvider
    • Use CPS's infrastructure e.g. UnwrapCollectionChainedProjectValueDataSource
  8. The "Dependencies" tree does not populate with the first version of the visible tree. Its children appear later.

The work is mostly self-contained within this repo, however it will also require synchronising changes with the WebTools team for NPM dependencies.

@drewnoakes drewnoakes added Tracking Tracking a bug against another repo or a larger thematic issue tracking a group of work. Feature-Dependency-Node "Dependencies" node in Solution Explorer that display project, binary & package references labels May 12, 2020
@drewnoakes drewnoakes self-assigned this May 12, 2020
@jjmew jjmew added the Triage-Approved Reviewed and prioritized label May 15, 2020
@jjmew jjmew added this to the 16.8 milestone May 15, 2020
drewnoakes added a commit to drewnoakes/project-system that referenced this issue May 19, 2020
Fixes dotnet#6204
Fixes AB#1126588

We are seeing some customer NFEs around this check. The validation is theoretically correct, but the consequences of continuing in such a scenario are minor-to-non-existant. Therefore I'm leaving it as an assert to assist during development of this code.

This code would likely disappear as part of dotnet#6183
drewnoakes added a commit to drewnoakes/project-system that referenced this issue May 19, 2020
Fixes dotnet#6204
Fixes AB#1126588

We are seeing some customer NFEs around this check. The validation is theoretically correct, but the consequences of continuing in such a scenario are minor-to-non-existant. Therefore I'm leaving it as an assert to assist during development of this code.

This code would likely disappear as part of dotnet#6183
drewnoakes added a commit to drewnoakes/project-system that referenced this issue May 19, 2020
Fixes dotnet#6204
Fixes AB#1126588

We are seeing some customer NFEs around this check where the active target framework is "Unsupported,Version=v0.0". The validation is theoretically correct, but the consequences of continuing in such a scenario are minor-to-non-existant. Therefore I'm leaving it as an assert to assist during development of this code.

The real fix for this problem would be covered by dotnet#6183.
@drewnoakes drewnoakes modified the milestones: 16.8, 16.9 Jul 24, 2020
@drewnoakes drewnoakes modified the milestones: 16.9, 16.10 Nov 23, 2020
drewnoakes added a commit to drewnoakes/project-system that referenced this issue Jan 11, 2021
The removed validation in is sound in theory, however is causing quite a few NFEs. For example dotnet#6656.

This commit disables it for now. The consequence of this test failing is that dependencies added to the tree are not exposed via extensibility APIs such as DTE/VSLangProj.

At some point we should revisit how the dependencies tree models its target frameworks, likely as part of dotnet#6183.
@drewnoakes drewnoakes modified the milestones: 16.10, 17.0 May 11, 2021
@drewnoakes drewnoakes modified the milestones: 17.0, 17.1 Aug 10, 2021
@drewnoakes drewnoakes modified the milestones: 17.1, 17.x Nov 26, 2021
@ghost ghost added the Resolution-Fixed The bug has been fixed, refer to the milestone to see in which release it was fixed. label May 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature-Dependency-Node "Dependencies" node in Solution Explorer that display project, binary & package references Resolution-Fixed The bug has been fixed, refer to the milestone to see in which release it was fixed. Tracking Tracking a bug against another repo or a larger thematic issue tracking a group of work. Triage-Approved Reviewed and prioritized
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants