fpco / inline-c

284 stars 49 forks source link

`C.includes` does not track changes #152

Open MathiasSven opened 9 months ago

MathiasSven commented 9 months ago

I am not sure if this is a cabal only issue or if it is an issue with inline-c too, but like with test.h under inline-c-cpp/test, I have a local header file that I include in my hs file, however when I make changes to it, there is no recompilation.

There seems to be an open issue for "cabal new-build does not track files added via TH’s addDependentFile", but in it there is also a comment that says that even if the issue is closed, that wouldn't fix inline-c's use case.

What is the best solution at the moment to deal with this issue? Is there a manual way that users of inline-c can tell cabal to watch specific files for changes?

junjihashimoto commented 9 months ago

Hi @MathiasSven , I tried it with ghc-9.6.3 and caba-3.10.2.0. At first, compile inline-c-cpp, next add comments to test.h, then ghc recompiles c++ sources. Without adding comments, ghc does not recompile them.

MathiasSven commented 9 months ago

@junjihashimoto I have made a minimal rep repo with nix at https://github.com/MathiasSven/inline-c-152-rep, perhaps I am missing some configuration, but from looking at the inline-c-cpp/inline-c-cpp.cabal the only thing that mentions the header file is extra-source-files which I have added to my cabal file.

Steps I took:

> nix develop
> ghc --version
The Glorious Glasgow Haskell Compilation System, version 9.6.3
> cabal --version
cabal-install version 3.10.2.1
compiled using version 3.10.2.1 of the Cabal library
> 
> cabal run -v0
2
> sed -i 's/1/5/g' src/test.hpp
> cabal run -v0
2 # Should have been 6, no recompilation happend
> echo "" >> src/Main.hs
> cabal run -v0
6
junjihashimoto commented 9 months ago

@MathiasSven Thank you for creating the example. I also tried it. https://gist.github.com/junjihashimoto/827d51d5a9de639280a7e7bb2ea6ce7a

Cabal detects updating src/test.hpp, and it reruns ghc, but the binary is not updated. https://gist.github.com/junjihashimoto/827d51d5a9de639280a7e7bb2ea6ce7a#file-gistfile1-txt-L25

So it seems that the issue is ghc not cabal.

junjihashimoto commented 9 months ago

When you pass the -fforce-recomp option to ghc, it seems to work as expected. Recompilation will not be executed if cabal determines that there is no update.

MathiasSven commented 9 months ago

@junjihashimoto That would make so whenever there are any changes in an included file, all modules would be recompiled. Couldn't inline-c add Language.Haskell.TH.Syntax.addDependentFile logic to its C.include? I have tested using addDependentFile by adding this to my Main.hs and it seems to work:

$(addDependentFile "/tmp/tmp.foSjrQBSoX/inline-c-152-rep/src/test.hpp" >> return [])

I have added a branch to the example repo which uses filepath and directory packages in order to turn addDependentFile into a relative import (like C.include) and it seems to behave exactly like I would expect. As in:

MathiasSven commented 9 months ago

For my use case, it seems to work just fine to simply add addDependentFile to include:

{-# LANGUAGE PackageImports #-}

module Language.C.Inline.Cpp
  ( module Upstream
  , include
  )
where

import "inline-c-cpp" Language.C.Inline.Cpp as Upstream hiding (include)

import Language.Haskell.TH.Syntax (addDependentFile)
import Language.Haskell.TH        (DecsQ, runIO, location, loc_filename)
import System.FilePath            ((</>), takeDirectory)
import System.Directory           (getCurrentDirectory)
import Control.Monad              (void)

include :: String -> DecsQ
include s
  | null s = fail "inline-c: empty string (include)"
  | head s == '<' = verbatim $ "#include " ++ s
  | otherwise = do
      void $ addDependentFileRelative s
      verbatim $ "#include \"" ++ s ++ "\""

addDependentFileRelative :: FilePath -> DecsQ
addDependentFileRelative relativeFile = do
    currentFilename <- loc_filename <$> location
    pwd             <- runIO getCurrentDirectory

    let invocationRelativePath = takeDirectory (pwd </> currentFilename) </> relativeFile

    addDependentFile invocationRelativePath

    return []

branch for reference

junjihashimoto commented 8 months ago

@MathiasSven Thank you for finding the solution! The PR is welcome.

MathiasSven commented 8 months ago

I will make a draft PR, although this works for my use case specifically, I don't know if it would be appropriate for every use case.