Skip to content

Commit

Permalink
add 'PackageInfo' to fallback to group settings automatically (for al…
Browse files Browse the repository at this point in the history
…l 'higher' APIs). Fix bug in garbage collector.
  • Loading branch information
matthid committed Aug 20, 2017
1 parent 2a681b8 commit 926a246
Show file tree
Hide file tree
Showing 14 changed files with 75 additions and 38 deletions.
7 changes: 4 additions & 3 deletions src/Paket.Core/Dependencies/CacheExtensions.fs
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@ module CacheExtensions =
let nuspec = Path.Combine(folder,sprintf "%O.nuspec" name)
Nuspec.Load nuspec

type PackageResolver.ResolvedPackage with
type PackageResolver.PackageInfo with
member x.Folder root groupName =
let includeVersion = defaultArg x.Settings.IncludeVersionInPath false
let storageConf = defaultArg x.Settings.StorageConfig PackagesFolderGroupConfig.Default
let settings = x.Settings
let includeVersion = defaultArg settings.IncludeVersionInPath false
let storageConf = defaultArg settings.StorageConfig PackagesFolderGroupConfig.Default
match (storageConf.Resolve root groupName x.Name x.Version includeVersion).Path with
| Some f -> f
| None ->
Expand Down
28 changes: 28 additions & 0 deletions src/Paket.Core/Dependencies/PackageResolver.fs
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,16 @@ module Resolution =
| ResolutionRaw.ConflictRaw conf -> Conflict conf
let Ok resolution =
Resolution.ofRaw [] (ResolutionRaw.OkRaw resolution)

let fixPackageSettings groupSettings (r:Resolution) =
{ r with
Raw =
match r.Raw with
| ResolutionRaw.OkRaw model ->
model
|> Map.map (fun k v -> { v with Settings = v.Settings + groupSettings } )
|> ResolutionRaw.OkRaw
| conf -> conf }
type Resolution with

