PackageInfo functionality is not guarded behind cabal-version.

[phadej]

Given a package with just foo.cabal:

cabal-version: 2.4
name: foo
version: 0
license: BSD-3-Clause
license-file: LICENSE
maintainer: Someone
category: Example
synopsis: Foo
description: FooBar
build-type: Simple

library
  default-language: Haskell2010
  build-depends: base <5
  autogen-modules: PackageInfo_foo
  exposed-modules: PackageInfo_foo

cabal-install-3.10.1.0 check doesn't complaint, and cabal build succeeds:

% ~/.ghcup/bin/cabal-3.10.1.0 check
No errors or warnings could be found in the package.

% ~/.ghcup/bin/cabal-3.10.1.0 build
Resolving dependencies...
Build profile: -w ghc-8.6.5 -O1
In order, the following will be built (use -v for more details):
 - foo-0 (lib) (configuration changed)
Configuring library for foo-0..
Preprocessing library for foo-0..
Building library for foo-0..
[1 of 1] Compiling PackageInfo_foo  ( ... )

---

However older cabal-installs, or if I change to build-type: Custom (and use say GHC-9.2.7) fails.

Older cabal-install (note, you need to clear dist-newstyle in between so autogenerated left-overs are removed):

% ~/.ghcup/bin/cabal-3.6.2.0 check 
No errors or warnings could be found in the package.

[polinukli] /codetmp/foo % ~/.ghcup/bin/cabal-3.6.2.0 build
Build profile: -w ghc-8.6.5 -O1
In order, the following will be built (use -v for more details):
 - foo-0 (lib) (first run)
Preprocessing library for foo-0..
cabal-3.6.2.0: can't find source for PackageInfo_foo in .,
/codetmp/foo/dist-newstyle/build/x86_64-linux/ghc-8.6.5/foo-0/build/autogen,
/codetmp/foo/dist-newstyle/build/x86_64-linux/ghc-8.6.5/foo-0/build/global-autogen

or with build-type: Custom:

% grep -A 4 build-type foo.cabal                
build-type: Custom

custom-setup
  setup-depends: base, Cabal

[polinukli] /codetmp/foo % ~/.ghcup/bin/cabal-3.10.1.0 build -w ghc-9.2.7
Build profile: -w ghc-9.2.7 -O1
In order, the following will be built (use -v for more details):
 - foo-0 (lib:foo) (cannot read state cache)
Configuring foo-0...
Preprocessing library for foo-0..
setup: can't find source for PackageInfo_foo in .,
/codetmp/foo/dist-newstyle/build/x86_64-linux/ghc-9.2.7/foo-0/build/autogen,
/codetmp/foo/dist-newstyle/build/x86_64-linux/ghc-9.2.7/foo-0/build/global-autogen

Error: cabal-3.10.1.0: Failed to build foo-0.

---

Features changing cabal_macros.h, Paths_pkgname.hs etc SHOULD be guarded behind cabal-version.

[andreabedini]

What can we do to prevent this kind of mistakes? The only things I can think of are:

1. some automation to add warnings and extra checks when a PR changes particular files
2. use something like structureHash to make sure changes in some datatypes are visible and acknowledged.

[phadej]

For each feature request / pull request you (= maintainers) should always answer the question "is this a change which should be guarded by cabal-version: ..."

(c.f. GHC folks ask themselves "does this need a GHC proposal").

Either it's clearly doesn't need to, or if someone is unsure, they should discuss. This kind of mistakes are very bad. IMO they also should be the highest priority to fix (or revert) - at least before Hackage starts using the affected Cabal versions. And it already does https://github.com/haskell/hackage-server/blob/30a4d8404dd9b70be0f00545d14a640b291107fc/hackage-server.cabal#L150, so "broken" packages can be uploaded to Hackage, which is permanent store.

[andreabedini]

I understand that these mistakes are bad; and that's why I am thinking how to prevent them.
It seems to be evident that "asking themselves" isn't working. It must be the third time (off memory) that you notice some mess-up with cabal-version. I think this is worth some preventive measures.

And we need to fix this of course.

[fgaz]

In #9329 (comment) @phadej suggests that the criterion could be whether we touch Cabal-syntax

[phadej]

@fgaz

@phadej suggests that the criterion could be whether we touch Cabal-syntax

Except this change wasn't in Cabal-syntax, as cabal_macros.h etc generation is in Cabal (which I think is not great, as macros or extra module generation should be versioned as well).

[andreabedini]

FWIW this change https://github.com/haskell/cabal/pull/8534/files#diff-2979e1782cdde6333d50cf4ca6922117186875cafbd01007575535482b33a344 should have given away the problem ...

[Mikolaj]

Embarrassing. How do we fix it? Move the feature behind cabal-version retroactively and survey Hackage to see that no package gets broken by such a restriction? And if any gets broken, negotiate with the author to unbreak it?

[geekosaur]

Sounds sensible to me.

[andreabedini]

👍 it can be done quickly in a patch release right?

Edit: Ah, lol, no 😭

To fix this we need to add CabalSpecV3_10 to CabalSpecVersion which is a breaking change to Cabal-syntax.

[andreabedini]

I had a go at fixing this, see the PR above.

[fgaz]

As I wrote in #9374 (comment), I propose the following:

• Completely disable the PackageInfo feature in a patch release
• Deprecate the other 3.10 releases
• On the next release, enable PackageInfo conditionally on cabal-version >= 3.11

Since we can't introduce cabal-version 3.10 this seems like the only viable option other than completely deprecating the entire 3.10 series

[phadej]

As I wrote in #9374 (comment), I propose the following:

• Completely disable the PackageInfo feature in a patch release
• Deprecate the other 3.10 releases
• On the next release, enable PackageInfo conditionally on cabal-version >= 3.11

Since we can't introduce cabal-version 3.10 this seems like the only viable option other than completely deprecating the entire 3.10 series

This is what I would do.. The sooner the 3.10 release with "reverted" change is on Hackage, the sooner hackage-server can start use new Cabal-syntax the better, and we can avoid bad packages.

[gbaz]

Note that even with the reversion, I don't think any package using this will be rejected anyway? cabal check passes fine -- its only when the build phase kicks in that the package fails.

Up until check knows about this and knows how to reject it, there's nothing preventing this unless we add some extra ad-hoc code to hackage's check.

[phadej]

there's nothing preventing this unless we add some extra ad-hoc code to hackage's check.

That's sadly true. You can have

autogen-modules: Whatever
exposed-modules: Whatever

and cabal check won't complain. That is unfortunate and somewhat separate issue (that we are not explicit that it's Cabal task to generate say PackageInfo_pkgname, and not custom-setup or ./configure)

---

The cabal-version check whoever would make PackageInfo_<pkgname> behave the same with past and future Cabal versions (unfortunately not with few present one). I.e. there won't be cases "it works on my machine, but not on yours" (cause of different cabal-install versions).

[Mikolaj]

So what's the conclusion? It's broken, but it was as badly broken even before the offending PR? So we should fix it (by guarding stuff with cabal-version and improving cabal check), but there's no reason to rush cabal-install 3.10.3.0 and we can just as well do this in 3.12? Or can be prevent some future headache, when cabal check is more strict, by reverting the PR in 3.10.3.0 ASAP?

[gbaz]

I don't think reversion helps much. A PR to add an ad-hoc check directly to hackage server for this seems like the best approach.

[phadej]

I don't think reversion helps much.

Ok, you are saying that no features are necessary to guard behind cabal-version?

That is dumb.

[gbaz]

That is dumb.

That would perhaps be dumb! Good think I'm not saying that, and I'd urge you to be less rude and more charitable in your interpretations of people's comments.

I'm saying we can add a cabal check to the next version of cabal, and in the meantime we can patch hackage server to have an ad-hoc check doing the same thing in the deployed version of hackage server. I don't think reverting the existing feature will actually solve the problem we want -- which is to prevent the hackage index from getting packages making use of these files but lacking the correct cabal version constraints.

[phadej]

I'm saying we can ...

Please be very explicit. Your first message gave me impression that nothing needs to be done in Cabal.

Especially as an answer to @Mikolaj question

So we should fix it (by guarding stuff with cabal-version and improving cabal check), but there's no reason to rush cabal-install 3.10.3.0 and we can just as well do this in 3.12? Or ...

---

I'm unsure what we'll happen with Cabal, but indeed it's not my business.

I'm sorry I wasted your time by opening this issue.

[gbaz]

Its a worthwhile issue, and I'm glad you brought it up. Everyone is welcome to open tickets and it is much appreciated. I just wish you would not be so rude in your interactions -- it makes it harder to take full advantage of your knowledge and thoughtfulness.

[phadej]

I don't see anything rude in my message.

it makes it harder to take full advantage of your knowledge and thoughtfulness.

Well, over past few years I have felt that my "knowledge" is not appreciated essentially every time I tried to contribute to Cabal in any way, and I have been sometimes very frustrated with responses. I really don't know why I bother anymore.

[gbaz]

I think for now we don't need to worry about making the check reusable -- we can eventually use the cabal check in the future on hackage.

I agree we should patch cabal 3.12 as described. I don't think a backport to 3.10 is possible because of the cabal-version issue.

[Mikolaj]

I agree we should patch cabal 3.12 as described. I don't think a backport to 3.10 is possible because of the cabal-version issue.

Is it impossible for both parts, guarding stuff and cabal check extension?

I agree that the "guarding stuff" is impossible in 3.10, because we don't want to suddenly define a new cabal-version in 3.10.3.0. Does it mean reverting the feature altogether in 3.10.3.0 is the only option? I've heard arguments it'd be useful later on when 3.12 is properly patched and so 3.10 would behave differently than 3.12 in this respect. Do you disagree it's worth reverting, @gbaz? Did I miss any other options?

[gbaz]

I just don't see what purpose reverting serves. We already have a cabal shipped with ghc that allows it. The most we can do is guard against it hitting hackage.

[Mikolaj]

Thank you. Let me draft a plan. No dependencies between the steps.

1. Patch the hackage server to reject autogen-modules, etc., without cabal-version 3.12 . [token holder: @gbaz]
2. In 3.12 extend cabal check to error out if cabal-version is below 3.12 and autogen-modules, etc. is used. If there's 3.10.3.0 in the right timeframe, backport to 3.10.3.0, which would essentially block the new feature in 3.10.3.0, but with a sensible message and path forward (and the feature can be used locally), ensuring all major cabal-install versions behave the same wrt publishing autogen-modules, etc. [token holder: @ffaf1]
3. Guard autogen-modules, etc. in cabal 3.12 behind cabal-version 3.12. Do not backport. [edit: this PR by @andreabedini is close: Guard PackageInfo behind cabal-version: 3.10 #9374]
4. [edit: tentatively rejected] [Under discussion, needs good arguments and a volunteer] Revert autogen-modules, etc. in 3.10.3.0 and release 3.10.3.0 no matter what, which also makes the backport of step 2 spurious.
5. [edit: accepted, because we are doing 3.10.3.0 anyway] [Under discussion, needs good arguments and a volunteer] Alternatively to step 4., release 3.10.3.0 no matter what, with the backport of step 2.

Comments?

[ffaf1]

re: cabal check, I see work is underway in #9374, although I haven't examined the logic.

[Mikolaj]

Actually, I think #9374 is step 3., though perhaps there's a tiny overlap with 2.

[gbaz]

just wanted to point out that the above linked hackage pr means that hackage server now has a very basic ad hoc check on PackageInfo functionality, so packages won't make their way to hackage that use it.

[Mikolaj]

Dear all, please kindly review #9481 and #9525 that fix this, respectively, in cabal 3.10 and 3.12. Together with the Hackage tweak, this closes the issue, I think (see the plan above).

[Mikolaj]

I think this is now done. Thank you all and especially @ffaf1, who redesigned and implemented steps 2 and 3 of the plan. Please re-open if anything crops up.