fix for #2755 #2770
fix for #2755 #2770
Conversation
| @@ -670,7 678,7 @@ let DownloadAndExtractPackage(alternativeProjectRoot, root, isLocalOverride:bool | |||
| use _ = Profile.startCategory Profile.Category.FileIO | |||
| let parent = Path.GetDirectoryName targetFileName | |||
| if not (Directory.Exists parent) then Directory.CreateDirectory parent |> ignore | |||
| File.Copy(nupkg.FullName,targetFileName) | |||
| File.Copy(nupkg.FullName,targetFileName,true) | |||
There was a problem hiding this comment.
should we leave the last parameter to false when no path is resolved (ie 'storage: none')?
There was a problem hiding this comment.
I'm not sure i understand how storage: none should work.
If we set File.Copy(..., overwrite) parameter to false Paket will just fail if file already exists and normally copy it otherwise.
In branch where source is not LocalNuGet we overwrite file with File.Create without any conditions. (NuGet.fs:757).
There was a problem hiding this comment.
The only thing we need to watch out for is that we never write into the cache (~/.nuget) when a local override is specified, this is more than invalid and leads to hard do diagnose errors in combination with NuGet. So we just need to ensure that this cannot ever happen. This mistake is easily introduced now, because storage: none means we use ~/.nuget instead of the packages directory.
This is what I'm a bit worried about.
I think this cannot happen, because we "throw" on the caller side of this function if force and storage: none are given the same time. storage: none basically is given when configResolved.Path is None. I suggest we check and throw in this function as well when we want to fix the bug like this.
I hope this makes sense. Thanks for taking care of this.
There was a problem hiding this comment.
Yes, thanks, i now understand the problem.
I can change commit - add check in download function, but such check already in 50 lines above - NuGet.fs:629 and download can't be called if this check fails.
We already have a test that assert that "local" package will not be copied to cache - "#1633 paket.local local source override"
So, maybe there is no need for additional check?
try to fix #2755
this solve two problems: overriding .nupkg on force restore and deal with .nupkg naming problem (normalized name from
~/.nugetvs "as is" from overrided source)~/.nugetwhen usingLocalNuGetsource