Skip to content

Implementation for #913 Paket.pack: add support for nuget dependencies conditional on target framework #2428

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 5 commits into from
Jun 18, 2017
Merged

Implementation for #913 Paket.pack: add support for nuget dependencies conditional on target framework #2428

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
edd4c0e
#913 Implementation for multiple targetFramework
bhugot Jun 16, 2017
bdfeef4
clean up a bit
bhugot Jun 16, 2017
dc1026c
Merge branch 'master' into PaketPackTargetFramework
Jun 16, 2017
86e5e3a
Refactoring
bhugot Jun 18, 2017
b6a742b
Add documentation
bhugot Jun 18, 2017
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
37 changes: 30 additions & 7 deletions src/Paket.Core/Packaging/NupkgWriter.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ open Paket.Xml
open System.Text
open System.Text.RegularExpressions
open System.Xml
open Paket.Requirements

module internal NupkgWriter =

Expand Down Expand Up @@ -83,23 +84,45 @@ module internal NupkgWriter =
frameworkAssembliesList |> List.iter (buildFrameworkReferencesNode >> d.Add)
metadataNode.Add d

let buildDependencyNode (Id, requirement:VersionRequirement) =
let buildDependencyNode (id, requirement:VersionRequirement) =

@forki forki Jun 18, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you please rename this thing. id is a function from the FSharp.Core and I think we should avoid confusion. Maybe name or idProperty or something like that

let dep = XElement(ns + "dependency")
dep.SetAttributeValue(XName.Get "id", Id)
dep.SetAttributeValue(XName.Get "id", id)
let version = requirement.FormatInNuGetSyntax()
if String.IsNullOrEmpty version then
dep.SetAttributeValue(XName.Get "version", "0.0")
else
dep.SetAttributeValue(XName.Get "version", version)
dep

let buildGroupNode (framework:FrameworkIdentifier, add) =
let g = XElement(ns + "group")
g.SetAttributeValue(XName.Get "targetFramework", framework.ToString())
add g
g


let buildDependencyNodes (excludedDependencies, add, dependencyList) =
let takeFirstOfThree (a,_,_) = a

@forki forki Jun 18, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That's a nice trick, but could you please manually inline this? I don't think we really need these functions here. Thx

let takeFirstTwoOfThree (a,b,_) = a,b
dependencyList
|> List.filter (fun d -> Set.contains (takeFirstOfThree d) excludedDependencies |> not)
|> List.map takeFirstTwoOfThree
|> List.iter (buildDependencyNode >> add)

let buildDependencyNodesByGroup excludedDependencies add dependencyList framework =
let node = buildGroupNode(framework, add)
buildDependencyNodes(excludedDependencies, node.Add, dependencyList)

let buildDependenciesNode excludedDependencies dependencyList =
if List.isEmpty dependencyList then () else
let d = XElement(ns + "dependencies")
dependencyList
|> List.filter (fun d -> Set.contains (fst d) excludedDependencies |> not)
|> List.iter (buildDependencyNode >> d.Add)
metadataNode.Add d
let groups = List.groupBy thirdOf3 dependencyList
if groups.Length = 1 && fst groups.Head = None then

@forki forki Jun 18, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you please use pattern matching here?

buildDependencyNodes(excludedDependencies, d.Add, dependencyList)
else
groups
|> List.iter (fun a -> buildDependencyNodesByGroup excludedDependencies d.Add (snd a) (fst a).Value)
metadataNode.Add d

let buildReferenceNode (fileName) =
let dep = XElement(ns + "reference")
Expand Down Expand Up @@ -369,7 +392,7 @@ module NuspecExtensions =
references.Groups |> Seq.collect (fun kvp ->
kvp.Value.NugetPackages |> List.choose (fun pkg ->
dependencies.TryGetPackage(kvp.Key,pkg.Name)
|> Option.map (fun verreq -> pkg.Name,verreq.VersionRequirement)))
|> Option.map (fun verreq -> pkg.Name,verreq.VersionRequirement, None)))
|> List.ofSeq
) |> Option.defaultValue []
let projectInfo, optionalInfo = project.GetTemplateMetadata ()
Expand Down
20 changes: 12 additions & 8 deletions src/Paket.Core/Packaging/PackageMetaData.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,14 @@ let (|Valid|Invalid|) md =
Symbols = s }
| _ -> Invalid

let addDependency (templateFile : TemplateFile) (dependency : PackageName * VersionRequirement) =


let addDependency (templateFile : TemplateFile) (dependency : PackageName * VersionRequirement * FrameworkIdentifier option) =
match templateFile with
| CompleteTemplate(core, opt) ->

@forki forki Jun 18, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same here. Please get rid of firstOf3

let firstOf3 (a, _, _) = a
let newDeps =
match opt.Dependencies |> List.tryFind (fun (n,_) -> n = fst dependency) with
match opt.Dependencies |> List.tryFind (fun (n,_,_) -> n = firstOf3 dependency) with
| None -> dependency :: opt.Dependencies
| _ -> opt.Dependencies
{ FileName = templateFile.FileName
Expand Down Expand Up @@ -253,7 +256,7 @@ let findDependencies (dependenciesFile : DependenciesFile) config platform (temp
match core.Version with
| Some v ->
let versionConstraint = if lockDependencies || pinProjectReferences then Specific v else Minimum v
PackageName core.Id, VersionRequirement(versionConstraint, getPreReleaseStatus v)
PackageName core.Id, VersionRequirement(versionConstraint, getPreReleaseStatus v), None
| None -> failwithf "There was no version given for %s." templateFile.FileName
| IncompleteTemplate ->
failwithf "You cannot create a dependency on a template file (%s) with incomplete metadata." templateFile.FileName)
Expand Down Expand Up @@ -357,7 +360,7 @@ let findDependencies (dependenciesFile : DependenciesFile) config platform (temp
| None ->
match version with
| Some v ->
np.Name,VersionRequirement.Parse (v.ToString())
np.Name,VersionRequirement.Parse (v.ToString()) , None
| None ->
if minimumFromLockFile then
let groupName =
Expand All @@ -368,7 +371,7 @@ let findDependencies (dependenciesFile : DependenciesFile) config platform (temp
|> Option.map fst

match groupName with
| None -> np.Name,specificVersionRequirement
| None -> np.Name,specificVersionRequirement, None
| Some groupName ->
let group = lockFile.GetGroup groupName

Expand All @@ -377,9 +380,9 @@ let findDependencies (dependenciesFile : DependenciesFile) config platform (temp
| Some resolvedPackage -> VersionRequirement(GreaterThan resolvedPackage.Version, getPreReleaseStatus resolvedPackage.Version)
| None -> specificVersionRequirement

np.Name,lockedVersion
np.Name,lockedVersion, None
else
np.Name,specificVersionRequirement
np.Name,specificVersionRequirement, None
| Some groupName ->
let dependencyVersionRequirement =
if not lockDependencies then
Expand Down Expand Up @@ -437,7 +440,8 @@ let findDependencies (dependenciesFile : DependenciesFile) config platform (temp
match dependencyVersionRequirement with
| Some installed -> installed
| None -> failwithf "No package with id '%O' installed in group %O." np.Name groupName
np.Name, dep)

np.Name, dep, None)

deps
|> List.fold addDependency withDepsAndIncluded
108 changes: 72 additions & 36 deletions src/Paket.Core/PaketConfigFiles/TemplateFile.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ open System.IO
open System.Text.RegularExpressions
open Chessie.ErrorHandling
open Paket.Domain
open Paket.Requirements

module private TemplateParser =
type private ParserState =
Expand Down Expand Up @@ -141,7 +142,7 @@ type OptionalPackagingInfo =
RequireLicenseAcceptance : bool
Tags : string list
DevelopmentDependency : bool
Dependencies : (PackageName * VersionRequirement) list
Dependencies : (PackageName * VersionRequirement * FrameworkIdentifier option) list
ExcludedDependencies : Set<PackageName>
ExcludedGroups : Set<GroupName>
References : string list
Expand Down Expand Up @@ -261,45 +262,80 @@ module internal TemplateFile =
| Some m -> ok m
| None -> failP file "No description line in paket.template file."

let private (|Framework|_|) (line:string) =
match line.Trim() with
| String.RemovePrefix "framework:" _ as trimmed -> Some (FrameworkDetection.Extract(trimmed.Replace("framework: ","")))
| _ -> None

let private (|Empty|_|) (line:string) =
match line.Trim() with
| _ when String.IsNullOrWhiteSpace line -> Some (Empty line)
| String.RemovePrefix "//" _ -> Some (Empty line)
| String.RemovePrefix "#" _ -> Some (Empty line)
| _ -> None
type TargetFrameworkGroup =
{
Framework : FrameworkIdentifier option
Dependencies : (PackageName * VersionRequirement * FrameworkIdentifier option) List}
static member ForNone = { Framework = None ; Dependencies = [] }
static member ForFramework framework = { Framework = framework ; Dependencies = [] }

let private getDependencieByLine (fileName, lockFile:LockFile,currentVersion:SemVerInfo option, specificVersions:Map<string, SemVerInfo>, line:string, framework:FrameworkIdentifier option)=

@forki forki Jun 18, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Small typo in function name

let reg = Regex(@"(?<id>\S+)(?<version>.*)").Match line
let name = PackageName reg.Groups.["id"].Value
let versionRequirement =
let versionString =
let s = reg.Groups.["version"].Value.Trim()
if s.Contains "CURRENTVERSION" then
match specificVersions.TryFind (string name) with
| Some v -> s.Replace("CURRENTVERSION", string v)
| None ->
match currentVersion with
| Some v -> s.Replace("CURRENTVERSION", string v)
| None -> failwithf "The template file %s contains the placeholder CURRENTVERSION, but no version was given." fileName

elif s.Contains "LOCKEDVERSION" then
match lockFile.Groups.[Constants.MainDependencyGroup].Resolution |> Map.tryFind name with
| Some p -> s.Replace("LOCKEDVERSION", string p.Version)
| None ->
let packages =
lockFile.GetGroupedResolution()
|> Seq.filter (fun kv -> snd kv.Key = name)
|> Seq.toList

match packages with
| [] -> failwithf "The template file %s contains the placeholder LOCKEDVERSION, but no version was given for package %O in paket.lock." fileName name
| [kv] -> s.Replace("LOCKEDVERSION", string kv.Value.Version)
| _ -> failwithf "The template file %s contains the placeholder LOCKEDVERSION, but more than one group contains package %O in paket.lock." fileName name

else s

DependenciesFileParser.parseVersionRequirement versionString
name, versionRequirement, framework

let private getDependenciesByTargetFramework fileName lockFile currentVersion specificVersions (lineNo, state: TargetFrameworkGroup list) line=
match state with
| current::other ->
let lineNo = lineNo + 1
match line with
| Framework framework ->
let group = TargetFrameworkGroup.ForFramework framework::current::other
lineNo, group
| Empty _ -> lineNo, current::other
| _ ->
let dependency = getDependencieByLine(fileName, lockFile, currentVersion, specificVersions, line, current.Framework)
lineNo ,{ current with Dependencies = current.Dependencies @ [dependency] }::other
| [] -> failwithf "Error in paket.dependencies line %d" lineNo

let private getDependencies (fileName, lockFile:LockFile, info : Map<string, string>,currentVersion:SemVerInfo option, specificVersions:Map<string, SemVerInfo>) =
match Map.tryFind "dependencies" info with
| None -> []
| Some d ->
d.Split '\n'
|> Array.map (fun d ->
let reg = Regex(@"(?<id>\S+)(?<version>.*)").Match d
let name = PackageName reg.Groups.["id"].Value
let versionRequirement =
let versionString =
let s = reg.Groups.["version"].Value.Trim()
if s.Contains "CURRENTVERSION" then
match specificVersions.TryFind (string name) with
| Some v -> s.Replace("CURRENTVERSION", string v)
| None ->
match currentVersion with
| Some v -> s.Replace("CURRENTVERSION", string v)
| None -> failwithf "The template file %s contains the placeholder CURRENTVERSION, but no version was given." fileName

elif s.Contains "LOCKEDVERSION" then
match lockFile.Groups.[Constants.MainDependencyGroup].Resolution |> Map.tryFind name with
| Some p -> s.Replace("LOCKEDVERSION", string p.Version)
| None ->
let packages =
lockFile.GetGroupedResolution()
|> Seq.filter (fun kv -> snd kv.Key = name)
|> Seq.toList

match packages with
| [] -> failwithf "The template file %s contains the placeholder LOCKEDVERSION, but no version was given for package %O in paket.lock." fileName name
| [kv] -> s.Replace("LOCKEDVERSION", string kv.Value.Version)
| _ -> failwithf "The template file %s contains the placeholder LOCKEDVERSION, but more than one group contains package %O in paket.lock." fileName name

else s

DependenciesFileParser.parseVersionRequirement versionString

name, versionRequirement)
|> Array.toList
d.Split '\n'
|> Array.fold (getDependenciesByTargetFramework fileName lockFile currentVersion specificVersions) (0, [TargetFrameworkGroup.ForNone])
|> snd
|> List.rev
|> List.collect (fun a -> a.Dependencies)


let private getExcludedDependencies (info : Map<string, string>) =
Expand Down
1 change: 0 additions & 1 deletion src/Paket.Core/Versioning/FrameworkHandling.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,6 @@ module FrameworkDetection =

open Logging
/// parse a string to construct a Netframework, NetCore, NetStandard, or other dotnet identifier
[<Obsolete "Use PlatformMatching.extractPlatforms instead">]

@forki forki Jun 18, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

?

let Extract =
memoize
(fun (path:string) ->
Expand Down
86 changes: 84 additions & 2 deletions tests/Paket.Tests/Packaging/NuspecWriterSpecs.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ open FsUnit
open NUnit.Framework
open TestHelpers
open Paket.Domain
open Paket.Requirements

[<Test>]
let ``should serialize core info``() =
Expand Down Expand Up @@ -58,8 +59,89 @@ let ``should serialize dependencies``() =
{ OptionalPackagingInfo.Epmty with
Tags = [ "f#"; "rules" ]
Dependencies =
[ PackageName "Paket.Core", VersionRequirement.Parse "[3.1]"
PackageName "xUnit", VersionRequirement.Parse "2.0" ] }
[ PackageName "Paket.Core", VersionRequirement.Parse "[3.1]", None
PackageName "xUnit", VersionRequirement.Parse "2.0", None ] }

let doc = NupkgWriter.nuspecDoc (core, optional)
doc.ToString()
|> normalizeLineEndings
|> shouldEqual (normalizeLineEndings result)


[<Test>]
let ``#913 should serialize dependencies by group``() =
let result = """<package xmlns="http://schemas.microsoft.com/packaging/2011/10/nuspec.xsd">
<metadata>
<id>Paket.Tests</id>
<version>1.0.0.0</version>
<authors>Two, Authors</authors>
<description>A description</description>
<tags>f# rules</tags>
<dependencies>
<group targetFramework="net35">
<dependency id="Paket.Core" version="[3.1.0]" />
<dependency id="xUnit" version="2.0.0" />
</group>
</dependencies>
</metadata>
</package>"""

let core : CompleteCoreInfo =
{ Id = "Paket.Tests"
Version = SemVer.Parse "1.0.0.0" |> Some
Authors = [ "Two"; "Authors" ]
Description = "A description"
Symbols = false }

let optional =
{ OptionalPackagingInfo.Epmty with
Tags = [ "f#"; "rules" ]
Dependencies =
[ PackageName "Paket.Core", VersionRequirement.Parse "[3.1]", Some(FrameworkIdentifier.DotNetFramework(FrameworkVersion.V3_5))
PackageName "xUnit", VersionRequirement.Parse "2.0", Some(FrameworkIdentifier.DotNetFramework(FrameworkVersion.V3_5)) ] }

let doc = NupkgWriter.nuspecDoc (core, optional)
doc.ToString()
|> normalizeLineEndings
|> shouldEqual (normalizeLineEndings result)

[<Test>]
let ``#913 should serialize dependencies by group with 2 group``() =
let result = """<package xmlns="http://schemas.microsoft.com/packaging/2011/10/nuspec.xsd">
<metadata>
<id>Paket.Tests</id>
<version>1.0.0.0</version>
<authors>Two, Authors</authors>
<description>A description</description>
<tags>f# rules</tags>
<dependencies>
<group targetFramework="net35">
<dependency id="Paket.Core" version="[3.1.0]" />
<dependency id="xUnit" version="2.0.0" />
</group>
<group targetFramework="netstandard13">
<dependency id="Paket.Core" version="[3.1.0]" />
<dependency id="xUnit" version="2.0.0" />
</group>
</dependencies>
</metadata>
</package>"""

let core : CompleteCoreInfo =
{ Id = "Paket.Tests"
Version = SemVer.Parse "1.0.0.0" |> Some
Authors = [ "Two"; "Authors" ]
Description = "A description"
Symbols = false }

let optional =
{ OptionalPackagingInfo.Epmty with
Tags = [ "f#"; "rules" ]
Dependencies =
[ PackageName "Paket.Core", VersionRequirement.Parse "[3.1]", Some(FrameworkIdentifier.DotNetFramework(FrameworkVersion.V3_5))
PackageName "xUnit", VersionRequirement.Parse "2.0", Some(FrameworkIdentifier.DotNetFramework(FrameworkVersion.V3_5))
PackageName "Paket.Core", VersionRequirement.Parse "[3.1]", Some(FrameworkIdentifier.DotNetStandard(DotNetStandardVersion.V1_3))
PackageName "xUnit", VersionRequirement.Parse "2.0", Some(FrameworkIdentifier.DotNetStandard(DotNetStandardVersion.V1_3)) ] }

let doc = NupkgWriter.nuspecDoc (core, optional)
doc.ToString()
Expand Down
Loading