-
Notifications
You must be signed in to change notification settings - Fork 2
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
Making nuget install binaries to ${project}\libs #7
base: master
Are you sure you want to change the base?
Conversation
Specifying the destination of the exe file on install so that a pre-build step can become something like: `$(ProjectDir)libs\AutoImplement` without any extra work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, this is a breaking change... if this was submitted as part of 1.1.0, anyone who upgrades to 1.1.0 from 1.0.1 will no longer be able to find the DLL without changing their applications.
Breaking changes should wait until we're ready for version 2.0.0. So either this pull request needs to wait for that, or be closed and changed into an issue as a desired feature for version 2.0.0.
@@ -22,7 +22,7 @@ | |||
<copyright>Copyright 2018</copyright> | |||
</metadata> | |||
<files> | |||
<file src="artifacts\AutoImplement\bin\Release\AutoImplement.exe" target="" /> | |||
<file src="artifacts\AutoImplement\bin\Release\AutoImplement.exe" target="content\libs" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is content\libs
the best possible place for this? I would think that content\tools
might be more appropriate. Libs/lib is usually reserved for libraries that you want to reference (such as the nuget target="lib" used for System.Delegation). "Tools" is probably a better name for something called as a pre-build step that isn't actually referenced by the project.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only selected libs because that is the path that is currently suggested in the wiki. I agree that tools makes more sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. While updating the wiki, I went ahead and updated that path based on the current expected install scenario. I can update it again once this gets merged in.
So, current behavior is installing the nuget puts the exe under the nuget root (which is typically under the users home directory, but can be customized even on a per solution basis), and the dll's that need to be depended on under lib (which causes them to be added to the project as resources). I would not expect this to be a breaking change as people should not be installing a Nuget to a project, but then using a global (system) path to call the exe. (Albeit, it is different than the current behavior and I can understand waiting for 2.0 to accept the merge). |
I haven't done anything to change the default behavior on my machine. "Manage Nuget Packages" through VisualStudio installed the nugets into a folder called "packages" inside my solution directory. This is reflected by the Not sure why the default nuget install location is different from your machine to mine, but it does reinforce that (1) this change is required, to help provide consistency in how a project using AutoImplement finds the exe, and (2) this is a breaking change that needs to wait for a major version bump. |
Specifying the destination of the files on install so that a pre-build step can become something like:
$(ProjectDir)libs\AutoImplement
without any extra work.