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

deps file is not regenerated when non-nuget references change #1942

Closed
nguerrera opened this issue Feb 8, 2018 · 8 comments
Closed

deps file is not regenerated when non-nuget references change #1942

nguerrera opened this issue Feb 8, 2018 · 8 comments
Assignees
Labels
Milestone

Comments

@nguerrera
Copy link
Contributor

Currently the target that generates the deps file only runs when project.assets.json is older than deps.json, but if non-package references are changed, then the deps file would also need to change.

We do want to keep incrementality because it is an expensive operation, but we should account for the other task parameters that can change the deps file.

@livarcocc
Copy link
Contributor

Is this something that we should/can address for 2.1? I ask so that I can figure out which milestone to put this at.

Also, where is the code for this? I saw the GenerateDepsFilei is set to true for core or for PreserverCompilationContext == true.

Is this in the task?

@nguerrera
Copy link
Contributor Author

Code is here:

Inputs="$(ProjectAssetsFile)"
Outputs="$(ProjectDepsFilePath)">

This only input is the assets file.

As far as milestone, since I found this by code inspection, and haven't seen a report yet, it can maybe wait until after 2.1.3xx.

@livarcocc
Copy link
Contributor

I missed the inputs, only looked at condition. All right. Marking for a future release then.

@nguerrera
Copy link
Contributor Author

nguerrera commented Feb 9, 2018

Cheap fix is adding MSBuildAllProjects to inputs. It isn't perfect, but many other targets use the same approximation for incrementality. We should maybe just do that. Otherwise, need to do fancier tricks like CoreCompile and GenerateAssemblyInfo do.

@livarcocc
Copy link
Contributor

I like simple fixes.

@ericstj
Copy link
Member

ericstj commented Feb 9, 2018

Do you gather anything more than just TargetPath from project references? (EG: copy local content, or binaries referenced by a ProjectReference) If so, that complicates things since those wouldn't be covered by MSBuildAllProjects of this project. If not then its probably OK.

@dsplaisted dsplaisted self-assigned this Mar 26, 2018
@dsplaisted dsplaisted modified the milestones: 2.3.0, 2.1.3xx Mar 26, 2018
@nguerrera
Copy link
Contributor Author

Do you gather anything more than just TargetPath from project references? (EG: copy local content, or binaries referenced by a ProjectReference) If so, that complicates things since those wouldn't be covered by MSBuildAllProjects of this project. If not then its probably OK.

@tannergooding is looking at gathering the reference dependencies.

@dsplaisted
Copy link
Member

I will likely include the simple fix for this (add $(MSBuildAllProjects) to the target inputs) together with the fix for #1847

dsplaisted added a commit to dsplaisted/sdk that referenced this issue Mar 27, 2018
dsplaisted added a commit to dsplaisted/sdk that referenced this issue Apr 4, 2018
dsplaisted added a commit to dsplaisted/sdk that referenced this issue Apr 9, 2018
dsplaisted added a commit to dsplaisted/sdk that referenced this issue Apr 13, 2018
dsplaisted added a commit to dsplaisted/sdk that referenced this issue Apr 18, 2018
dsplaisted added a commit to dsplaisted/sdk that referenced this issue Apr 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants