Skip to content

Fixes for #4183 #4184

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

Merged
forki merged 1 commit into from
Nov 18, 2022
Merged

Fixes for #4183 #4184

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion global.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{
"sdk": {
"version": "6.0.301"
"version": "6.0.301",
"rollForward": "latestMinor"

@TheAngryByrd TheAngryByrd Nov 10, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Needed since dotnet versions keep moving on

}
}
44 changes: 42 additions & 2 deletions integrationtests/Paket.IntegrationTests/PackSpecs.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1012,7 +1012,7 @@ let private scrapeDeps (nuspecPath: string) =
doc.LoadXml text

allDeps doc
|> Map.ofSeq


[<Test>]
let ``#2883 writes warning for missing direct dependencies``() =
Expand Down Expand Up @@ -1041,14 +1041,54 @@ let ``#2883 writes ranges for floating deps``() =

let inputNuspecPath = Path.Combine(scenarioRoot, "before.nuspec")
let inputDeps = scrapeDeps inputNuspecPath

let dispose, messages = paket (sprintf "fix-nuspecs files %s project-file %s" inputNuspecPath fsprojPath) scenario
use __ = dispose
let outputDeps = scrapeDeps inputNuspecPath

match outputDeps |> Map.tryFind "FSharp.Compiler.Service" with
match outputDeps |> Map |> Map.tryFind "FSharp.Compiler.Service" with
| Some "[35.0.0,36.0.0)" -> ()
| _ ->
failwithf "Expected to modify deps for FSharp.Compiler.Service package with floating version constraint.\nBefore:\t%A\nAfter:\t%A\nMessages:\t%A"
inputDeps
outputDeps
messages

[<Test>]
let ``#4183 writes ranges for floating deps``() =
let scenario = "i004183-apply-with-multiple-groups"
use __ = prepareSdk scenario
let scenarioRoot = scenarioTempPath scenario
let fsprojPath = Path.Combine(scenarioRoot, "before.fsproj")

let inputNuspecPath = Path.Combine(scenarioRoot, "before.nuspec")
let refFile = Path.Combine(scenarioRoot, "paket.references")
let inputDeps = scrapeDeps inputNuspecPath

let dispose, messages = paket (sprintf "fix-nuspecs files %s project-file %s" inputNuspecPath fsprojPath) scenario
use __ = dispose
let outputDeps = scrapeDeps inputNuspecPath

let actualPackages = outputDeps |> Seq.filter (fst>>(=)"FSharp.Core")
let expectedVersionRanges =
[
"[4.7.2,5.0.0)"
"[7.0.0,8.0.0)"
]
|> Set.ofSeq

let actualVersionRanges =
actualPackages
|> Seq.map snd
|> Set.ofSeq

let diff = Set.difference expectedVersionRanges actualVersionRanges


if diff |> Seq.length = 0 then ()
else

failwithf "Expected to modify deps for FSharp.Core package with floating version constraint.\nBefore:\t%A\nAfter:\t%A\nMessages:\t%A"
inputDeps
outputDeps
messages
Comment on lines +1058 to +1094

@TheAngryByrd TheAngryByrd Nov 10, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Test to simulate multi-targeting

557 changes: 557 additions & 0 deletions ...iontests/scenarios/i004183-apply-with-multiple-groups/before/.paket/Paket.Restore.targets
View file
Open in desktop

Large diffs are not rendered by default.

5 changes: 5 additions & 0 deletions integrationtests/scenarios/i004183-apply-with-multiple-groups/before/Library.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
namespace before

module Say =
let hello name =
printfn "Hello %s" name
10 changes: 10 additions & 0 deletions integrationtests/scenarios/i004183-apply-with-multiple-groups/before/before.fsproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?xml version="1.0" encoding="utf-8"?>
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>netstandard2.0; netstandard2.1</TargetFrameworks>
</PropertyGroup>
<ItemGroup>
<Compile Include="Library.fs" />
</ItemGroup>
<Import Project=".paket\Paket.Restore.targets" />
</Project>
17 changes: 17 additions & 0 deletions integrationtests/scenarios/i004183-apply-with-multiple-groups/before/before.nuspec
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?xml version="1.0" encoding="utf-8"?>
<package xmlns="http://schemas.microsoft.com/packaging/2012/06/nuspec.xsd">
<metadata>
<id>before</id>
<version>1.0.0</version>
<authors>before</authors>
<description>Package Description</description>
<dependencies>
<group targetFramework=".NETStandard2.0">
<dependency id="FSharp.Core" version="4.7.2" exclude="Build,Analyzers" />
</group>
<group targetFramework=".NETStandard2.1">
<dependency id="FSharp.Core" version="7.0.0" exclude="Build,Analyzers" />
</group>
Comment on lines +9 to +14

@TheAngryByrd TheAngryByrd Nov 10, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Multi-target nuspec file

</dependencies>
</metadata>
</package>
14 changes: 14 additions & 0 deletions integrationtests/scenarios/i004183-apply-with-multiple-groups/before/paket.dependencies
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@

group testGroup1
source https://api.nuget.org/v3/index.json
storage: none
nuget FSharp.Core ~> 4
framework: netstandard2.0
condition: netstandard2_0

group testGroup2
source https://api.nuget.org/v3/index.json
storage: none
nuget FSharp.Core ~> 7
framework: netstandard2.1
condition: netstandard2_1
Comment on lines +2 to +14

@TheAngryByrd TheAngryByrd Nov 10, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

paket.dependencies file with multi-targeting

17 changes: 17 additions & 0 deletions integrationtests/scenarios/i004183-apply-with-multiple-groups/before/paket.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@


GROUP testGroup1
STORAGE: NONE
CONDITION: NETSTANDARD2_0
RESTRICTION: == netstandard2.0
NUGET
remote: https://api.nuget.org/v3/index.json
FSharp.Core (4.7.2)

GROUP testGroup2
STORAGE: NONE
CONDITION: NETSTANDARD2_1
RESTRICTION: == netstandard2.1
NUGET
remote: https://api.nuget.org/v3/index.json
FSharp.Core (7.0)
5 changes: 5 additions & 0 deletions integrationtests/scenarios/i004183-apply-with-multiple-groups/before/paket.references
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
group testGroup1
FSharp.Core

group testGroup2
FSharp.Core
3 changes: 2 additions & 1 deletion src/Paket.Core/PaketConfigFiles/LockFile.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -846,6 +846,7 @@ type LockFile (fileName:string, groups: Map<GroupName,LockFileGroup>) =
| exn ->
raise (Exception (sprintf "Error during parsing of '%s'." lockFileName, exn))


member this.GetPackageHull(referencesFile:ReferencesFile) =
let usedPackages = Dictionary<_,_>()

Expand All @@ -854,7 +855,7 @@ type LockFile (fileName:string, groups: Map<GroupName,LockFileGroup>) =
for p in g.Value.NugetPackages do
let k = g.Key,p.Name
if usedPackages.ContainsKey k |> not then
usedPackages.Add(k,p)
usedPackages.Add(k,{p with Settings = lockGroup.Options.Settings})

@TheAngryByrd TheAngryByrd Nov 10, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We we're getting the lock file's framework restrictions here. Let me know if I should use another way to get this and revert this change.


for r in g.Value.RemoteFiles do
let lockRemote = findRemoteFile referencesFile.FileName lockGroup.RemoteFiles r.Name
Expand Down
43 changes: 34 additions & 9 deletions src/Paket.Core/PublicAPI.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -880,13 +880,18 @@ type Dependencies(dependenciesFileName: string) =
|> List.map (fun proj -> proj.NameWithoutExtension)
|> Set.ofList
let depsFile = deps.GetDependenciesFile()
let allFrameworkRestrictions =
locked.GetPackageHull referencesFile
|> Seq.map(fun kvp -> snd kvp.Key, fst kvp.Key, kvp.Value.Settings.FrameworkRestrictions.GetExplicitRestriction())


// NuGet has thrown away "group" association, so this is best effort.
let implicitOrExplictlyReferencedPackages =
locked.GetPackageHull referencesFile
|> Seq.map (fun kv -> kv.Key |> snd)
|> Set.ofSeq


