-
Notifications
You must be signed in to change notification settings - Fork 103
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
Do not include Paths_* when infering other-modules #303
Comments
RE warning suppression: I think we'd need it, and also it should go in |
Yes, I agree that it should go into |
Apparently, with
|
I've just been bitten by this. If hpack generated a |
Hey, |
@dbaynard alternatively, you can also use |
This was preventing us from uploading the package to Hackage. It was caused by #353 which (as expected) bumped the .cabal file to require version 2. Unfortunately, this ran afoul of an hpack issue: sol/hpack#303 Specifically: (a) hpack auto-adds a `Paths_*` module, and (b) Cabal v2 expects that to be listed explicitly in `autogen-modules`, which hpack does not do. Work around this by following a suggestion from the above thread: include an explicit `verbatim`/`other-modules` clause, which prevents hpack from auto-adding the `Paths_*` module.
This was preventing us from uploading the package to Hackage. It was caused by #353 which (as expected) bumped the .cabal file to require version 2. Unfortunately, this ran afoul of an hpack issue: sol/hpack#303 Specifically: (a) hpack auto-adds a `Paths_*` module, and (b) Cabal v2 expects that to be listed explicitly in `autogen-modules`, which hpack does not do. Work around this by following a suggestion from the above thread: include an explicit `verbatim`/`other-modules` clause, which prevents hpack from auto-adding the `Paths_*` module.
Is it possible to summarize in the documentation what to write to avoid having For libraries the verbatim block works well, but for executables it seems to remove hpack's ability to automatically search for |
I just hit this with the Haskell Language Server because the package doesn't have a dependency on base but the server is still led to try and evaluate the |
I recently ran into this problem in a different context (commercialhaskell/stack#5407). The TL;DR is that we had when:
- condition: false
other-modules: Paths_pkg I hope that helps other people dealing with this issue. 🌠 |
This issue is still alive and well as of 2023.03.10. Shame. |
Are there any numbers on what proportion of packages are using the weird workaround vs depending on the existing behavior? |
TL;DR
If you don't want
hpack
to includePaths_pkg
when inferringother-modules
, add this to your library/executable/test section:(substitute
pkg
with your package name)Thanks @tfausak for providing this workaround!
Problem
The main motivation for adding
Paths_*
when inferringother-modules
is: We want the user to benefit from module inference forother-modules
even if she usesPaths_*
.However, adding
Paths_*
can be problematic in certain situations (see #291 and #251).Proposed solution
Now that we have
generated-other-modules
, we could take a different approach, that is: We do not addPaths_*
by default when inferringother-modules
; a user then can add it togenerated-other-modules
when needed while still retaining module inference forother-modules
.Disadvantages
generated-other-modules
maps to Cabal'sautogen-modules
, which requirescabal-version: 2.0
package.yaml
To address (1):
When
generated-other-modules
only containsPaths_*
we will not addautogen-modules
to the generated.cabal
file and instead only addPaths_*
toother-modules
. I'm not aware of any issues with this approach, as Cabal accepts, but does not requirePaths_*
inautogen-modules
.To address (2):
To provide an upgrade path we retain the existing behavior and emit a warning when all of the following conditions are true:
other-modules
is not specified explicitlyPaths_*
Paths_*
is not mentioned explicitlyThis is a best effort approach. There is no easy way to make this bulletproof due to
CPP
and preprocessors. A small number of packages may still break due to this change.Open question: How to deal with false positives when emitting warnings? Do we need a way to silence the warning?
The text was updated successfully, but these errors were encountered: