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

Paket should not overwrite changed content files #202

Closed
agross opened this issue Sep 30, 2014 · 26 comments
Closed

Paket should not overwrite changed content files #202

agross opened this issue Sep 30, 2014 · 26 comments
Labels

Comments

@agross
Copy link
Contributor

agross commented Sep 30, 2014

NuGet won't restore content files because the user is expected to edit them. Or remove a subset of content files from the project. Or some other reason outlined in the post above.

The reasoning behind it is a bit unclear, especially since it allows you to get your dependencies into an inconsistent state:

PM> Install-Package jQuery -Version 2.1.0

# edit Scripts/jquery-2.1.0.js

PM> Update-Package jQuery
Updating 'jQuery' from version '2.1.0' to '2.1.1' in project 'ClassLibrary1'.
Removing 'jQuery 2.1.0' from ClassLibrary1.
Skipping 'Scripts\jquery-2.1.0.js' because it was modified.
...

This will leave you with an inconsistent set of jQuery files: the modified jquery-2.1.0.js stays in place, but you get an extra jquery-2.1.1.js. All other jQuery 2.1.0 files (minified, map) will be replaced with their jQuery 2.1.1 counterparts.

What are our options when we detect modified content files?

  • Rebuild NuGet's behavior, possibly allowing inconsistencies
  • Fail
  • ?

Should do the same and install content just once and then leave them untouched? If we opt to always install content files, this would essentially remove the possibility to have the user modify them.

@ilkerde
Copy link
Contributor

ilkerde commented Oct 3, 2014

I have a very strict opinion here. I'd like Paket to fail on update when edits are detected. That is, content should not be editable at all if they are managed through a paket dependency. If any edit happens, Paket will fail.

For the sake of user friendliness:

  • paket update --force may ignore edits, wipe the old stuff and update the package
  • paket outdated may detect user edits and warn user about his dangerous path

@isaacabraham
Copy link
Contributor

+1 @ilkerde. When I started work on the source file stuff, I was considered marking the files as read-only or something - any files that are pulled down i.e. source file or content should be considered read-only IMO.

@Ravadre
Copy link

Ravadre commented Oct 14, 2014

+1 @ilkerde I just realised after converting some of my projects to Paket that all my NLog.config settings were wiped, even update'ing packages when there are no updates to be made wipes config clean (if you reference NLog.Configuration).

UX wise, I would also propose:

  • packet update --interactive to ask for every content file to overwrite, if files are different.

@mexx
Copy link
Member

mexx commented Oct 15, 2014

👍 @ilkerde

content should not be editable at all

If it were, it would be as if you edit the library provided through nuget.
For me the content files are libraries just in an other format.

@tpluscode
Copy link
Contributor

In many packages, content files are actually intended for modification. NLog.Configuration @Ravadre metions is a good example. Anoter one, which I struggle with, is Fody. Especially that the nature of Fody is to probably be added to every single project in a solution. And then any Paket operation overwrites the configuration content file. Very annoying.

As per solution, NuGet asks for each content file that has been modified before overwriting. Maybe that is an acceptable workaround?

@forki
Copy link
Member

forki commented Nov 18, 2014

I don't think that's a good solution.
We need to find a way to distinguish between these one time files and real content files.

@tpluscode
Copy link
Contributor

@forki what do you mean "one time files"?

@forki
Copy link
Member

forki commented Nov 18, 2014

there are packages that install config files for you. These files are meant to be modified.
Most content files are not.

@tpluscode
Copy link
Contributor

I agree, most content files are not meant to be modified. And thus most will not. Why do you think that asking to confirm overwriting is not an option? I guess that answering that question will still be less annoying that having to go through changes every time a package is installed.
I don't think that there actually is a way to separate such files from the rest. Well, other than manually maintaining a list.

@forki
Copy link
Member

forki commented Nov 18, 2014

asking every single time on every update is more annoying than maintaing such a list.

@forki
Copy link
Member

forki commented Nov 18, 2014

@theimowski how about a special syntax in paket.references to mark files that will not be overwritten?

@Ravadre
Copy link

Ravadre commented Nov 18, 2014

@forki This what you wrote or even maybe just a separate config file? Something like .gitignore - files listed are "ignored" and therefore protected from overwriting.

