From 51a2545550b7d453af96a29d28b7c527abbb30fd Mon Sep 17 00:00:00 2001 From: Steffen Forkmann Date: Sun, 30 Nov 2014 17:33:16 +0100 Subject: [PATCH 1/2] We only fix the package dependencies for non-related packages during selective UpdateProcess --- src/Paket.Core/LockFile.fs | 23 ++++++--- src/Paket.Core/UpdateProcess.fs | 13 +++-- .../Lockfile/GenerateAuthModeSpecs.fs | 2 +- .../GenerateContentNoneOptionSpecs.fs | 2 +- .../Lockfile/GenerateStrictModeSpecs.fs | 2 +- tests/Paket.Tests/Lockfile/QuerySpecs.fs | 50 +++++++++++++++++++ tests/Paket.Tests/Paket.Tests.fsproj | 3 +- 7 files changed, 78 insertions(+), 17 deletions(-) create mode 100644 tests/Paket.Tests/Lockfile/QuerySpecs.fs diff --git a/src/Paket.Core/LockFile.fs b/src/Paket.Core/LockFile.fs index 8e37b22797..aa5ad131e4 100644 --- a/src/Paket.Core/LockFile.fs +++ b/src/Paket.Core/LockFile.fs @@ -218,15 +218,22 @@ module LockFileParser = /// Allows to parse and analyze paket.lock files. type LockFile(fileName:string,options,resolution:PackageResolution,remoteFiles:ResolvedSourceFile list) = - let lowerCaseResolution = - resolution - |> Map.fold (fun resolution name p -> Map.add name p resolution) Map.empty member __.SourceFiles = remoteFiles member __.ResolvedPackages = resolution member __.FileName = fileName member __.Options = options + /// Checks if the first package is a dependency of the second package + member this.IsDependencyOf(dependentPackage,package) = + let start = NormalizedPackageName package + let target = NormalizedPackageName dependentPackage + let current = resolution.[start] + + if target = start then true else + current.Dependencies + |> Seq.exists (fun (d,_,_) -> this.IsDependencyOf(dependentPackage,d)) + /// Updates the Lock file with the analyzed dependencies from the paket.dependencies file. member __.Save() = let output = @@ -237,9 +244,13 @@ type LockFile(fileName:string,options,resolution:PackageResolution,remoteFiles:R File.WriteAllText(fileName, output) tracefn "Locked version resolutions written to %s" fileName - /// Parses a paket.lock file from lines + /// Parses a paket.lock file from file static member LoadFrom(lockFileName) : LockFile = - LockFileParser.Parse(File.ReadAllLines lockFileName) + LockFile.Parse(lockFileName, File.ReadAllLines lockFileName) + + /// Parses a paket.lock file from lines + static member Parse(lockFileName,lines) : LockFile = + LockFileParser.Parse lines |> fun state -> LockFile(lockFileName, state.Options ,state.Packages |> Seq.fold (fun map p -> Map.add (NormalizedPackageName p.Name) p map) Map.empty, List.rev state.SourceFiles) member this.GetPackageHull(referencesFile:ReferencesFile) = @@ -247,7 +258,7 @@ type LockFile(fileName:string,options,resolution:PackageResolution,remoteFiles:R let rec addPackage directly (packageName:PackageName) = let identity = NormalizedPackageName packageName - match lowerCaseResolution.TryFind identity with + match resolution.TryFind identity with | Some package -> if usedPackages.Add packageName then if not this.Options.Strict then diff --git a/src/Paket.Core/UpdateProcess.fs b/src/Paket.Core/UpdateProcess.fs index 9ae84bdf23..d3bd83d883 100644 --- a/src/Paket.Core/UpdateProcess.fs +++ b/src/Paket.Core/UpdateProcess.fs @@ -4,6 +4,7 @@ module Paket.UpdateProcess open Paket open System.IO open Paket.Domain +open Paket.PackageResolver /// Update command let Update(dependenciesFileName, forceResolution, force, hard) = @@ -26,14 +27,12 @@ let Update(dependenciesFileName, forceResolution, force, hard) = InstallProcess.Install(sources, force, hard, lockFile) let private fixOldDependencies (oldLockFile:LockFile) (dependenciesFile:DependenciesFile) (package:PackageName) = - let packageKeys = dependenciesFile.DirectDependencies |> Seq.map (fun kv -> NormalizedPackageName kv.Key) |> Set.ofSeq - oldLockFile.ResolvedPackages + oldLockFile.ResolvedPackages + |> Seq.map (fun kv -> kv.Value) + |> Seq.filter (fun p -> not <| oldLockFile.IsDependencyOf(p.Name,package)) |> Seq.fold - (fun (dependenciesFile : DependenciesFile) kv -> - let resolvedPackage = kv.Value - let name = NormalizedPackageName resolvedPackage.Name - if name = NormalizedPackageName package || not <| packageKeys.Contains name then dependenciesFile else - dependenciesFile.AddFixedPackage(resolvedPackage.Name, "= " + resolvedPackage.Version.ToString())) + (fun (dependenciesFile : DependenciesFile) resolvedPackage -> + dependenciesFile.AddFixedPackage(resolvedPackage.Name, "= " + resolvedPackage.Version.ToString())) dependenciesFile let updateWithModifiedDependenciesFile(dependenciesFile:DependenciesFile,packageName:PackageName, force) = diff --git a/tests/Paket.Tests/Lockfile/GenerateAuthModeSpecs.fs b/tests/Paket.Tests/Lockfile/GenerateAuthModeSpecs.fs index 34dec9c258..2abda0a9ca 100644 --- a/tests/Paket.Tests/Lockfile/GenerateAuthModeSpecs.fs +++ b/tests/Paket.Tests/Lockfile/GenerateAuthModeSpecs.fs @@ -1,4 +1,4 @@ -module paket.lockFile.GenerateAuthModeSpecs +module Paket.LockFile.GenerateAuthModeSpecs open Paket open NUnit.Framework diff --git a/tests/Paket.Tests/Lockfile/GenerateContentNoneOptionSpecs.fs b/tests/Paket.Tests/Lockfile/GenerateContentNoneOptionSpecs.fs index 6be3094c33..49d7570dfe 100644 --- a/tests/Paket.Tests/Lockfile/GenerateContentNoneOptionSpecs.fs +++ b/tests/Paket.Tests/Lockfile/GenerateContentNoneOptionSpecs.fs @@ -1,4 +1,4 @@ -module paket.lockFile.GenerateContentNoneOptionSpecs +module Paket.LockFile.GenerateContentNoneOptionSpecs open Paket open NUnit.Framework diff --git a/tests/Paket.Tests/Lockfile/GenerateStrictModeSpecs.fs b/tests/Paket.Tests/Lockfile/GenerateStrictModeSpecs.fs index c67d9f0e29..284887de55 100644 --- a/tests/Paket.Tests/Lockfile/GenerateStrictModeSpecs.fs +++ b/tests/Paket.Tests/Lockfile/GenerateStrictModeSpecs.fs @@ -1,4 +1,4 @@ -module paket.lockFile.GenerateStrictModeSpecs +module Paket.LockFile.GenerateStrictModeSpecs open Paket open NUnit.Framework diff --git a/tests/Paket.Tests/Lockfile/QuerySpecs.fs b/tests/Paket.Tests/Lockfile/QuerySpecs.fs new file mode 100644 index 0000000000..78034a7180 --- /dev/null +++ b/tests/Paket.Tests/Lockfile/QuerySpecs.fs @@ -0,0 +1,50 @@ +module Paket.LockFile.QuerySpecs + +open Paket +open NUnit.Framework +open FsUnit +open TestHelpers +open Paket.Domain + +let data = """NUGET + remote: https://nuget.org/api/v2 + specs: + Castle.Windsor (2.1) + Castle.Windsor-log4net (3.3) + Castle.Windsor (>= 2.0) + log4net (>= 1.0) + Rx-Core (2.1) + Rx-Main (2.0) + Rx-Core (>= 2.1) + log (1.2) + log4net (1.1) + log (>= 1.0) +GITHUB + remote: fsharp/FAKE + specs: + src/app/FAKE/Cli.fs (7699e40e335f3cc54ab382a8969253fecc1e08a9) + src/app/Fake.Deploy.Lib/FakeDeployAgentHelper.fs (Globbing) +""" + +let lockFile = LockFile.Parse("Test",toLines data) + + +[] +let ``should detect itself as dependency``() = + lockFile.IsDependencyOf(PackageName "Rx-Core",PackageName "Rx-Core") + |> shouldEqual true + +[] +let ``should detect direct dependencies``() = + lockFile.IsDependencyOf(PackageName "Rx-Core",PackageName "Rx-Main") + |> shouldEqual true + +[] +let ``should detect indirect dependencies``() = + lockFile.IsDependencyOf(PackageName "log",PackageName "Castle.Windsor-log4net") + |> shouldEqual true + +[] +let ``should detect when packages are unrelated``() = + lockFile.IsDependencyOf(PackageName "log",PackageName "Rx-Core") + |> shouldEqual false \ No newline at end of file diff --git a/tests/Paket.Tests/Paket.Tests.fsproj b/tests/Paket.Tests/Paket.Tests.fsproj index 2d0adda0bd..1e66af70f8 100644 --- a/tests/Paket.Tests/Paket.Tests.fsproj +++ b/tests/Paket.Tests/Paket.Tests.fsproj @@ -1,4 +1,4 @@ - + @@ -158,6 +158,7 @@ + From bc4acf7225aea9101ea9a315af63ad1469b6b5d2 Mon Sep 17 00:00:00 2001 From: Steffen Forkmann Date: Sun, 30 Nov 2014 17:56:05 +0100 Subject: [PATCH 2/2] Don't recompute all dependencies on every lookup --- src/Paket.Core/LockFile.fs | 47 ++++++++++++++++++--------------- src/Paket.Core/UpdateProcess.fs | 4 ++- 2 files changed, 29 insertions(+), 22 deletions(-) diff --git a/src/Paket.Core/LockFile.fs b/src/Paket.Core/LockFile.fs index aa5ad131e4..2eb14226ed 100644 --- a/src/Paket.Core/LockFile.fs +++ b/src/Paket.Core/LockFile.fs @@ -224,15 +224,28 @@ type LockFile(fileName:string,options,resolution:PackageResolution,remoteFiles:R member __.FileName = fileName member __.Options = options + /// Gets all dependencies of the given package + member this.GetAllDependenciesOf(package) = + let usedPackages = HashSet<_>() + + let rec addPackage (packageName:PackageName) = + let identity = NormalizedPackageName packageName + match resolution.TryFind identity with + | Some package -> + if usedPackages.Add packageName then + if not this.Options.Strict then + for d,_,_ in package.Dependencies do + addPackage d + | None -> + failwithf "Package %O was referenced, but it was not found in the paket.lock file." packageName + + addPackage package + + usedPackages + /// Checks if the first package is a dependency of the second package - member this.IsDependencyOf(dependentPackage,package) = - let start = NormalizedPackageName package - let target = NormalizedPackageName dependentPackage - let current = resolution.[start] - - if target = start then true else - current.Dependencies - |> Seq.exists (fun (d,_,_) -> this.IsDependencyOf(dependentPackage,d)) + member this.IsDependencyOf(dependentPackage,package) = + this.GetAllDependenciesOf(package).Contains dependentPackage /// Updates the Lock file with the analyzed dependencies from the paket.dependencies file. member __.Save() = @@ -256,18 +269,10 @@ type LockFile(fileName:string,options,resolution:PackageResolution,remoteFiles:R member this.GetPackageHull(referencesFile:ReferencesFile) = let usedPackages = HashSet<_>() - let rec addPackage directly (packageName:PackageName) = - let identity = NormalizedPackageName packageName - match resolution.TryFind identity with - | Some package -> - if usedPackages.Add packageName then - if not this.Options.Strict then - for d,_,_ in package.Dependencies do - addPackage false d - | None -> - failwithf "%s references package %O, but it was not found in the paket.lock file." referencesFile.FileName packageName - referencesFile.NugetPackages - |> List.iter (addPackage true) + |> List.iter (fun package -> + try + usedPackages.UnionWith(this.GetAllDependenciesOf(package)) + with exn -> failwithf "%s - in %s" exn.Message referencesFile.FileName) - usedPackages \ No newline at end of file + usedPackages \ No newline at end of file diff --git a/src/Paket.Core/UpdateProcess.fs b/src/Paket.Core/UpdateProcess.fs index d3bd83d883..32476b21e9 100644 --- a/src/Paket.Core/UpdateProcess.fs +++ b/src/Paket.Core/UpdateProcess.fs @@ -27,9 +27,11 @@ let Update(dependenciesFileName, forceResolution, force, hard) = InstallProcess.Install(sources, force, hard, lockFile) let private fixOldDependencies (oldLockFile:LockFile) (dependenciesFile:DependenciesFile) (package:PackageName) = + let allDependencies = oldLockFile.GetAllDependenciesOf package + oldLockFile.ResolvedPackages |> Seq.map (fun kv -> kv.Value) - |> Seq.filter (fun p -> not <| oldLockFile.IsDependencyOf(p.Name,package)) + |> Seq.filter (fun p -> not <| allDependencies.Contains p.Name) |> Seq.fold (fun (dependenciesFile : DependenciesFile) resolvedPackage -> dependenciesFile.AddFixedPackage(resolvedPackage.Name, "= " + resolvedPackage.Version.ToString()))