Skip to content

Commit

Permalink
Fix paket add if package is already there - fixes #814
Browse files Browse the repository at this point in the history
  • Loading branch information
forki committed May 6, 2015
1 parent 473e352 commit 6b7a44e
Show file tree
Hide file tree
Showing 5 changed files with 99 additions and 28 deletions.
3 changes: 3 additions & 0 deletions RELEASE_NOTES.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
#### 1.3.22 - 06.05.2015
* BUGFIX: Fix `paket add` if package is already there - https://github.com/fsprojects/Paket/issues/814

#### 1.3.21 - 06.05.2015
* BUGFIX: Fix `paket add` for very first dependency - https://github.com/fsprojects/Paket/issues/814

Expand Down
50 changes: 27 additions & 23 deletions src/Paket.Core/DependenciesFile.fs
Original file line number Diff line number Diff line change
Expand Up @@ -337,35 +337,39 @@ type DependenciesFile(fileName,options,sources,packages : PackageRequirement lis

// Try to find alphabetical matching position to insert the package
let smaller = Seq.takeWhile (fun (p:PackageRequirement) -> p.Name <= packageName) packages |> List.ofSeq
let containsLine = Seq.exists (fun (p:PackageRequirement) -> p.Name = packageName) packages

let newLines =
let list = new System.Collections.Generic.List<_>()
list.AddRange textRepresentation
match smaller with
| [] ->
match packages with
| [] ->
if remoteFiles <> [] then
list.Insert(0,"")
match tryFindPackageLine packageName with
| Some pos -> list.[pos] <- packageString
| None ->
match smaller with
| [] ->
match packages with
| [] ->
if remoteFiles <> [] then
list.Insert(0,"")

match sources with
| [] ->
list.Insert(0,packageString)
list.Insert(0,"")
list.Insert(0,DependenciesFileSerializer.sourceString Constants.DefaultNugetStream)
| _ ->
list.Add("")
list.Add(packageString)
| p::_ ->
match sources with
| [] ->
list.Insert(0,packageString)
list.Insert(0,"")
list.Insert(0,DependenciesFileSerializer.sourceString Constants.DefaultNugetStream)
| _ ->
list.Add("")
list.Add(packageString)
| p::_ ->
match tryFindPackageLine p.Name with
| None -> list.Add packageString
| Some pos -> list.Insert(pos,packageString)
| _ ->
let p = Seq.last smaller

match tryFindPackageLine p.Name with
| None -> list.Add packageString
| Some pos -> list.Insert(pos,packageString)
| _ ->
let p = Seq.last smaller

match tryFindPackageLine p.Name with
| None -> list.Add packageString
| Some pos -> list.Insert(pos + 1,packageString)
| Some pos -> list.Insert(pos + 1,packageString)

list |> Seq.toArray

Expand Down Expand Up @@ -411,7 +415,7 @@ type DependenciesFile(fileName,options,sources,packages : PackageRequirement lis
member this.Add(packageName,version:string,?installSettings : InstallSettings) =
let installSettings = defaultArg installSettings InstallSettings.Default
let (PackageName name) = packageName
if this.HasPackage packageName then
if this.HasPackage packageName && String.IsNullOrWhiteSpace version then
traceWarnfn "%s contains package %s already. ==> Ignored" fileName name
this
else
Expand Down
4 changes: 2 additions & 2 deletions src/Paket/Paket.fsproj
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,10 @@
<StartProgram>paket.exe</StartProgram>
<StartWorkingDirectory>D:\code\Pakettest</StartWorkingDirectory>
<StartWorkingDirectory>D:\code\Paketkopie</StartWorkingDirectory>
<StartArguments>add nuget fsunit project "D:\code\gitnav\src\app\GitNav\Program.fs "</StartArguments>
<StartArguments>install</StartArguments>
<StartAction>Project</StartAction>
<StartProgram>paket.exe</StartProgram>
<StartWorkingDirectory>D:\code\gitnav\src\app\GitNav\</StartWorkingDirectory>
<StartWorkingDirectory>D:\code\PaketTest</StartWorkingDirectory>
</PropertyGroup>
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
<DebugType>pdbonly</DebugType>
Expand Down
34 changes: 32 additions & 2 deletions tests/Paket.Tests/DependenciesFile/AddPackageSpecs.fs
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ github fsprojects/Chessie src/Chessie/ErrorHandling.fs"""
|> shouldEqual (normalizeLineEndings expected)

[<Test>]
let ``should add Microsoft.AspNet.WebApi package in first position if only souce is given``() =
let ``should add Microsoft.AspNet.WebApi package in first position if only source is given``() =
let config = """source https://nuget.org/api/v2"""

let cfg = DependenciesFile.FromCode(config).Add(PackageName "Microsoft.AspNet.WebApi","")
Expand All @@ -273,4 +273,34 @@ let ``should add Microsoft.AspNet.WebApi package in first position if only souce
nuget Microsoft.AspNet.WebApi"""

cfg.ToString()
|> shouldEqual (normalizeLineEndings expected)
|> shouldEqual (normalizeLineEndings expected)

[<Test>]
let ``should add Microsoft.AspNet.WebApi package in correct position if package is already given``() =
let config = """source http://internalfeed/NugetWebFeed/nuget
nuget Microsoft.AspNet.WebApi.Client 5.2.3
nuget Microsoft.AspNet.WebApi.Core 5.2.3
nuget Microsoft.AspNet.WebApi.WebHost 5.2.3
nuget log4net
source https://nuget.org/api/v2
nuget Microsoft.AspNet.WebApi
nuget log4net 1.2.10"""

let cfg = DependenciesFile.FromCode(config).Add(PackageName "Microsoft.AspNet.WebApi","5.2.3")

let expected = """source http://internalfeed/NugetWebFeed/nuget
nuget Microsoft.AspNet.WebApi.Client 5.2.3
nuget Microsoft.AspNet.WebApi.Core 5.2.3
nuget Microsoft.AspNet.WebApi.WebHost 5.2.3
nuget log4net
source https://nuget.org/api/v2
nuget Microsoft.AspNet.WebApi 5.2.3
nuget log4net 1.2.10"""

cfg.ToString()
|> shouldEqual (normalizeLineEndings expected)

36 changes: 35 additions & 1 deletion tests/Paket.Tests/Lockfile/ParserSpecs.fs
Original file line number Diff line number Diff line change
Expand Up @@ -440,4 +440,38 @@ let ``should parse reactiveui lockfile``() =

packages.[10].Name |> shouldEqual (PackageName "Rx-Xaml")
packages.[10].Version |> shouldEqual (SemVer.Parse "2.2.5")
packages.[10].Settings.FrameworkRestrictions.ToString() |> shouldEqual "[winv4.5; wpv8.0; >= net45]"
packages.[10].Settings.FrameworkRestrictions.ToString() |> shouldEqual "[winv4.5; wpv8.0; >= net45]"

let multipleFeedLockFile = """NUGET
remote: http://internalfeed/NugetWebFeed/nuget
specs:
Internal_1 (1.2.10)
Newtonsoft.Json (>= 6.0.0 < 6.1.0)
log4net (1.2.10)
Newtonsoft.Json (6.0.6)
remote: https://www.nuget.org/api/v2
specs:
Microsoft.AspNet.WebApi (5.2.3)
Microsoft.AspNet.WebApi.WebHost (>= 5.2.3 < 5.3.0)
Microsoft.AspNet.WebApi.Client (5.2.3)
Microsoft.Net.Http (>= 2.2.22) - framework: portable-wp80+win+net45+wp81+wpa81
Newtonsoft.Json (>= 6.0.4) - framework: portable-wp80+win+net45+wp81+wpa81, >= net45
Microsoft.AspNet.WebApi.Core (5.2.3)
Microsoft.AspNet.WebApi.Client (>= 5.2.3)
Microsoft.AspNet.WebApi.WebHost (5.2.3)
Microsoft.AspNet.WebApi.Core (>= 5.2.3 < 5.3.0)"""

[<Test>]
let ``should parse lockfile with multiple feeds``() =
let lockFile = LockFileParser.Parse(toLines multipleFeedLockFile)
let references = lockFile.SourceFiles

references.Length |> shouldEqual 0

let packages = List.rev lockFile.Packages
packages.Length |> shouldEqual 7

packages.[3].Name |> shouldEqual (PackageName "Microsoft.AspNet.WebApi")
packages.[3].Version |> shouldEqual (SemVer.Parse "5.2.3")
packages.[3].Settings.FrameworkRestrictions.ToString() |> shouldEqual "[]"
packages.[3].Source.ToString() |> shouldEqual "https://www.nuget.org/api/v2"

2 comments on commit 6b7a44e

@dzendras
Copy link

Choose a reason for hiding this comment

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

Maybe it'd be better to use a version for Internal_1 that is different from log4net's. I guess it shouldn't make a difference except for removing potential confusion from the reader of the code.

@forki
Copy link
Member Author

@forki forki commented on 6b7a44e May 6, 2015

Choose a reason for hiding this comment

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

Actually that test was not important. It required no application change. It was only the first thing I checked.

Please sign in to comment.