for nuspecFile in nuspecFileList do
if not (File.Exists nuspecFile) then
failwithf "Specified file '%s' does not exist." nuspecFile
Expand Down Expand Up @@ -947,14 +952,16 @@ type Dependencies(dependenciesFileName: string) =
then
UnknownDependency
else

tracefn "Many groups detected %O" groupsForProjectReferencedDeps
match groupsForProjectReferencedDeps.TryFind p with
| None -> IndirectDependency
| Some [] -> IndirectDependency
| Some [group] -> DirectDependency (group, p)
| Some (firstGroup::_) -> DirectDependency (firstGroup, p)
// | Some [group] -> DirectDependency (group, p)
| Some (groups) ->
DirectDependency (groups, p)

let (|MatchesRange|NeedsRangeUpdate|LockRangeNotFound|) (groupAndPackage: (GroupName * PackageName), nuspecVersionRequirement: VersionRequirement) =

match lockedPackageVersionRequirements.TryFind groupAndPackage with
| Some lockedFileRange ->
if lockedFileRange = nuspecVersionRequirement
Expand All @@ -974,6 +981,22 @@ type Dependencies(dependenciesFileName: string) =
let nodesToRemove = ResizeArray()
for node in parent.ChildNodes do
if node.Name = "dependency" then
let targetFramework =
attr "targetFramework" node.ParentNode
|> Option.bind(fun tfm ->
PlatformMatching.forceExtractPlatforms tfm |> fun p -> p.ToTargetProfile true
)
let results =
targetFramework
|> Option.map(fun tfm ->
allFrameworkRestrictions
|> Seq.filter(fun (_, _, fr) -> fr.IsMatch tfm)
|> Seq.toList

)
|> Option.defaultValue []
Comment on lines +984 to +997

@TheAngryByrd TheAngryByrd Nov 10, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Code to align the nuspec group's targetFramework with a paket group.



let packName = attr "id" node |> Option.map PackageName
let versionRange = attr "version" node |> Option.bind VersionRequirement.TryParse
tracefn "Checking dependency status for package %O, version %O" packName versionRange
Expand All @@ -987,22 +1010,24 @@ type Dependencies(dependenciesFileName: string) =
| Some UnknownDependency ->
tracefn "Package '%O' is not part of the explicit dependency tree in %s and so will be skipped" packName.Value referencesFile.FileName
()
| Some (DirectDependency ((GroupName(grp, _)) as g, (PackageName.PackageName(pkg, _) as p))) ->
match versionRange with
| Some versionRange ->
match (g, p), versionRange with
| Some (DirectDependency (g, (PackageName.PackageName(pkg, _) as p))) ->
let groupWithinFramework = results |> Seq.tryFind(fun (p1,g1,_) -> p = p1 && g |> Seq.exists(fun x -> x = g1)) |> Option.map(fun (_,g,_) -> g)
match versionRange, groupWithinFramework with
| Some versionRange, Some grp ->
match (grp, p), versionRange with
| MatchesRange ->
tracefn "Package '%s' is a direct dependency and requires no version patching" pkg
()
| LockRangeNotFound ->
tracefn "Couldn't find a version range for package '%s' in group '%s', is this package in your paket.dependencies file?" pkg grp
let grp = g |> List.map(fun (GroupName(name,_)) -> name) |> String.concat ", "
tracefn "Couldn't find a version range for package '%s' in any group '%s', is this package in your paket.dependencies file?" pkg grp
()
| NeedsRangeUpdate newVersionRange ->
let oldVersionRangeString = versionRange.FormatInNuGetSyntax()
let nugetVersionRangeString = newVersionRange.FormatInNuGetSyntax()
tracefn "Package '%s' is a direct dependency and requires version patching from %s to %s"pkg oldVersionRangeString nugetVersionRangeString
node.Attributes.["version"].InnerText <- nugetVersionRangeString
| None ->
| _, _ ->
tracefn "Package '%s' is a direct dependency but no desired range was found, so it will be skipped" pkg
()
| None ->
Expand Down