member self.GetConflicts () = Resolution.getConflicts self
Expand Down Expand Up @@ -1316,6 +1326,7 @@ let Resolve (getVersionsRaw, getPreferredVersionsRaw, getPackageDetailsRaw, grou

exceptionThrown <- false
resolution
//|> fixPackageSettings
finally
// some cleanup
cts.Cancel()
Expand All @@ -1340,3 +1351,20 @@ let Resolve (getVersionsRaw, getPreferredVersionsRaw, getPackageDetailsRaw, grou
else reraise()
| e when exceptionThrown ->
traceErrorfn "Error while waiting for worker to finish: %O" e


type PackageInfo =
{ Resolved : ResolvedPackage
GroupSettings : InstallSettings
Settings : InstallSettings }
member x.Name = x.Resolved.Name
member x.Version = x.Resolved.Version
member x.Dependencies = x.Resolved.Dependencies
member x.Unlisted = x.Resolved.Unlisted
member x.IsRuntimeDependency = x.Resolved.IsRuntimeDependency
member x.IsCliTool = x.Resolved.IsCliTool
member x.Source = x.Resolved.Source
static member from v s =
{ Resolved = v
GroupSettings = s
Settings = v.Settings + s }
12 changes: 6 additions & 6 deletions src/Paket.Core/Installation/DependencyChangeDetection.fs
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ type DependencyChangeType =

let findNuGetChangesInDependenciesFile(dependenciesFile:DependenciesFile,lockFile:LockFile,strict) =
let allTransitives groupName = lockFile.GetTransitiveDependencies groupName
let getChanges groupName transitives (newRequirement:PackageRequirement) (originalPackage:ResolvedPackage) =
let getChanges groupName transitives (newRequirement:PackageRequirement) (originalPackage:PackageInfo) =
let settingsChanged() =
if newRequirement.Settings <> originalPackage.Settings then
if newRequirement.Settings = { originalPackage.Settings with FrameworkRestrictions = AutoDetectFramework } then
if newRequirement.Settings = { originalPackage.Resolved.Settings with FrameworkRestrictions = AutoDetectFramework } then

This comment has been minimized.

Copy link
@lust4life

lust4life Dec 22, 2018

Contributor

@matthid @forki hi, seems we need to compare with { originalPackage.Settings with FrameworkRestrictions = AutoDetectFramework } here not originalPackage.Resolved.Settings ? Do you remember why change this behavior ?

This comment has been minimized.

Copy link
@matthid

matthid Dec 22, 2018

Author Member

apparently this was a bugfix. I don't recall right now what the difference between originalPackage.Settings and Resolved.Settings is.
Can you send a pr and/or create a testcase on the changed behavior?

[]
elif newRequirement.Settings.FrameworkRestrictions <> originalPackage.Settings.FrameworkRestrictions then
let isTransitive = transitives |> Seq.contains originalPackage.Name
Expand Down Expand Up @@ -57,11 +57,11 @@ let findNuGetChangesInDependenciesFile(dependenciesFile:DependenciesFile,lockFil
match lockFileGroup with
| None -> [GroupNotFoundInLockFile]
| Some group ->
match group.Resolution.TryFind name with
match group.TryFind name with
| Some lockFilePackage ->
getChanges groupName transitives
{ dependenciesFilePackage with Settings = depsGroup.Options.Settings + dependenciesFilePackage.Settings }
{ lockFilePackage with Settings = group.Options.Settings + lockFilePackage.Settings }
lockFilePackage
| _ -> [PackageNotFoundInLockFile])
|> Seq.filter (fun (_,_, changes) -> changes.Length > 0)
|> Seq.map (fun (p,_, changes) -> groupName, p, changes)
Expand All @@ -76,12 +76,12 @@ let findNuGetChangesInDependenciesFile(dependenciesFile:DependenciesFile,lockFil
|> Seq.map (fun d -> d.Name,{ d with Settings = group.Options.Settings + d.Settings })
|> Map.ofSeq

[for t in lockFile.GetTopLevelDependencies(groupName) do
[for t in lockFile.GetTopLevelDependencies(groupName) do
let name = t.Key
match directMap.TryFind name with
| Some pr ->
let t = t.Value
let t = { t with Settings = lockFile.GetGroup(groupName).Options.Settings + t.Settings }
//let t = { t with Settings = lockFile.GetGroup(groupName).Options.Settings + t.Settings }
yield groupName, name, getChanges groupName transitives pr t // Modified
| _ -> yield groupName, name, [PackageNotFoundInDependenciesFile] // Removed
]
Expand Down
6 changes: 4 additions & 2 deletions src/Paket.Core/Installation/GarbageCollection.fs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ let deleteUnusedPackages (lockFile:LockFile) =
let defaultStorage = defaultArg g.Value.Options.Settings.StorageConfig PackagesFolderGroupConfig.Default
g.Value.Resolution
|> Seq.map (fun r -> defaultArg r.Value.Settings.StorageConfig defaultStorage)
|> Seq.append [defaultStorage]
|> Seq.append [defaultStorage; PackagesFolderGroupConfig.DefaultPackagesFolder]
|> Seq.map (fun storageOption -> groupName, storageOption))
// always consider default packages folder for GC
|> Seq.append [Constants.MainDependencyGroup, PackagesFolderGroupConfig.DefaultPackagesFolder]
Expand Down Expand Up @@ -77,7 +77,9 @@ let deleteUnusedPackages (lockFile:LockFile) =
else
// might be in the resultion, but the storage path changed
let group = lockFile.Groups.[package.GroupName]
let packageStorage = defaultArg group.Resolution.[package.PackageName].Settings.StorageConfig PackagesFolderGroupConfig.Default
let packageInfo = group.GetPackage package.PackageName
let storageConfig = packageInfo.Settings.StorageConfig
let packageStorage = defaultArg storageConfig PackagesFolderGroupConfig.Default
let resolvePack = packageStorage.ResolveGroupDir lockFile.RootPath package.GroupName
if groupDir <> resolvePack then
tracefn "Garbage collecting %O" package.Path
Expand Down
2 changes: 1 addition & 1 deletion src/Paket.Core/Installation/InstallProcess.fs
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ let CreateModel(alternativeProjectRoot, root, force, dependenciesFile:Dependenci
let caches = dependenciesFile.Groups.[kv'.Key].Caches
kv'.Value.Resolution
|> Map.filter (fun name _ -> packages.Contains(kv'.Key,name))
|> Seq.map (fun kv -> RestoreProcess.CreateInstallModel(alternativeProjectRoot, root,kv'.Key,sources,caches,force,kv.Value))
|> Seq.map (fun kv -> RestoreProcess.CreateInstallModel(alternativeProjectRoot, root,kv'.Key,sources,caches,force,kv'.Value.GetPackage kv.Key))
|> Seq.toArray
|> Async.Parallel
|> Async.RunSynchronously)
Expand Down
12 changes: 6 additions & 6 deletions src/Paket.Core/Installation/RestoreProcess.fs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@ let FindPackagesNotExtractedYet(dependenciesFileName) =
lockFile.GetGroupedResolution()
|> Map.toList
|> List.filter (fun ((group,package),resolved) ->
let groupSetting = defaultArg lockFile.Groups.[group].Options.Settings.StorageConfig PackagesFolderGroupConfig.Default
let packSetting = defaultArg resolved.Settings.StorageConfig groupSetting
let packSetting = defaultArg resolved.Settings.StorageConfig PackagesFolderGroupConfig.Default
let includeVersionInPath = defaultArg resolved.Settings.IncludeVersionInPath false
let resolvedPath = packSetting.Resolve root group package resolved.Version includeVersionInPath
NuGetCache.IsPackageVersionExtracted(resolvedPath, package, resolved.Version) |> not)
Expand All @@ -40,7 +39,7 @@ let CopyToCaches force caches fileName =
traceWarnfn "Could not copy %s to cache %s%s%s" fileName cache.Location Environment.NewLine exn.Message)

/// returns - package, libs files, props files, targets files, analyzers files
let private extractPackage caches package alternativeProjectRoot root source groupName version includeVersionInPath force =
let private extractPackage caches (package:PackageInfo) alternativeProjectRoot root source groupName version includeVersionInPath force =
let downloadAndExtract force detailed = async {
let cfg = defaultArg package.Settings.StorageConfig PackagesFolderGroupConfig.Default

Expand Down Expand Up @@ -69,7 +68,7 @@ let private extractPackage caches package alternativeProjectRoot root source gro

/// Downloads and extracts a package.
/// returns - package, libs files, props files, targets files, analyzers files
let ExtractPackage(alternativeProjectRoot, root, groupName, sources, caches, force, package : ResolvedPackage, localOverride) =
let ExtractPackage(alternativeProjectRoot, root, groupName, sources, caches, force, package : PackageInfo, localOverride) =
async {
let storage = defaultArg package.Settings.StorageConfig PackagesFolderGroupConfig.Default
let v = package.Version
Expand Down Expand Up @@ -143,10 +142,11 @@ let ExtractPackage(alternativeProjectRoot, root, groupName, sources, caches, for
let internal restore (alternativeProjectRoot, root, groupName, sources, caches, force, lockFile : LockFile, packages : Set<PackageName>, overriden : Set<PackageName>) =
async {
RemoteDownload.DownloadSourceFiles(Path.GetDirectoryName lockFile.FileName, groupName, force, lockFile.Groups.[groupName].RemoteFiles)
let group = lockFile.Groups.[groupName]
let! _ =
lockFile.Groups.[groupName].Resolution
|> Map.filter (fun name _ -> packages.Contains name)
|> Seq.map (fun kv -> ExtractPackage(alternativeProjectRoot, root, groupName, sources, caches, force, kv.Value, Set.contains kv.Key overriden))
|> Seq.map (fun kv -> ExtractPackage(alternativeProjectRoot, root, groupName, sources, caches, force, group.GetPackage kv.Key, Set.contains kv.Key overriden))
|> Async.Parallel
return ()
}
Expand Down Expand Up @@ -271,7 +271,7 @@ let createPaketCLIToolsFile (cliTools:ResolvedPackage seq) (fileInfo:FileInfo) =
if verbose then
tracefn " - %s already up-to-date" fileInfo.FullName

let createProjectReferencesFiles (dependenciesFile:DependenciesFile) (lockFile:LockFile) (projectFile:FileInfo) (referencesFile:ReferencesFile) (resolved:Lazy<Map<GroupName*PackageName,ResolvedPackage>>) targetFilter (groups:Map<GroupName,LockFileGroup>) =
let createProjectReferencesFiles (dependenciesFile:DependenciesFile) (lockFile:LockFile) (projectFile:FileInfo) (referencesFile:ReferencesFile) (resolved:Lazy<Map<GroupName*PackageName,PackageInfo>>) targetFilter (groups:Map<GroupName,LockFileGroup>) =
let list = System.Collections.Generic.List<_>()
let cliTools = System.Collections.Generic.List<_>()
for kv in groups do
Expand Down
4 changes: 2 additions & 2 deletions src/Paket.Core/Installation/ScriptGeneration.fs
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ module ScriptGeneration =
loadScriptsRootFolder </> folder </> fileName

// -- LOAD SCRIPT FORMATTING POINT --
let scriptFolder groupName (package: PackageResolver.ResolvedPackage) =
let scriptFolder groupName (package: PackageResolver.PackageInfo) =
let group = if groupName = Constants.MainDependencyGroup then String.Empty else (string groupName)
let framework = if isDefaultFramework then String.Empty else string framework
loadScriptsRootFolder </> framework </> group </> package.Name
Expand All @@ -188,7 +188,7 @@ module ScriptGeneration =
|> Seq.map (fun (groupName,packages) ->
// fold over a map constructing load scripts to ensure shared packages don't have their scripts duplicated
((Map.empty,[]),packages)
||> Seq.fold (fun ((knownIncludeScripts,scriptFiles): Map<_,string>*_) (package: PackageResolver.ResolvedPackage) ->
||> Seq.fold (fun ((knownIncludeScripts,scriptFiles): Map<_,string>*_) (package: PackageResolver.PackageInfo) ->

let scriptFolder = scriptFolder groupName package

Expand Down
4 changes: 2 additions & 2 deletions src/Paket.Core/PackageAnalysis/Queries.fs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ let getLockFileFromDependenciesFile dependenciesFileName =
let lockFileName = DependenciesFile.FindLockfile dependenciesFileName
LockFile.LoadFrom lockFileName.FullName

let listPackages (packages: System.Collections.Generic.KeyValuePair<GroupName*PackageName, PackageResolver.ResolvedPackage> seq) =
let listPackages (packages: System.Collections.Generic.KeyValuePair<GroupName*PackageName, PackageResolver.PackageInfo> seq) =
packages
|> Seq.map (fun kv ->
let groupName,packageName = kv.Key
Expand All @@ -23,7 +23,7 @@ let getInstalledPackageModel (lockFile: LockFile) (QualifiedPackageName(groupNam
match lockFile.Groups |> Map.tryFind groupName with
| None -> failwithf "Group %O can't be found in paket.lock." groupName
| Some group ->
match group.Resolution.TryFind(packageName) with
match group.TryFind(packageName) with
| None -> failwithf "Package %O is not installed in group %O." packageName groupName
| Some resolvedPackage ->
let packageName = resolvedPackage.Name
Expand Down
4 changes: 2 additions & 2 deletions src/Paket.Core/Paket.Core.fsproj
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@
<OutputPath>..\..\bin</OutputPath>
<DefineConstants>TRACE;DEBUG;USE_WEB_CLIENT_FOR_UPLOAD</DefineConstants>
<WarningLevel>3</WarningLevel>
<StartArguments>pack out out</StartArguments>
<StartArguments>update</StartArguments>
<StartAction>Project</StartAction>
<StartProgram>paket.exe</StartProgram>
<StartWorkingDirectory>
</StartWorkingDirectory>
<StartWorkingDirectory>C:\temp\PaketPackRepro3</StartWorkingDirectory>
<StartWorkingDirectory>C:\proj\testing\testpaketfailure\</StartWorkingDirectory>
</PropertyGroup>
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
<Optimize>true</Optimize>
Expand Down
4 changes: 2 additions & 2 deletions src/Paket.Core/PaketConfigFiles/DependencyCache.fs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ type DependencyCache (dependencyFile:DependenciesFile, lockFile:LockFile) =
let loadedGroups = HashSet<GroupName>()
let mutable nuspecCache = ConcurrentDictionary<PackageName*SemVerInfo, Nuspec>()
let mutable installModelCache = ConcurrentDictionary<GroupName * PackageName,InstallModel>()
let mutable orderedGroupCache = ConcurrentDictionary<GroupName,ResolvedPackage list>()
let mutable orderedGroupCache = ConcurrentDictionary<GroupName,PackageInfo list>()
let mutable orderedGroupReferences = ConcurrentDictionary<(GroupName * FrameworkIdentifier),ReferenceType list>()


Expand Down Expand Up @@ -64,7 +64,7 @@ type DependencyCache (dependencyFile:DependenciesFile, lockFile:LockFile) =

let getPackageOrderResolvedPackage =
getPackageOrderGeneric
(fun (p:PackageResolver.ResolvedPackage) -> p.Name)
(fun (p:PackageResolver.PackageInfo) -> p.Name)
(fun p -> p.Dependencies |> Seq.map (fun (n,_,_) -> n))


Expand Down
22 changes: 14 additions & 8 deletions src/Paket.Core/PaketConfigFiles/LockFile.fs
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,18 @@ open Paket.PackageSources
open Paket.Requirements
open Chessie.ErrorHandling

type LockFileGroup = {
Name: GroupName
type LockFileGroup =
{ Name: GroupName
Options:InstallOptions
Resolution:PackageResolution
RemoteFiles:ResolvedSourceFile list
}

RemoteFiles:ResolvedSourceFile list }
member x.GetPackage name =
PackageInfo.from x.Resolution.[name] x.Options.Settings
member x.TryFind name =
match x.Resolution.TryFind name with
| Some r ->
Some (PackageInfo.from r x.Options.Settings)
| None -> None
module LockFileSerializer =
/// [omit]
let serializeOptionsAsLines options = [
Expand Down Expand Up @@ -728,17 +733,18 @@ type LockFile (fileName:string, groups: Map<GroupName,LockFileGroup>) =

group.Resolution
|> Map.filter (fun name _ -> transitive.Contains name |> not)
|> Map.map (fun name v -> PackageInfo.from v group.Options.Settings)

member this.GetGroupedResolution () =
this.Groups
|> Seq.map (fun kv -> kv.Value.Resolution |> Seq.map (fun kv' -> (kv.Key,kv'.Key),kv'.Value))
|> Seq.map (fun kv -> kv.Value.Resolution |> Seq.map (fun kv' -> (kv.Key,kv'.Key),PackageInfo.from kv'.Value kv.Value.Options.Settings))
|> Seq.concat
|> Map.ofSeq


member this.GetResolvedPackages () =
groups |> Map.map (fun groupName lockGroup ->
lockGroup.Resolution |> Seq.map (fun x -> x.Value) |> List.ofSeq
lockGroup.Resolution |> Seq.map (fun x -> PackageInfo.from x.Value lockGroup.Options.Settings) |> List.ofSeq
)


Expand Down Expand Up @@ -944,7 +950,7 @@ type LockFile (fileName:string, groups: Map<GroupName,LockFileGroup>) =
match this.Groups |> Map.tryFind groupName with
| None -> failwithf "Group %O can't be found in paket.lock." groupName
| Some group ->
match group.Resolution.TryFind(packageName) with
match group.TryFind(packageName) with
| None -> failwithf "Package %O is not installed in group %O." packageName groupName
| Some resolvedPackage ->
let packageName = resolvedPackage.Name
Expand Down
2 changes: 1 addition & 1 deletion src/Paket.Core/PaketConfigFiles/RuntimeGraph.fs
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ module RuntimeGraph =
open System.IO
/// Downloads the given package into the nuget cache and read its runtime.json.
let getRuntimeGraphFromNugetCache root groupName (package:ResolvedPackage) =
let config = defaultArg package.Settings.StorageConfig PackagesFolderGroupConfig.Default
let config = PackagesFolderGroupConfig.NoPackagesFolder
// 1. downloading packages into cache
let targetFileName, _ =
NuGet.DownloadPackage (None, root, config, package.Source, [], groupName, package.Name, package.Version, package.IsCliTool, false, false, false)
Expand Down
4 changes: 2 additions & 2 deletions src/Paket.Core/PublicAPI.fs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ open Chessie.ErrorHandling

/// Paket API which is optimized for F# Interactive use.
type Dependencies(dependenciesFileName: string) =
let listPackages (packages: System.Collections.Generic.KeyValuePair<GroupName*PackageName, PackageResolver.ResolvedPackage> seq) =
let listPackages (packages: System.Collections.Generic.KeyValuePair<GroupName*PackageName, PackageResolver.PackageInfo> seq) =
packages
|> Seq.map (fun kv ->
let groupName,packageName = kv.Key
Expand Down Expand Up @@ -447,7 +447,7 @@ type Dependencies(dependenciesFileName: string) =
match this.GetLockFile().Groups |> Map.tryFind groupName with
| None -> failwithf "Group %O can't be found in paket.lock." groupName
| Some group ->
match group.Resolution.TryFind(packageName) with
match group.TryFind(packageName) with
| None -> failwithf "Package %O is not installed in group %O." packageName groupName
| Some resolvedPackage ->
let packageName = resolvedPackage.Name
Expand Down
2 changes: 1 addition & 1 deletion src/Paket/Paket.fsproj
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
<StartProgram>paket.exe</StartProgram>
<StartAction>Project</StartAction>
<StartArguments>update</StartArguments>
<StartWorkingDirectory>C:\temp\Library2</StartWorkingDirectory>
<StartWorkingDirectory>C:\proj\testing\testpaketfailure\</StartWorkingDirectory>
</PropertyGroup>
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
<Optimize>true</Optimize>
Expand Down

0 comments on commit 926a246

Please sign in to comment.