Probably an option to "force" overwrite and ignore such config / directives in paket.references could be handy, so that if you would like to get vanilla content files you would not need to modify paket.references and remove this special syntax marks

@tpluscode
Copy link
Contributor

@forki paket.references are only per project right? Maybe paket.dependencies would be a better place for that. So that it's possible to protect selected files in every project where a package is installed. For example

nuget NLog.Configuration
   protect:
      NLog.config

@theimowski
Copy link
Member

https://twitter.com/PaketManager/status/519064737695678464 - based on this, I would vote for not implementing such a feature.
Had a look at NLog.Configuration package - it looks like all it has is one <1KB file, which is just a bootstrap for configuration. What's are the pros of keeping such file under a package manager?

@Ravadre
Copy link

Ravadre commented Nov 18, 2014

@theimowski disclaimer: I am not an author of NLog and I also would not like to defend such packet layout. In my previous posts I gave it as an example of package that is used and which might be problematic with current Paket behavior.

NLog ships in 3 packages basically, library, schema and config - which just wraps library + schema + sample config file. Installing the config package gives you NLog + quick bootstrap for config + schema (which is much bigger than empty config file) which gives you intellisense for configuration.

Why is it useful to have this under a package manager? Because you don't have to keep such prepared config + schema for it on local drive and copy it every time you want to install and configure NLog (with intellisense).

If you want intellisense + bootstrapped config right now with Paket you need to install NLog.Config, remove it and install NLog (core) so that Paket will not touch your config (or schema in case you would like to edit it, however - most likely you don't so this doesn't matter).

My personal view: I can imagine more packages doing similar thing - using content with text files as poor's-man bootstrapping (which is better idea than running powershell scripts, I think we agree on that? :-) ). Based on this I think an option to not forcefully overwrite such files is a viable.

@tpluscode
Copy link
Contributor

I mentioned Fody earlier. It adds an XML file to projects, which is then used to configure the code weaving plugins. The plugins are also NuGet packages. There were well over 60 last time I checked.

The problem is that if a solution uses Fody, it will probably have at least one plugin in every project. In my case I usually have NullGuard everywhere to inject null checks during build. With Paket I have to revert changes to the overwritten config files every time I add another package etc. This would mean tens of files in moderate sized solution. So I fix it and then later update some package: uh oh the same all over again.

@theimowski
Copy link
Member

Ok, so I understand there are valid cases when a package contains more than just that single file, which is supposed to be modified. Whitelisting such files sounds like a reasonable solution here, however not sure if we want that on solution or project level.

@tpluscode
Copy link
Contributor

I would go for both. Or project if we wer to decide on only one way. Solution maybe makes more sense but would be less flexible.

@theimowski
Copy link
Member

@forki what you think?

@dnauck
Copy link
Contributor

dnauck commented Feb 11, 2015

+1

@forki forki closed this as completed Aug 13, 2015
@304NotModified
Copy link
Contributor

304NotModified commented Nov 9, 2016

Is this ever implemented?

(Google takes me to this Github issue)

Is this content: none in the paket.dependencies file?

@agross
Copy link
Contributor Author

agross commented Nov 9, 2016

I think you can specify a mode for content files now.

nuget jQuery content: none // we don't install jQuery content files
nuget Fody content: once // install content files but don't overwrite
nuget ServiceStack.Swagger content: true // install content and always override nuget

From: http://fsprojects.github.io/Paket/nuget-dependencies.html

⁣Beste Grüße,

​Alex

http://therightstuff.de/
Tiny phone, tiny mail​

On Nov 9, 2016, 23:51, at 23:51, Julian Verdurmen notifications@github.com wrote:

Is this ever implemented?

You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
#202 (comment)

@304NotModified
Copy link
Contributor

304NotModified commented Nov 9, 2016

Cool! Could we add that to the "Converting from NuGet" and "FAQ" pages?

@agross
Copy link
Contributor Author

agross commented Nov 9, 2016

You can make it happen by creating an issue. Submit a pull request if you like, it would be greatly appreciated!

@304NotModified
Copy link
Contributor

It's just to the gh-pages branch isn't? The content is updated by hand or with a tool?

@agross
Copy link
Contributor Author

agross commented Nov 9, 2016

See the docs directory. It's markdown that is used to create the documentation.

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

10 participants