NuGet should have a mechanism for getting the path to the lock file in MSBuild #3351

Closed
eerhardt opened this Issue Aug 23, 2016 · 9 comments

Comments

Projects
None yet
6 participants
@eerhardt

When a build task needs to consume the lock file (project.assets.json), it needs to calculate the path to the lock file in order to read it. However, calculating the path isn't trivial, and the algorithm may change in the future.

To fix this, we should have some sort common MSBuild $(Property) or <Target> that is responsible for calculating the path to the lock file. That way all consuming tasks can use this property/target instead of having to calculate the path themselves.

/cc @emgarten @rrelyea @yishaigalatzer

@rrelyea rrelyea added this to the 3.6 Beta1 milestone Aug 24, 2016

@yishaigalatzer

This comment has been minimized.

Show comment
Hide comment
@yishaigalatzer

yishaigalatzer Sep 1, 2016

I see it differently, the current design should eliminate all readers of the assets file, and there should be a single task reading it and translating it to msbuild language.

From: Eric Erhardt notifications@github.com
Reply-To: NuGet/Home reply@reply.github.com
Date: Monday, August 22, 2016 at 5:33 PM
To: NuGet/Home Home@noreply.github.com
Cc: Yishai Galatzer yigalatz@microsoft.com, Mention mention@noreply.github.com
Subject: [NuGet/Home] NuGet should have a mechanism for getting the path to the lock file in MSBuild (#3351)

When a build task needs to consume the lock file (project.assets.json), it needs to calculate the path to the lock file in order to read it. However, calculating the path isn't trivial, and the algorithm may change in the future.

To fix this, we should have some sort common MSBuild $(Property) or that is responsible for calculating the path to the lock file. That way all consuming tasks can use this property/target instead of having to calculate the path themselves.

/cc @emgartenhttps://github.com/emgarten @rrelyeahttps://github.com/rrelyea @yishaigalatzerhttps://github.com/yishaigalatzer


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com/NuGet/Home/issues/3351, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ABLmt25KRFfozOmyifV8jWV8rbdeDBMnks5qij_rgaJpZM4JqcPx.

I see it differently, the current design should eliminate all readers of the assets file, and there should be a single task reading it and translating it to msbuild language.

From: Eric Erhardt notifications@github.com
Reply-To: NuGet/Home reply@reply.github.com
Date: Monday, August 22, 2016 at 5:33 PM
To: NuGet/Home Home@noreply.github.com
Cc: Yishai Galatzer yigalatz@microsoft.com, Mention mention@noreply.github.com
Subject: [NuGet/Home] NuGet should have a mechanism for getting the path to the lock file in MSBuild (#3351)

When a build task needs to consume the lock file (project.assets.json), it needs to calculate the path to the lock file in order to read it. However, calculating the path isn't trivial, and the algorithm may change in the future.

To fix this, we should have some sort common MSBuild $(Property) or that is responsible for calculating the path to the lock file. That way all consuming tasks can use this property/target instead of having to calculate the path themselves.

/cc @emgartenhttps://github.com/emgarten @rrelyeahttps://github.com/rrelyea @yishaigalatzerhttps://github.com/yishaigalatzer


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com/NuGet/Home/issues/3351, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ABLmt25KRFfozOmyifV8jWV8rbdeDBMnks5qij_rgaJpZM4JqcPx.

@eerhardt

This comment has been minimized.

Show comment
Hide comment
@eerhardt

eerhardt Sep 1, 2016

I don't agree because "msbuild language" doesn't represent graphs of objects as well as .NET does.

It is orders of magnitudes easier to consume a .NET object model as a graph than it is a bunch of string-string metadata items. You also lose all sorts of type safety, intellisense, etc. when consuming "msbuild language".

Thus there are certain classes of tasks that are better written consuming the assets file using the object model https://github.com/NuGet/NuGet.Client/tree/dev/src/NuGet.Core/NuGet.ProjectModel/LockFile.

eerhardt commented Sep 1, 2016

I don't agree because "msbuild language" doesn't represent graphs of objects as well as .NET does.

It is orders of magnitudes easier to consume a .NET object model as a graph than it is a bunch of string-string metadata items. You also lose all sorts of type safety, intellisense, etc. when consuming "msbuild language".

Thus there are certain classes of tasks that are better written consuming the assets file using the object model https://github.com/NuGet/NuGet.Client/tree/dev/src/NuGet.Core/NuGet.ProjectModel/LockFile.

@yishaigalatzer

This comment has been minimized.

Show comment
Hide comment
@yishaigalatzer

yishaigalatzer Sep 1, 2016

Let me re-iterate/clarify the stance, there will be a single point in msbuild that can load a lock file, there will be no guarantees to the shape of the lock file or the location other than that one entry point can load it.

From: Eric Erhardt notifications@github.com
Reply-To: NuGet/Home reply@reply.github.com
Date: Thursday, September 1, 2016 at 1:08 PM
To: NuGet/Home Home@noreply.github.com
Cc: Yishai Galatzer yigalatz@microsoft.com, Mention mention@noreply.github.com
Subject: Re: [NuGet/Home] NuGet should have a mechanism for getting the path to the lock file in MSBuild (#3351)

I don't agree because "msbuild language" doesn't represent graphs of objects as well as .NET does.

It is orders of magnitudes easier to consume a .NET object model as a graph than it is a bunch of string-string metadata items. You also lose all sorts of type safety, intellisense, etc. when consuming "msbuild language".

Thus there are certain classes of tasks that are better written consuming the assets file using the object model https://github.com/NuGet/NuGet.Client/tree/dev/src/NuGet.Core/NuGet.ProjectModel/LockFile.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com/NuGet/Home/issues/3351#issuecomment-244196410, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ABLmt87xxIvx2uERr9_5mOc7an8aloQEks5qlzCjgaJpZM4JqcPx.

Let me re-iterate/clarify the stance, there will be a single point in msbuild that can load a lock file, there will be no guarantees to the shape of the lock file or the location other than that one entry point can load it.

From: Eric Erhardt notifications@github.com
Reply-To: NuGet/Home reply@reply.github.com
Date: Thursday, September 1, 2016 at 1:08 PM
To: NuGet/Home Home@noreply.github.com
Cc: Yishai Galatzer yigalatz@microsoft.com, Mention mention@noreply.github.com
Subject: Re: [NuGet/Home] NuGet should have a mechanism for getting the path to the lock file in MSBuild (#3351)

I don't agree because "msbuild language" doesn't represent graphs of objects as well as .NET does.

It is orders of magnitudes easier to consume a .NET object model as a graph than it is a bunch of string-string metadata items. You also lose all sorts of type safety, intellisense, etc. when consuming "msbuild language".

Thus there are certain classes of tasks that are better written consuming the assets file using the object model https://github.com/NuGet/NuGet.Client/tree/dev/src/NuGet.Core/NuGet.ProjectModel/LockFile.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com/NuGet/Home/issues/3351#issuecomment-244196410, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ABLmt87xxIvx2uERr9_5mOc7an8aloQEks5qlzCjgaJpZM4JqcPx.

@livarcocc

This comment has been minimized.

Show comment
Hide comment
@livarcocc

livarcocc Sep 7, 2016

@yishaigalatzer the CLI reads the lock file to find the tools available for the project when resolving tools commands. We will then use the tool's lock file to generate its deps file. So, we use it twice.

@yishaigalatzer the CLI reads the lock file to find the tools available for the project when resolving tools commands. We will then use the tool's lock file to generate its deps file. So, we use it twice.

@yishaigalatzer

This comment has been minimized.

Show comment
Hide comment
@yishaigalatzer

yishaigalatzer Sep 7, 2016

We should really get rid of that and go through a single channel in the msbuild task if possible and not produce multiple code paths that understand the lock file. IMHO the lock file is an implementation detail of NuGet and not something we should have partners take a dependency on.

By doing so we will be able to raise our confidence when making changes (even if additive) to the file.

I suggest getting in a room with David Kean and us if you still think this is not doable.

We should really get rid of that and go through a single channel in the msbuild task if possible and not produce multiple code paths that understand the lock file. IMHO the lock file is an implementation detail of NuGet and not something we should have partners take a dependency on.

By doing so we will be able to raise our confidence when making changes (even if additive) to the file.

I suggest getting in a room with David Kean and us if you still think this is not doable.

@brthor

This comment has been minimized.

Show comment
Hide comment
@brthor

brthor Sep 7, 2016

Member

Assuming we keep the tool resolution and execution in c#, we need to be able to consume something that describes the dependency graph of the tool so we can find the appropriate assembly to execute.

Today, that is the lock file through the LockFile model.
Is this task becoming the thing we should consume to find the dependency graph, and other lock file information (tool groups for example)?

Member

brthor commented Sep 7, 2016

Assuming we keep the tool resolution and execution in c#, we need to be able to consume something that describes the dependency graph of the tool so we can find the appropriate assembly to execute.

Today, that is the lock file through the LockFile model.
Is this task becoming the thing we should consume to find the dependency graph, and other lock file information (tool groups for example)?

@yishaigalatzer

This comment has been minimized.

Show comment
Hide comment
@yishaigalatzer

yishaigalatzer Sep 7, 2016

I'm saying we should strive for one single model, so yes that should be the goal

I'm saying we should strive for one single model, so yes that should be the goal

@rrelyea rrelyea modified the milestones: 3.6 Beta1, 3.6 Beta2 Sep 22, 2016

@rrelyea

This comment has been minimized.

Show comment
Hide comment
@rrelyea

rrelyea Sep 23, 2016

Contributor

@emgarten - we'll move to the right msbuild property name in rc. currently we look at $(intermediateobjectpath) I believe.

Contributor

rrelyea commented Sep 23, 2016

@emgarten - we'll move to the right msbuild property name in rc. currently we look at $(intermediateobjectpath) I believe.

@rrelyea rrelyea modified the milestones: 3.6 RC, 3.6 Beta2 Sep 23, 2016

@rrelyea rrelyea added the Priority:1 label Sep 23, 2016

@emgarten emgarten modified the milestones: 4.0 RTM, Future-1 Feb 14, 2017

@emgarten

This comment has been minimized.

Show comment
Hide comment
@emgarten

emgarten Feb 14, 2017

Contributor

The path to project.assets.json is now written out under ProjectAssetsFile to the .props file.

Contributor

emgarten commented Feb 14, 2017

The path to project.assets.json is now written out under ProjectAssetsFile to the .props file.

@emgarten emgarten closed this Feb 14, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment