Fresh ~/.cabal/config written by cabal 3.10 crashes any older cabal

[Mikolaj]

This was originally reported in haskell/actions#202, partially worked around there, but affects any users and any mixed-GHC CI (either testing many GHCs with the same CI cache or picking a random cabal version for a given GHC).

Judging by the dates of the PRs involved, this seems to affect already cabal 3.8.1.0 (please correct if not).

This was probably caused by the helpful and seemingly benign #8054, later amended in #8522, that make Nix flags easier to read. Note that old cabal happily ignore ~/.cabal/config options they don't understand, but choke on old options they recognize, but can't parse in a new way. That seems to be the root cause of the breakage.

I think, without a very good reason, we should not let cabal break the config file for old cabals, so let's think how to fix this. Ideally, without reverting the PRs in question.

@cbclemmer: I'm a bit lost in the history of these changes. Do you perhaps remember why we ended up changing the config option at all? Do you have any suggestions how to eat the cake (improve the command-line options) and have it too (parse the config option using the old parsing code in old cabals)?

[ulysses4ever]

Do you perhaps remember why we ended up changing the config option at all?

The old definition of the nix flag, albeit straightforward, led to an (automatically composed) --help message that read very weird:

 --disable-nix                  Disable Nix integration: run commands through
                                nix-shell if a 'shell.nix' file exists

(--disable-nix does not run commands in a nix shell, despite the message claiming it.)

[gbaz]

My preference here as to a solution would be simply to have the value of this flag, if it is the default, written in generated config files commented out rather than enabled and explicitly set to the default.

[Mikolaj]

That's good enough IMHO too.

[Mikolaj]

@cbclemmer: given your knowledge of cabal usage in Nix, could you tell if writing the offending line in a fresh config commented out is likely to cause any problems for the Nix users? If you think no problems are likely, would you be up to implementing this?

[cbclemmer]

Hey @Mikolaj, thanks for pinging me again. I looked at this issue earlier and meant to get back to you about it, but it got lost in my notifications. I'll look into this and see if I can fix it.

[cbclemmer]

@Mikolaj So my problem with the #8522 PR was that I assumed what ~/.cabal/config would be and made a file based on that. I tested removing my ~/.cabal/config file and creating a cabal repo with cabal 3.6.2.0 when I checked the config file it had nix commented out (which it should be). Then, when I changed to 3.10.1.0, removed the config file again, and built a new repo, the nix line is not commented out. So I don't think what I changed necessarily caused this issue but if I had made the test for the PR fetch the default config from the createDefaultConfigFile function in Config.hs instead of hard coding it, whatever PR that caused this issue would have caught it in the integration tests. After playing around with the Config.hs file I can't find a difference between say the http-transport parameter (which is still commented out), and the nix parameter. Any ideas on how this could have changed recently? I don't mind fixing it still, but I'm kind of stumped now...

[Mikolaj]

Wow, that indeed is mysterious. So it seems another PR, at some point between 3.6.2 and 3.10.1, changed the nix option from commented out to normal. And we have no idea which PR it was and how to change it back (but only for nix). Did I get this right?

Indeed, there's barely any difference between http-transport and nix. Could legacySharedConfigFieldDescrs be the key?

[gbaz]

I found the issue -- its here, and was introduced with that PR:

https://github.com/haskell/cabal/blob/master/cabal-install/src/Distribution/Client/Setup.hs#L372

Basically we need to make the NoFlag return Just "" instead of text. This may also involve fixing the parser to accept that?

The reason for this is as follows. Eventually we call ppField defined here: https://github.com/haskell/cabal/blob/master/cabal-install/src/Distribution/Client/ParseUtils.hs#L194

That will detect if the docstring in the field prints as empty or not. If it does print as empty, it prints the field commented out. If it prints as nonempty (i.e. "disabled") then it will print the value.

So all other fields have the property that map (\x -> fieldGet x mempty) $ configFieldDescriptions ConstraintSourceUnknown produces an empty string -- i.e. the mempty Config produces empty field descriptions. This field uniquely does not, and the current result is

[,,disable,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,]

I would propose both fixing this, and also adding a regression test for this property so we don't violate it in the future.

[Mikolaj]

Right, this makes sense and my hint was probably a blind alley. @cbclemmer: let us know if this works for you.

[cbclemmer]

@gbaz Thanks for the help. Unfortunately returning Just "" makes the default config an empty string, but the pretty print function needs it to be pp.empty. I was able to fix this, however, by just returning an empty list. I've created a PR for this issue that should fix everything. Now it will generate the default config dynamically and ensure that the nix option is commented out.

PR #8878