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

We should be respecting $(ProjectAssetsFile) #1437

Closed
davkean opened this issue Feb 2, 2017 · 9 comments · Fixed by #5006
Closed

We should be respecting $(ProjectAssetsFile) #1437

davkean opened this issue Feb 2, 2017 · 9 comments · Fixed by #5006
Labels
Feature-NuGet NuGet integration including pushing it properties, project and package references, and Pack support. Triage-Approved Reviewed and prioritized
Milestone

Comments

@davkean
Copy link
Member

davkean commented Feb 2, 2017

We find the assets files ourselves in the ProjectLockFileWatcher. We shouldn't be doing that, we should respecting $(ProjectAssetsFile).
See: NuGet/Home#4463 (comment)

@srivatsn srivatsn self-assigned this Feb 8, 2017
@srivatsn srivatsn added this to the 1.1 milestone Feb 8, 2017
@srivatsn srivatsn added the Bug label Feb 8, 2017
@srivatsn srivatsn modified the milestones: 15.3, 15.6 May 26, 2017
@Pilchie Pilchie modified the milestones: 15.5, 15.6 Oct 17, 2017
@Pilchie Pilchie added Area-New-Project-System Feature-NuGet NuGet integration including pushing it properties, project and package references, and Pack support. labels Dec 19, 2017
@Pilchie Pilchie assigned natidea and unassigned srivatsn Dec 19, 2017
@Pilchie Pilchie modified the milestones: 15.6, Unknown Dec 19, 2017
@nguerrera
Copy link
Contributor

Something just occurred to me. @dsplaisted made changes so that SDK will not default ProjectAssetsFile and instead use what restore writes to generated temp props. Would this create a chicken-and-egg problem for initiating the watch when restore hasn't been done yet?

@davkean
Copy link
Member Author

davkean commented May 11, 2018

@nguerrera Yes, this is a chicken and egg. If we can't find $(ProjectAssetsFile) we can't tell CPS that it needs to reevaluate. We were also planning to move to a model where CPS itself monitored the file based on https://github.com/dotnet/project-system/blob/master/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/DesignTimeTargets/Microsoft.Managed.DesignTime.targets#L281 but sounds like this would also be broken by that.

@nguerrera
Copy link
Contributor

:(

@dsplaisted
Copy link
Member

I don't know how feasible this is, but I think the "right" thing to do would be to monitor the MSBuildProjectExtensionsPath. NuGet restore will drop .props and .targets which should be imported there, as well as the assets.json file. So when the new .props and .targets show up there the project should be re-evaluated. The .props file will then set the ProjectAssetsFile property, and you can then additionally watch for changes in that file if it's necessary.

@davkean
Copy link
Member Author

davkean commented May 11, 2018

We already monitor the props/targets when we reevaluate and they appear, also wouldn't play nicely with shared intermediates.

Was this change to remove logic from SDK? If so, can we add it back to NuGet targets?

@dsplaisted
Copy link
Member

@davkean To clarify, will CPS detect when new .props / .targets files show up in the MSBuildProjectExtensionsPath, or will it only detect if files that were part of the evaluation initially are updated?

@davkean
Copy link
Member Author

davkean commented May 11, 2018

  • Assets file changed
  • Reevaluate
  • New props/targets are picked up

We don't monitor for props/targets existence under they appear in evaluation - there's thousand of locations they could appear.

@jjmew jjmew added the Triage-Approved Reviewed and prioritized label Jan 14, 2019
@dzmitry-lahoda

This comment has been minimized.

@nguerrera

This comment has been minimized.

davkean added a commit to davkean/project-system that referenced this issue Jul 9, 2019
IFileWatchDataSource is a new feature that was added to unify the logic of re-revaluating a project based on external files (such as .editorconfig and project.assets.json). Plug in <AdditionalDesignTimeBuildInput /> items, of which project.assets.json is the only one.

Fixes: dotnet#1437
davkean added a commit to davkean/project-system that referenced this issue Jul 9, 2019
IFileWatchDataSource is a new feature that was added to unify the logic of re-revaluating a project based on external files (such as .editorconfig and project.assets.json). Plug in <AdditionalDesignTimeBuildInput /> items, of which project.assets.json is the only one.

Fixes: dotnet#1437
davkean added a commit to davkean/project-system that referenced this issue Jul 9, 2019
IFileWatchDataSource is a new feature that was added to unify the logic of re-revaluating a project based on external files (such as .editorconfig and project.assets.json). Plug in <AdditionalDesignTimeBuildInput /> items, of which project.assets.json is the only one.

Fixes: dotnet#1437
davkean added a commit to davkean/project-system that referenced this issue Jul 9, 2019
IFileWatchDataSource is a new feature that was added to unify the logic of re-revaluating a project based on external files (such as .editorconfig and project.assets.json). Plug in <AdditionalDesignTimeBuildInput /> items, of which project.assets.json is the only one.

Fixes: dotnet#1437
@drewnoakes drewnoakes removed the Bug label Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature-NuGet NuGet integration including pushing it properties, project and package references, and Pack support. Triage-Approved Reviewed and prioritized
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants