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.

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

cabal new-build does not track files added via TH’s addDependentFile #4746

Open
cocreature opened this issue Sep 6, 2017 · 62 comments
Open

Comments

@cocreature
Copy link
Collaborator

Steps to reproduce

Expected behavior

cabal new-build rebuilds because fileinput has changed.

Actual behavior

cabal new-build exits without rebuilding after saying Up to date.

This issue affects both cabal-2.0 as well as the master branch.

@phadej
Copy link
Collaborator

phadej commented Sep 6, 2017

One hack solution is to watch extra-source-files too. But that won't solve dynamic (e.g. what inline-c uses?) stuff.

@23Skidoo
Copy link
Member

23Skidoo commented Sep 6, 2017

BTW, is there a way to force ghc --make always rerun a TH function? E.g. if I'm doing some kind of I/O at compile-time, like reading from an environment variable.

@phadej
Copy link
Collaborator

phadej commented Sep 6, 2017

@23Skidoo that would force GHC to always build that module, that doesn't sound like a very good idea.

@23Skidoo
Copy link
Member

23Skidoo commented Sep 6, 2017

Yep, but sometimes you want that in a build system. For example, Shake rules can be marked alwaysRerun.

Re: extra-source-files, that field is package-global and commonly used for things like README and changelog. So we'd need to allow per-component extra-source-files to avoid unnecessary recompilation.

@23Skidoo
Copy link
Member

23Skidoo commented Sep 7, 2017

An alternative way to fix this is to get the list of files to be monitored from ghc --make via -ddump-module-graph (a new feature that is going to be in 8.4).

@phadej
Copy link
Collaborator

phadej commented Sep 7, 2017

Does -ddump-module-graph dump addDependentFile & co stuff?

@23Skidoo
Copy link
Member

23Skidoo commented Sep 7, 2017

@phadej If you click that link, you'll see that I asked that exact question there.

@cocreature
Copy link
Collaborator Author

Given that this feature will only be in GHC 8.4 I don’t think that’s a reasonable solution at this point.

@23Skidoo
Copy link
Member

23Skidoo commented Sep 7, 2017

@cocreature With old versions we can simulate that feature with ghc -M (which will be slower because we'll have to do dependency resolution twice - once for ghc -M and once for ghc --make, but for legacy GHC versions we can afford to not care). BTW, ghc -M does support addDependentFile dependencies:

# DO NOT DELETE: Beginning of Haskell dependencies
Data/FileEmbed.so : Data/FileEmbed.hs
Main.so : Main.hs
Main.so : Data/FileEmbed.shi
# DO NOT DELETE: End of Haskell dependencies

@hvr
Copy link
Member

hvr commented Sep 7, 2017

@23Skidoo btw, we can avoid paying for the slow-legacy fallback when it's not needed, because we know when TH is needed (we already require TemplateHaskell to be declared in *-extensions in order to enable other logic specific to TH). So I think that's a workable fallback.

@hvr
Copy link
Member

hvr commented Aug 8, 2018

Unfortunately we not only missed the boat for GHC 8.4, but most likely also for GHC 8.6 to get supporting features into GHC... as https://phabricator.haskell.org/D3898 got stuck on a decision which design path to go down... :-/

PS: Screen-scraping GHC's dump-hi output would be an obvious but terrible hack for the obvious reasons and since it's not intended to be consumed by tooling its format is neither documented nor guaranteed to be correct and can change without notice.

@ygale
Copy link
Contributor

ygale commented Nov 18, 2018

Upvoting this issue. It is critical for larger web-dev shops that have people who are totally focused on front-end web UI development. They require instant reload; they don't have time to poke around at files manually every time to induce rebuilds.

We are a yesod shop, so we are using stack. For comparison, stack detects this by parsing dump-hi files. The syntax change in dump-hi files in GHC 8.4 caused a similar issue in stack for a time, now fixed.

@tom-audm
Copy link

Another upvote for this issue!

I think type:bug is a more accurate description than type: enhancement. It's a regression from cabal old-build's behavior, and can cause truly wtf moments when using e.g. gitrev (see my issue #5866 ). I'd say it actually breaks the gitrev package for practical usage.

@kaldonir
Copy link

We ran into the same issue today. Is there any progress here?

@phadej
Copy link
Collaborator

phadej commented Mar 10, 2020

@kaldonir, no. As far as I know no-one is actively working on adding necessary features to GHC (e.g. ghc-proposals/ghc-proposals#245 or something similar so the compiler could tell Cabal what extra dependencies exist).

Alternatively or additionally cabal could track extra-source-files, which is imprecise yet better than nothing. Again, I'm not aware anybody is working on that either.

@kmicklas
Copy link
Collaborator

@phadej Are you opposed to making cabal track extra-source-files as an intermediate step? This wouldn't help with gitrev but would solve this for I suspect most use cases, until we have a proper fix in GHC. I'm willing to implement this if there's support for it.

@phadej
Copy link
Collaborator

phadej commented Jun 10, 2020

I'm not. It should be easy, I think it should go to the same place as c2f4625

PR welcome!

kmicklas added a commit to kmicklas/cabal that referenced this issue Jun 11, 2020
This should alleviate haskell#4746 for most use cases, if the dependent files
are declared in `extra-source-files`. The downside is that `cabal`
will not be able to avoid invoking GHC in as many cases, but GHC's
`--make` should be smart enough to avoid any "actual" rebuilding that
isn't necessary.
@ulysses4ever
Copy link
Collaborator

@Mikolaj we should give the permissions to Michael, I think

@Mikolaj
Copy link
Member

Mikolaj commented Dec 13, 2022

No way, @michaelpj, I've just checked and you weren't even a lowly Read-level member of the project according to the Settings page. I've invited you to the respectable Triage level. Could you try?

@michaelpj
Copy link
Collaborator

done

@Mikolaj
Copy link
Member

Mikolaj commented Dec 13, 2022

Perfect. Thank you.

@NorfairKing
Copy link

+1 Still an issue it seems.

@hsyl20
Copy link
Collaborator

hsyl20 commented May 16, 2023

Stack's code for parsing .hi files is in a separate library: https://github.com/commercialhaskell/hi-file-parser. I've recently updated it to support 9.4.5 and 9.6.1. Cabal could use it to detect dynamic dependencies per module. The dependency on RIO could easily be removed.

@NorfairKing
Copy link

Linking this to #8605 because I can't start using cabal until this is fixed.

@gbaz
Copy link
Collaborator

gbaz commented May 27, 2023

Thinking about hi based tracking I think there's a potential issue: if files are not in the .cabal manifest they still won't get packaged up by an sdist. So that means that an sdist of a package that works locally with some "dynamic" tracking will fail if packaged up. I think that's not a good situation. As it stands, I'm not sure why tracking extra-src-files for rebuild along with glob-expansion of them doesn't suffice.

martijnbastiaan added a commit to bittide/bittide-hardware that referenced this issue Aug 18, 2023
Cabal/GHC doesn't know about files produced by cargo, and will therefore
fail to invalidate caches. While there are ways to tell Cabal/GHC to
depend on these files, they are known to be broken in our tool versions.

See: haskell/cabal#4746

Note that we don't use `cabal clean`, as it will also remove any sources
it downloaded (specified in `cabal.project`).
martijnbastiaan added a commit to bittide/bittide-hardware that referenced this issue Aug 18, 2023
Cabal/GHC doesn't know about files produced by cargo, and will therefore
fail to invalidate caches. While there are ways to tell Cabal/GHC to
depend on these files, they are known to be broken in our tool versions.

See: haskell/cabal#4746

Note that we don't use `cabal clean`, as it will also remove any sources
it downloaded (specified in `cabal.project`).
martijnbastiaan added a commit to bittide/bittide-hardware that referenced this issue Aug 18, 2023
Cabal/GHC doesn't know about files produced by cargo, and will therefore
fail to invalidate caches. While there are ways to tell Cabal/GHC to
depend on these files, they are known to be broken in our tool versions.

See: haskell/cabal#4746

Note that we don't use `cabal clean`, as it will also remove any sources
it downloaded (specified in `cabal.project`).
martijnbastiaan added a commit to bittide/bittide-hardware that referenced this issue Aug 18, 2023
Cabal/GHC doesn't know about files produced by cargo, and will therefore
fail to invalidate caches. While there are ways to tell Cabal/GHC to
depend on these files, they are known to be broken in our tool versions.

See: haskell/cabal#4746

Note that we don't use `cabal clean`, as it will also remove any sources
it downloaded (specified in `cabal.project`).
martijnbastiaan added a commit to bittide/bittide-hardware that referenced this issue Aug 18, 2023
Cabal/GHC doesn't know about files produced by cargo, and will therefore
fail to invalidate caches. While there are ways to tell Cabal/GHC to
depend on these files, they are known to be broken in our tool versions.

See: haskell/cabal#4746

Note that we don't use `cabal clean`, as it will also remove any sources
it downloaded (specified in `cabal.project`).
@KristianBalaj
Copy link

I think this is a related issue I've just created - #9745

@NorfairKing
Copy link

I'm very surprised to see this being labeled as "priority:low" given that I consider this a dealbreaking "cannot consider cabal usable until fixed"-bug.

@Mikolaj
Copy link
Member

Mikolaj commented Jun 24, 2024

@NorfairKing: I've bumped priority. PRs welcome. Actually, design welcome first, IIRC? Especially that it's marked as blocked on GHC, so we either need to create a patch for GHC or devise a special workaround?

@NorfairKing
Copy link

@Mikolaj I don't understand how this is blocked on GHC when stack has not had this bug since 2015.

@Mikolaj
Copy link
Member

Mikolaj commented Jun 24, 2024

Me neither, but it's a long time since I looked into this long discussion thread (assuming I understood it then). Do you think things may have changed since then? Would you like to re-assess?

@TeofilC
Copy link
Collaborator

TeofilC commented Jun 24, 2024

My understanding is that stack gets this information by parsing binary .hi files using (See: commercialhaskell/stack#5134).

https://gitlab.haskell.org/ghc/ghc/-/merge_requests/11994 is some movement towards getting a proper interface for exposing the dependency graph from the GHC side. Though I'm not sure if that includes TH dependencies

@geekosaur
Copy link
Collaborator

@Mikolaj I don't understand how this is blocked on GHC when stack has not had this bug since 2015.

Only GHC knows the result of running TH (do you really want cabal duplicating enough of GHC that it can run TH itself? Remember that you can do conditionals) so it would have been blocked on GHC exporting that information usefully. Later messages in this thread indicate it's now part of the .hi file, so presumably this can proceed now.

@Mikolaj
Copy link
Member

Mikolaj commented Jun 24, 2024

Thank you for your remarks.

@phadej has also educated me on this issue just now:

"
The #4746 is softly fixed in Cabal too. If you add a file to extra-source-files, it will track its dirtyness. Maybe stack just tracks everything (i.e. overestimates even more). To fix that issue exactly, GHC must tell what files were actually used, so they could be recorded as an exact dependency.
like CPP implementation tell which headers were actually included when you preprocess a source file.
(FWIW, these aren't recorded either, it's just not so obvious)
https://gitlab.haskell.org/ghc/ghc/-/issues/25017
addDependentFile dependencies are at least recorded in the interface files, so if one really wants, then by parsing ghc --show-iface output one can figure those out.
"

So maybe we could look into the .hi files. Worth investigating. Volunteers welcome.

@ulysses4ever
Copy link
Collaborator

So maybe we could look into the .hi files.

That's what Sylvain said above: Stack parses hi for this reason (not sure alone or not) with https://github.com/commercialhaskell/hi-file-parser, and we should adopt it.

@phadej
Copy link
Collaborator

phadej commented Jun 25, 2024

@ulysses4ever

https://github.com/commercialhaskell/hi-file-parser, and we should adopt it.

No you shouldn't. see e.g. commercialhaskell/stack#5134.

Stack does some not so good design choices, and that's a good example of a bad design.

But if you want one more thing to slow down making recent ghc compatible releases, go ahead: AFAICT, hi-file-parser doesn't support GHC-9.10, at the very least it's not tested: the last commit is from 7 months ago.

@mpickering
Copy link
Collaborator

I completely agree with Oleg that depending on "hi-file-parser" is a bad idea.

@ulysses4ever
Copy link
Collaborator

@phadej @mpickering what are our options?

Reading Mikolaj's post above, I thought that Oleg supports the idea of parsing .hi files. Is there anything wrong with hi-file-parser in particular or is there anything else that is both better and achievable with our limited resources?

This issue looks important for some amount of users. Feature-parity with stack is a desirable goal too (modulo some things like installing GHC). I wish we could come to an agreement on some way forward for this.

@phadej
Copy link
Collaborator

phadej commented Jun 28, 2024

I thought that Oleg supports the idea of parsing .hi files.

I didn't say that, and I only implied that one could parse ghc --show-iface output, but even that is fragile, and thus bad idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests