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

Fix fast restore #3608

Merged
merged 6 commits into from
Jul 3, 2019
Merged

Fix fast restore #3608

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift click to select a range
0b10fd7
Add (failing) integration test for #2684.
matthid Jul 1, 2019
fe85d95
fix regression
matthid Jul 1, 2019
d8302c7
Fix tests and some more msbuild foo to handle invalid cache files.
matthid Jul 2, 2019
fabcf81
add reference to https://github.com/fsharp/FAKE/issues/2348
matthid Jul 2, 2019
241b159
Merge branch 'master' of github.com:fsprojects/Paket into fix_fast_re…
matthid Jul 2, 2019
b004afe
fix tests by using proper exception type...
matthid Jul 2, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion integrationtests/Paket.IntegrationTests/InfoSpecs.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 17,7 @@ let ``#3200 info should locate paket.dependencies``() =

let ``paket info --paket-dependencies-dir`` workingDir =
directPaketInPathEx "info --paket-dependencies-dir" workingDir
|> Seq.map PaketMsg.getMessage
|> Seq.map OutputMsg.getMessage
|> List.ofSeq

// paket.dependencies not exists
Expand Down
4 changes: 3 additions & 1 deletion integrationtests/Paket.IntegrationTests/InitSpecs.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 29,9 @@ let ``#1743 empty log file``() =
use __ = paket "init --log-file" "i001040-init-downloads-bootstrapper" |> fst
failwith "expected error"
with
| exn when exn.Message.Split('\n').[0].Contains "--log-file" -> ()
| ProcessFailedWithExitCode(_, _, msgs) ->
(msgs.Errors |> Seq.head).Contains "--log-file"
|> shouldEqual true

[<Test>]
#if PAKET_NETCORE
Expand Down
4 changes: 2 additions & 2 deletions integrationtests/Paket.IntegrationTests/LoadingScriptGenerationTests.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 144,7 @@ let ``fails on wrong framework given`` () =

use __ = paket "install" scenario |> fst

let failure = Assert.Throws (fun () ->
let failure = Assert.Throws<ProcessFailedWithExitCode> (fun () ->
let result = directPaket "generate-load-scripts framework foo framework bar framework net45" scenario
printf "%s" result
)
Expand All @@ -161,7 161,7 @@ let ``fails on wrong scripttype given`` () =

use __ = paket "install" scenario |> fst

let failure = Assert.Throws (fun () ->
let failure = Assert.Throws<ProcessFailedWithExitCode> (fun () ->
let result = directPaket (sprintf "generate-load-scripts type foo type bar framework net45") scenario
printf "%s" result
)
Expand Down
27 changes: 27 additions & 0 deletions integrationtests/Paket.IntegrationTests/RestoreSpecs.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 8,33 @@ open FsUnit
open Paket
open Paket.Utils

[<Test>]
let ``#3608 dotnet build should work with unparsable cache``() =
let project = "console"
let scenario = "i003608-invalid-cache"
use __ = prepareSdk scenario
let wd = (scenarioTempPath scenario) @@ project
// Build should work immediately (and call 'paket restore')
directDotnet true (sprintf "build %s.fsproj" project) wd
|> ignore

[<Test>]
let ``#2684 Paket should not be called the second time in msbuild (Restore Performance)``() =
// NOTE: This test also ensure that FAKE can be used without paket on the CI server, see https://github.com/fsharp/FAKE/issues/2348
let project = "console"
let scenario = "i002684-fast-restore"
use __ = prepareSdk scenario

let wd = (scenarioTempPath scenario) @@ project
// first call paket restore (to restore and to extract the targets file as well as emulate a "full" restore)
directPaket "restore" scenario |> ignore
// second time no more paket calls should be required (as we already did a full restore)
directDotnetEx [ "PAKET_ERROR_ON_MSBUILD_EXEC", "true" ] true (sprintf "restore %s.fsproj" project) wd
|> ignore
// make sure it builds as well (checks if restore-targets contains syntax errors)
directDotnet true (sprintf "build %s.fsproj" project) wd
|> ignore

[<Test>]
let ``#2496 Paket fails on projects that target multiple frameworks``() =
let project = "EmptyTarget"
Expand Down
47 changes: 30 additions & 17 deletions integrationtests/Paket.IntegrationTests/TestHelper.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 92,23 @@ let prepareSdk scenario =
FileHelper.CopyFile tmpPaketFolder targetsFile
cleanup

type PaketMsg =
type OutputMsg =
{ IsError : bool; Message : string }
static member isError ({ IsError = e}:PaketMsg) = e
static member getMessage ({ Message = msg }:PaketMsg) = msg
static member isError ({ IsError = e}:OutputMsg) = e
static member getMessage ({ Message = msg }:OutputMsg) = msg
type OutputData =
internal { _Messages : ResizeArray<OutputMsg> }
member x.Messages : seq<OutputMsg> = x._Messages :> _
member x.Errors = x._Messages |> Seq.filter OutputMsg.isError |> Seq.map OutputMsg.getMessage
member x.Outputs = x._Messages |> Seq.filter (not << OutputMsg.isError) |> Seq.map OutputMsg.getMessage

exception ProcessFailedWithExitCode of exitCode:int * fileName:string * msgs:OutputData
with
override x.Message =
let output = String.Join(Environment.NewLine,x.msgs.Messages |> Seq.map (fun m -> (if m.IsError then "ERR:" else "OUT:") m.Message ))
sprintf "The process '%s' exited with code %i, output: \n%s" x.fileName x.exitCode output

let directToolEx isPaket toolInfo commands workingDir =
let directToolEx env isPaket toolInfo commands workingDir =
let processFilename, processArgs =
match fst toolInfo, snd toolInfo with
| "", path ->
Expand All @@ -123,7 134,7 @@ let directToolEx isPaket toolInfo commands workingDir =
Environment.SetEnvironmentVariable("PAKET_DETAILED_WARNINGS", "true")
printfn "%s> %s %s" workingDir (if isPaket then "paket" else processFilename) processArgs
let perfMessages = ResizeArray()
let msgs = ResizeArray<PaketMsg>()
let msgs = ResizeArray<OutputMsg>()
let mutable perfMessagesStarted = false
let addAndPrint isError msg =
if not isError then
Expand All @@ -138,6 149,8 @@ let directToolEx isPaket toolInfo commands workingDir =
try
ExecProcessWithLambdas (fun info ->
info.FileName <- processFilename
for (key, value) in env do
info.EnvironmentVariables.[key] <- value
info.WorkingDirectory <- workingDir
info.CreateNoWindow <- true
info.Arguments <- processArgs)
Expand All @@ -163,38 176,38 @@ let directToolEx isPaket toolInfo commands workingDir =
if isPaket then
// Only throw after the result <> 0 check because the current test might check the argument parsing
// this is the only case where no performance is printed
let isUsageError = result <> 0 && msgs |> Seq.filter PaketMsg.isError |> Seq.map PaketMsg.getMessage |> Seq.exists (fun msg -> msg.Contains "USAGE:")
let isUsageError = result <> 0 && msgs |> Seq.filter OutputMsg.isError |> Seq.map OutputMsg.getMessage |> Seq.exists (fun msg -> msg.Contains "USAGE:")
if not isUsageError then
printfn "Performance:"
for msg in perfMessages do
printfn "%s" msg

if result <> 0 then
let errors = String.Join(Environment.NewLine,msgs |> Seq.filter PaketMsg.isError |> Seq.map PaketMsg.getMessage)
if String.IsNullOrWhiteSpace errors then
failwithf "The process exited with code %i" result
else
failwith errors
if result <> 0 then
raise <| ProcessFailedWithExitCode(result, processFilename, { _Messages = msgs })

msgs
#endif

let directPaketInPathEx command scenarioPath =
directToolEx true paketToolPath command scenarioPath
directToolEx [] true paketToolPath command scenarioPath

let checkResults msgs =
msgs
|> Seq.filter PaketMsg.isError
|> Seq.filter OutputMsg.isError
|> Seq.toList
|> shouldEqual []

let directDotnet checkZeroWarn command workingDir =
let msgs = directToolEx false ("", dotnetToolPath) command workingDir
let directDotnetEx env checkZeroWarn command workingDir =
let msgs = directToolEx env false ("", dotnetToolPath) command workingDir
if checkZeroWarn then checkResults msgs
msgs


let directDotnet checkZeroWarn command workingDir =
directDotnetEx [] checkZeroWarn command workingDir

let private fromMessages msgs =
String.Join(Environment.NewLine,msgs |> Seq.map PaketMsg.getMessage)
String.Join(Environment.NewLine,msgs |> Seq.map OutputMsg.getMessage)

let directPaketInPath command scenarioPath = directPaketInPathEx command scenarioPath |> fromMessages

Expand Down
8 changes: 8 additions & 0 deletions integrationtests/scenarios/i002684-fast-restore/before/console/Program.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 1,8 @@
// Learn more about F# at http://fsharp.org

open System

[<EntryPoint>]
let main argv =
printfn "Hello World from F#!"
0 // return an integer exit code
13 changes: 13 additions & 0 deletions integrationtests/scenarios/i002684-fast-restore/before/console/console.fsproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 1,13 @@
<?xml version="1.0" encoding="utf-8"?>
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>netcoreapp2.1</TargetFramework>
</PropertyGroup>

<ItemGroup>
<Compile Include="Program.fs" />
</ItemGroup>

<Import Project="..\.paket\Paket.Restore.targets" />
</Project>
1 change: 1 addition & 0 deletions integrationtests/scenarios/i002684-fast-restore/before/console/paket.references
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 1 @@
FSharp.Core
4 changes: 4 additions & 0 deletions integrationtests/scenarios/i002684-fast-restore/before/paket.dependencies
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 1,4 @@
source https://api.nuget.org/v3/index.json
restriction: || (==netstandard20) (== netcoreapp21)
storage: none
nuget FSharp.Core
5 changes: 5 additions & 0 deletions integrationtests/scenarios/i002684-fast-restore/before/paket.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 1,5 @@
STORAGE: NONE
RESTRICTION: || (== netcoreapp2.1) (== netstandard2.0)
NUGET
remote: https://api.nuget.org/v3/index.json
FSharp.Core (4.6.2)
8 changes: 8 additions & 0 deletions integrationtests/scenarios/i003608-invalid-cache/before/console/Program.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 1,8 @@
// Learn more about F# at http://fsharp.org

open System

[<EntryPoint>]
let main argv =
printfn "Hello World from F#!"
0 // return an integer exit code
13 changes: 13 additions & 0 deletions integrationtests/scenarios/i003608-invalid-cache/before/console/console.fsproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 1,13 @@
<?xml version="1.0" encoding="utf-8"?>
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>netcoreapp2.1</TargetFramework>
</PropertyGroup>

<ItemGroup>
<Compile Include="Program.fs" />
</ItemGroup>

<Import Project="..\.paket\Paket.Restore.targets" />
</Project>
1 change: 1 addition & 0 deletions integrationtests/scenarios/i003608-invalid-cache/before/console/paket.references
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 1 @@
FSharp.Core
1 change: 1 addition & 0 deletions integrationtests/scenarios/i003608-invalid-cache/before/paket-files/paket.restore.cached
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 1 @@
Some not parsable File
4 changes: 4 additions & 0 deletions integrationtests/scenarios/i003608-invalid-cache/before/paket.dependencies
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 1,4 @@
source https://api.nuget.org/v3/index.json
restriction: || (==netstandard20) (== netcoreapp21)
storage: none
nuget FSharp.Core
5 changes: 5 additions & 0 deletions integrationtests/scenarios/i003608-invalid-cache/before/paket.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 1,5 @@
STORAGE: NONE
RESTRICTION: || (== netcoreapp2.1) (== netstandard2.0)
NUGET
remote: https://api.nuget.org/v3/index.json
FSharp.Core (4.6.2)
5 changes: 4 additions & 1 deletion src/Paket.Core/Installation/InstallProcess.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -585,7 585,10 @@ let InstallIntoProjects(options : InstallerOptions, forceTouch, dependenciesFile
first := false

let restoreCacheFile = Path.Combine(root, Constants.PaketRestoreHashFilePath)
Paket.RestoreProcess.saveToFile (lockFile.ToString()) (FileInfo restoreCacheFile) |> ignore
let hash = Paket.RestoreProcess.getLockFileHashFromContent (lockFile.ToString())
// NIT: We probably need to check if we have really fully restored (could be partial install)
// Lets see if users report issues
Paket.RestoreProcess.writeRestoreCache restoreCacheFile { PackagesDownloadedHash = hash; ProjectsRestoredHash = hash }
Paket.RestoreProcess.WriteGitignore restoreCacheFile

for project, _ in projectsAndReferences do
Expand Down
13 changes: 9 additions & 4 deletions src/Paket.Core/Installation/RestoreProcess.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 182,7 @@ let copiedElements = ref false

type private MyAssemblyFinder () = class end

let saveToFile newContent (targetFile:FileInfo) =
let private saveToFile newContent (targetFile:FileInfo) =
let rec loop trials =
try
if not targetFile.Directory.Exists then
Expand Down Expand Up @@ -567,6 567,7 @@ let internal getStringHash (s:string) =
|> BitConverter.ToString
|> fun s -> s.Replace("-", "")


type internal Hash =
| Hash of string
member x.HashString =
Expand All @@ -576,7 577,11 @@ type internal Hash =
match x with
| Hash s -> String.IsNullOrEmpty s
static member OfString s = Hash (getStringHash s)


let internal getLockFileHashFromContent (content:string) =
Hash.OfString content
let internal getLockFileHash (f:string) =
getLockFileHashFromContent (File.ReadAllText f)

type internal RestoreCache =
{ PackagesDownloadedHash : Hash
Expand All @@ -591,7 596,7 @@ type internal RestoreCache =
{ PackagesDownloadedHash = Hash ""
ProjectsRestoredHash = Hash "" }

let private writeRestoreCache (file:string) { PackagesDownloadedHash = Hash packagesDownloadedHash; ProjectsRestoredHash = Hash projectsRestoredHash} =
let internal writeRestoreCache (file:string) { PackagesDownloadedHash = Hash packagesDownloadedHash; ProjectsRestoredHash = Hash projectsRestoredHash} =
let jobj = new Newtonsoft.Json.Linq.JObject()
jobj.["packagesDownloadedHash"] <- Newtonsoft.Json.Linq.JToken.op_Implicit packagesDownloadedHash
jobj.["projectsRestoredHash"] <- Newtonsoft.Json.Linq.JToken.op_Implicit projectsRestoredHash
Expand Down Expand Up @@ -668,7 673,7 @@ let Restore(dependenciesFileName,projectFile:RestoreProjectOptions,force,group,i
None, RestoreCache.Empty, Hash "", false
else
let cache = readRestoreCache(lockFileName)
let lockFileHash = Hash.OfString (File.ReadAllText lockFileName.FullName)
let lockFileHash = getLockFileHash lockFileName.FullName
let updatedCache =
{ PackagesDownloadedHash = lockFileHash // we always download all packages in that situation
ProjectsRestoredHash = if projectFile = AllProjects then lockFileHash else cache.ProjectsRestoredHash }
Expand Down