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.

Sign up for GitHub

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

Jump to bottom

[WIP] Give reasons for resolving group (#3008) #3302

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

[WIP] Give reasons for resolving group (#3008) #3302

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d05788e
First
TeaDrivenDev Jul 9, 2018
1e6cf24
List reasons for resolving group
TeaDrivenDev Jul 14, 2018
4677469
Extract install code to function
TeaDrivenDev Jul 14, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 55 additions & 19 deletions src/Paket.Core/Installation/DependencyChangeDetection.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
module Paket.DependencyChangeDetection

open Paket.Domain
open Paket.Requirements
open Paket.PackageResolver

Expand All @@ -10,13 +11,25 @@ type DependencyChangeType =
| SettingsChanged
/// The Version in the LockFile doesn't match the spec in the dependencies file.
| VersionNotValid
/// The Version in the LockFile is a prerelease, but the dependencies file does not allow prereleases for this package.
| PrereleaseVersionNotAllowed
/// Package from dependencies file was not found in lockfile
| PackageNotFoundInLockFile
/// Group from dependencies file was not found in lockfile
| GroupNotFoundInLockFile
/// Package from lock file was not found in dependencies file
| PackageNotFoundInDependenciesFile

override this.ToString() =
match this with
| RestrictionsChanged -> "Framework restrictions have changed"
| SettingsChanged -> "Settings have changed"
| VersionNotValid -> "Installed version is not valid"
| PrereleaseVersionNotAllowed -> "Prerelease version installed, but not allowed"
| PackageNotFoundInLockFile -> "Package was not found in lock file"
| GroupNotFoundInLockFile -> "Group was not found in lock file"
| PackageNotFoundInDependenciesFile -> "Package was not found in dependencies file"

let findNuGetChangesInDependenciesFile(dependenciesFile:DependenciesFile,lockFile:LockFile,strict) =
let allTransitives groupName = lockFile.GetTransitiveDependencies groupName
let getChanges groupName transitives (newRequirement:PackageRequirement) (originalPackage:PackageInfo) =
Expand All @@ -37,13 +50,19 @@ let findNuGetChangesInDependenciesFile(dependenciesFile:DependenciesFile,lockFil
else []

let requirementOk =
let isInRange =
let requirement =
if strict then
newRequirement.VersionRequirement.IsInRange originalPackage.Version
newRequirement.VersionRequirement
else
newRequirement.IncludingPrereleases().VersionRequirement.IsInRange originalPackage.Version
newRequirement.IncludingPrereleases().VersionRequirement

let isInRange = requirement.IsInRange originalPackage.Version

if not isInRange then
[VersionNotValid]
if originalPackage.Version.PreRelease.IsSome && requirement.PreReleases = No then
[PrereleaseVersionNotAllowed]
else
[VersionNotValid]
else []

requirementOk @ settingsChanged()
Expand Down Expand Up @@ -218,6 +237,11 @@ let GetPreferredNuGetVersions (dependenciesFile:DependenciesFile,lockFile:LockFi
| None -> kv.Key, (kv.Value.Version, kv.Value.Source))
|> Map.ofSeq

type ResolutionTriggeringChange =
| NuGetChange of PackageName * DependencyChangeType
| RemoteChange of projectName:string * fileName:string
| SettingsChange

let GetChanges(dependenciesFile,lockFile,strict) =
let nuGetChanges = findNuGetChangesInDependenciesFile(dependenciesFile,lockFile,strict)
let nuGetChangesPerGroup =
Expand All @@ -231,37 +255,49 @@ let GetChanges(dependenciesFile,lockFile,strict) =
|> Seq.groupBy fst
|> Map.ofSeq

let hasNuGetChanges groupName =
let getNuGetChanges groupName =
match nuGetChangesPerGroup |> Map.tryFind groupName with
| None -> false
| Some x -> Seq.isEmpty x |> not

let hasRemoteFileChanges groupName =
| None -> []
| Some x ->
x
|> Seq.collect (fun (_, package, changes) ->
changes
|> List.map (fun change -> NuGetChange (package, change)))
|> Seq.toList

let getRemoteFileChanges groupName =
match remoteFileChangesPerGroup |> Map.tryFind groupName with
| None -> false
| Some x -> Seq.isEmpty x |> not
| None -> []
| Some x ->
x
|> Seq.map (fun (_, change) -> RemoteChange (change.Project, change.Name))
|> Seq.toList

let hasChangedSettings groupName =
let getChangedSettings groupName =
match dependenciesFile.Groups |> Map.tryFind groupName with
| None -> true
| None -> [ SettingsChange ]
| Some dependenciesFileGroup ->
match lockFile.Groups |> Map.tryFind groupName with
| None -> true
| None -> [ SettingsChange ]
| Some lockFileGroup ->
let lockFileGroupOptions =
if dependenciesFileGroup.Options.Settings.FrameworkRestrictions = AutoDetectFramework then
{ lockFileGroup.Options with Settings = { lockFileGroup.Options.Settings with FrameworkRestrictions = AutoDetectFramework } }
else
lockFileGroup.Options
dependenciesFileGroup.Options <> lockFileGroupOptions

if dependenciesFileGroup.Options <> lockFileGroupOptions then
[ SettingsChange ]
else []

let hasChanges groupName _ =
hasChangedSettings groupName || hasNuGetChanges groupName || hasRemoteFileChanges groupName
let getChanges groupName _ =
[ getChangedSettings groupName; getNuGetChanges groupName; getRemoteFileChanges groupName ]
|> List.concat

let hasAnyChanges =
dependenciesFile.Groups
|> Map.filter hasChanges
|> Map.filter (fun groupName group -> getChanges groupName group |> List.isEmpty |> not)
|> Map.isEmpty
|> not

hasAnyChanges,nuGetChanges,remoteFileChanges,hasChanges
hasAnyChanges,nuGetChanges,remoteFileChanges,getChanges
72 changes: 40 additions & 32 deletions src/Paket.Core/Installation/UpdateProcess.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ open Chessie.ErrorHandling
open Paket.Logging
open InstallProcess

type RTC = DependencyChangeDetection.ResolutionTriggeringChange
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only to bring in the type to avoid having to fully qualify

The alternatives I currently see are

  • opening the DependencyChangeDetection module; not sure if we'd want to do that here
  • specifying the type and module every time in the pattern match, which seems ugly
  • moving the original type somewhere else, like to Paket.Domain; not sure if that would be correct


let selectiveUpdate force getSha1 getVersionsF getPackageDetailsF getRuntimeGraphFromPackage (lockFile:LockFile) (dependenciesFile:DependenciesFile) updateMode semVerUpdateMode =
let dependenciesFile =
let processFile createRequirementF =
Expand Down Expand Up @@ -42,6 +44,42 @@ let selectiveUpdate force getSha1 getVersionsF getPackageDetailsF getRuntimeGrap

let getPreferredVersionsF,getPackageDetailsF,groupsToUpdate =
let changes,groups =
let install groupName =
let hasAnyChanges,nuGetChanges,remoteFileChanges,getChanges = DependencyChangeDetection.GetChanges(dependenciesFile,lockFile,true)

let hasChanges groupName x =
match getChanges groupName x with
| [] ->
tracefn "Skipping resolver for group %O since it is already up-to-date" groupName
false
| changes ->
tracefn "Resolving group %O because of changes:" groupName

changes
|> List.map (function
| RTC.NuGetChange (packageName, change) ->
sprintf "Package %O: %O" packageName change
| RTC.RemoteChange (project, file) ->
sprintf "Remote file %s changed in project %s" file project
| RTC.SettingsChange -> "Group has settings changes")
|> List.iter (tracefn "- %s")

true

let groupFilter =
match groupName with
| Some groupName -> fun g -> g = groupName
| None -> fun _ -> true

let groups =
dependenciesFile.Groups
|> Map.filter (fun g _ -> groupFilter g)
|> Map.filter hasChanges

nuGetChanges
|> Set.map (fun (f,s,_) -> f,s)
|> Set.filter (fst >> groupFilter), groups

match updateMode with
| UpdateAll ->
let changes =
Expand Down Expand Up @@ -78,38 +116,8 @@ let selectiveUpdate force getSha1 getVersionsF getPackageDetailsF getRuntimeGrap
|> Map.filter (fun k _ -> k = groupName || changes |> Seq.exists (fun (g,_) -> g = k))

changes,groups
| InstallGroup groupName ->
let hasAnyChanges,nuGetChanges,remoteFileChanges,hasChanges = DependencyChangeDetection.GetChanges(dependenciesFile,lockFile,true)

let hasChanges groupName x =
let hasChanges = hasChanges groupName x
if not hasChanges then
tracefn "Skipping resolver for group %O since it is already up-to-date" groupName
hasChanges

let groups =
dependenciesFile.Groups
|> Map.filter (fun k _ -> k = groupName)
|> Map.filter hasChanges

nuGetChanges
|> Set.map (fun (f,s,_) -> f,s)
|> Set.filter (fun (g,_) -> g = groupName), groups
| Install ->
let hasAnyChanges,nuGetChanges,remoteFileChanges,hasChanges = DependencyChangeDetection.GetChanges(dependenciesFile,lockFile,true)

let hasChanges groupName x =
let hasChanges = hasChanges groupName x
if not hasChanges then
tracefn "Skipping resolver for group %O since it is already up-to-date" groupName
hasChanges

let groups =
dependenciesFile.Groups
|> Map.filter hasChanges

nuGetChanges
|> Set.map (fun (f,s,_) -> f,s), groups
| InstallGroup groupName -> install (Some groupName)
| Install -> install None

let preferredVersions =
match updateMode with
Expand Down
4 changes: 2 additions & 2 deletions src/Paket.Core/Paket.Core.fsproj
View file
Open in desktop
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>update</StartArguments>
<StartArguments>install</StartArguments>
<StartAction>Project</StartAction>
<StartProgram>paket.exe</StartProgram>
<StartWorkingDirectory>
</StartWorkingDirectory>
<StartWorkingDirectory>C:\proj\testing\testpaketfailure\</StartWorkingDirectory>
<StartWorkingDirectory>D:\Development\Staging\Paket\3008_prerelease_resolution_message\Paket3008\.paket\</StartWorkingDirectory>
</PropertyGroup>
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
<Optimize>true</Optimize>
Expand Down
6 changes: 3 additions & 3 deletions src/Paket/Paket.fsproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
<DebugSymbols>true</DebugSymbols>
<Optimize>false</Optimize>
<Tailcalls>false</Tailcalls>
<OutputPath>..\..\bin\</OutputPath>
<OutputPath>D:\Development\Staging\Paket\3008_prerelease_resolution_message\Paket3008\.paket</OutputPath>
<DefineConstants>DEBUG;TRACE</DefineConstants>
<WarningLevel>3</WarningLevel>
<DocumentationFile>
Expand All @@ -34,14 +34,14 @@
<StartArguments>install</StartArguments>
<StartWorkingDirectory>C:\temp\Gu.Reactive</StartWorkingDirectory>
<StartArguments>update</StartArguments>
<StartArguments>add -g Foo Newtonsoft.Json</StartArguments>
<StartArguments>install</StartArguments>
<StartWorkingDirectory>C:\proj\Paket</StartWorkingDirectory>
<StartWorkingDirectory>D:\temp\PaketTargetFrameworkRepro\</StartWorkingDirectory>
<StartWorkingDirectory>D:\code\bookstore</StartWorkingDirectory>
<StartWorkingDirectory>C:\temp\paket-conflict\paket.conflict.app</StartWorkingDirectory>
<StartWorkingDirectory>D:\temp\i3032</StartWorkingDirectory>
<StartWorkingDirectory>C:\code\Paket\integrationtests\scenarios\i003062-external-lock\before</StartWorkingDirectory>
<StartWorkingDirectory>C:\temp\flip</StartWorkingDirectory>
<StartWorkingDirectory>D:\Development\Staging\Paket\3008_prerelease_resolution_message\Paket3008</StartWorkingDirectory>
</PropertyGroup>
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
<Optimize>true</Optimize>
Expand Down
2 changes: 2 additions & 0 deletions src/Paket/Program.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -918,3 +918,5 @@ let main() =
else printError exn

main()

ignore ()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@forki That's one of the things to.... ignore. ;-) I used that to set a final breakpoint for debugging and didn't want to sort out what to commit while this is still unfinished. As I said, I'll clean everything up when I finish the PR.