diff --git a/src/Fable.Cli/Main.fs b/src/Fable.Cli/Main.fs index 707771c41..5316ab94a 100644 --- a/src/Fable.Cli/Main.fs +++ b/src/Fable.Cli/Main.fs @@ -965,7 +965,7 @@ let private areCompiledFilesUpToDate (state: State) (filesToCompile: string[]) = ) |> Array.forall (fun source -> let outPath = getOutPath state.CliArgs pathResolver source - // Empty files are not written to disk so we only check date for existing files + if IO.File.Exists(outPath) then foundCompiledFile <- true @@ -979,7 +979,12 @@ let private areCompiledFilesUpToDate (state: State) (filesToCompile: string[]) = upToDate else - true + // A missing output means the source is new or its output was deleted, so it + // must be compiled. A file generating empty code is never written and also + // lands here, costing that project the skip-compilation shortcut. + Log.verbose (lazy $"Output file {File.relPathToCurDir outPath} does not exist") + + false ) // If we don't find compiled files, assume we need recompilation upToDate && foundCompiledFile diff --git a/src/Fable.Compiler/ProjectCracker.fs b/src/Fable.Compiler/ProjectCracker.fs index ffdb1a941..2ebb479c8 100644 --- a/src/Fable.Compiler/ProjectCracker.fs +++ b/src/Fable.Compiler/ProjectCracker.fs @@ -837,6 +837,73 @@ let loadPrecompiledInfo (opts: CrackerOptions) otherOptions sourceFiles = Some info, otherOptions, sourceFiles | None -> None, otherOptions, sourceFiles +/// Transitive `` targets plus the implicit Directory.Build.props/targets +/// and Directory.Packages.props found upwards. These can change the `` list +/// without touching any .fsproj timestamp, so the cache must invalidate on them too. +let private getProjectImports (projFile: string) : string list = + let imports = ResizeArray() + let visited = HashSet(StringComparer.OrdinalIgnoreCase) + + // Paths built from other properties are skipped below: missing an import costs + // a stale cache, guessing wrong costs a bogus path. + let expandMacros (fileDir: string) (path: string) = + let withSep = fileDir + string IO.Path.DirectorySeparatorChar + + path.Replace("$(MSBuildThisFileDirectory)", withSep).Replace("$(MSBuildProjectDirectory)", withSep) + + let rec collectFrom (file: string) = + // Guard against import cycles and repeated visits + if visited.Add(file) && IO.File.Exists(file) then + let fileDir = IO.Path.GetDirectoryName(file: string) + + let importPaths = + try + XDocument.Load(file).Descendants() + |> Seq.filter (fun el -> el.Name.LocalName = "Import") + |> Seq.choose (fun el -> + match el.Attribute(XName.Get "Project") with + | null -> None + | attr -> Some(expandMacros fileDir attr.Value) + ) + // Unresolved properties or wildcards need a real MSBuild evaluation + |> Seq.filter (fun path -> not (path.Contains("$(") || path.Contains("*"))) + |> Seq.toList + with _ -> + // A malformed or unreadable project is the cracker's problem, not ours + [] + + for importPath in importPaths do + let fullPath = + if IO.Path.IsPathRooted(importPath) then + importPath + else + IO.Path.Combine(fileDir, importPath) + + let fullPath = IO.Path.GetFullPath(fullPath) + + if IO.File.Exists(fullPath) then + imports.Add(fullPath) + collectFrom fullPath + + collectFrom (IO.Path.GetFullPath(projFile)) + + // Implicit imports the SDK adds for every project + let projDir = IO.Path.GetDirectoryName(IO.Path.GetFullPath(projFile)) + + for implicitFile in + [ + "Directory.Build.props" + "Directory.Build.targets" + "Directory.Packages.props" + ] do + match File.tryFindUpwards implicitFile projDir with + | Some path -> + imports.Add(path) + collectFrom path + | None -> () + + List.ofSeq imports + let getFullProjectOpts (resolver: ProjectCrackerResolver) (opts: CrackerOptions) : CrackerResponse = if not (IO.File.Exists(opts.ProjFile)) then Fable.FableError("Project file does not exist: " + opts.ProjFile) |> raise @@ -862,6 +929,10 @@ let getFullProjectOpts (resolver: ProjectCrackerResolver) (opts: CrackerOptions) cacheInfo.Version = Literals.VERSION && cacheInfo.Exclude = opts.Exclude && cacheInfo.FableOptions.Language = opts.FableOptions.Language + // An imported .props/.targets can add or remove source files with no .fsproj touched + && ([ cacheInfo.ProjectPath; yield! cacheInfo.References ] + |> List.collect getProjectImports + |> List.forall isOlderThanCache) && ([ cacheInfo.ProjectPath; yield! cacheInfo.References ] |> List.forall (fun fsproj -> if IO.File.Exists(fsproj) && isOlderThanCache fsproj then diff --git a/tests/Integration/Integration/CacheInvalidationTests.fs b/tests/Integration/Integration/CacheInvalidationTests.fs new file mode 100644 index 000000000..8aeaaa701 --- /dev/null +++ b/tests/Integration/Integration/CacheInvalidationTests.fs @@ -0,0 +1,121 @@ +module Fable.Tests.CacheInvalidation + +open System +open System.IO +open Expecto + +/// A project whose compile order lives in an imported .props, so adding a source file +/// leaves the .fsproj timestamp untouched. In a temp dir: these tests mutate the project. +let private createProject (dir: string) = + Directory.CreateDirectory(dir) |> ignore + + File.WriteAllText( + Path.Combine(dir, "Test.fsproj"), + """ + + net10.0 + Major + + + +""" + ) + + File.WriteAllText( + Path.Combine(dir, "Tests.props"), + """ + + + + + +""" + ) + + File.WriteAllText(Path.Combine(dir, "Lib.fs"), "module Lib\n\nlet greeting = \"hello\"\n") + + // A second file, so deleting one output leaves another behind: with none left, the + // "no compiled files found" guard forces a recompilation and masks the case under test. + File.WriteAllText(Path.Combine(dir, "Main.fs"), "module Main\n\nlet run () = printfn \"%s\" Lib.greeting\n") + +/// Cache invalidation compares timestamps, so a file rewritten within the same tick as the +/// previous compilation could look unchanged. Nudge it forward to keep the test from flaking. +let private writeNewer (path: string) (content: string) = + File.WriteAllText(path, content) + File.SetLastWriteTime(path, DateTime.Now.AddSeconds(2.0)) + +let private compile (dir: string) (outDir: string) = + Fable.Cli.Entry.main + [| + Path.Combine(dir, "Test.fsproj") + "--cwd" + dir + "--lang" + "javascript" + "--outDir" + outDir + |] + +let private withTempProject name (f: string -> string -> unit) = + let unique = Guid.NewGuid().ToString("N") + let dir = Path.Combine(Path.GetTempPath(), $"fable-cache-tests-%s{name}-%s{unique}") + + try + createProject dir + f dir (Path.Combine(dir, "out")) + finally + try + Directory.Delete(dir, true) + with _ -> + () + +let tests = + testList + "CacheInvalidation" + [ + // A `` arriving through an imported .props touches no .fsproj + // timestamp, so the options cache used to be reused with a stale source list. + testCase "Source file added via an imported .props is compiled" + <| fun () -> + withTempProject + "added" + (fun dir outDir -> + Expect.equal (compile dir outDir) 0 "First compilation should succeed" + + writeNewer (Path.Combine(dir, "Scalars.fs")) "module Scalars\n\nlet tests = \"scalars\"\n" + + writeNewer + (Path.Combine(dir, "Tests.props")) + """ + + + + + + +""" + + Expect.equal (compile dir outDir) 0 "Second compilation should succeed" + + Expect.isTrue + (File.Exists(Path.Combine(outDir, "Scalars.js"))) + "The file added through the imported .props should have been generated" + ) + + // `getFilesToCompile` selects a source whose output is missing, so the up-to-date + // check must not then skip the compilation it just asked for. + testCase "Deleted output file is regenerated" + <| fun () -> + withTempProject + "deleted" + (fun dir outDir -> + Expect.equal (compile dir outDir) 0 "First compilation should succeed" + + let outFile = Path.Combine(outDir, "Lib.js") + Expect.isTrue (File.Exists outFile) "Expected the first compilation to generate Lib.js" + File.Delete outFile + + Expect.equal (compile dir outDir) 0 "Second compilation should succeed" + Expect.isTrue (File.Exists outFile) "The deleted output file should have been regenerated" + ) + ] diff --git a/tests/Integration/Integration/Fable.Tests.Integration.fsproj b/tests/Integration/Integration/Fable.Tests.Integration.fsproj index a87aacd98..ca408a70d 100644 --- a/tests/Integration/Integration/Fable.Tests.Integration.fsproj +++ b/tests/Integration/Integration/Fable.Tests.Integration.fsproj @@ -17,6 +17,7 @@ + diff --git a/tests/Integration/Integration/Main.fs b/tests/Integration/Integration/Main.fs index 3346c5e75..3ff92dce0 100644 --- a/tests/Integration/Integration/Main.fs +++ b/tests/Integration/Integration/Main.fs @@ -6,6 +6,7 @@ let allTests = Cli.tests FileWatcher.tests CompilationTests.tests + CacheInvalidation.tests ] open Expecto