Only use cpphs for .cpphs files

[vmchale]

This addresses https://github.com/haskell/cabal/issues/4278. I think mboes is correct and it should simply fail to process .cpphs files if cpphs is not available, especially since new-build and build-tool-depends: cpphs:cpphs

• [x] Patches conform to the coding conventions.
• [x] Any changes that could be relevant to users have been recorded in the changelog.
• [ ] The documentation has been updated, if necessary.

Tested by building the exe and running it locally.

[Mikolaj]

Would there be a way to cheaply test this in our test suite? Is it blocking you and would a backport to 3.8 potentially help?

[vmchale]

@Mikolaj I'd say a backpack to 3.8 would be useful, yes! GHC 9.4.1 has a bug in its preprocessor.

[Mikolaj]

@Mikolaj I'd say a backpack to 3.8 would be useful, yes! GHC 9.4.1 has a bug in its preprocessor.

3.8 is the Cabal version shipped with 9.4.1. Backport to 3.6 is not likely. An option is include this PR only in 3.10 that is not yet even planned, but I gather we'd want to avoid that.

[Mikolaj]

Let me rebase to include a workaround to our CI instability and fix the tests.

[Mikolaj]

@mergify rebase

[mergify[bot]]

rebase

✅ Branch has been successfully rebased

[ulysses4ever]

Wow: it just said that it was me who pushed the rebased committs?..

[Mikolaj]

BTW, we are ready to assist with modifying an existing test or writing a new one.

[ulysses4ever]

Has anyone thought about what would a test amount to? For one (it seems to me), it'd require cpphs installed in the test environment. Does anyone know of a way to get a Hackage-hosted tool installed in the test environment for one test only? From what I know, I think it's only possible to install stuff globally for all tests. Do we want that?

[Mikolaj]

I don't know. Perhaps the new feature for specifying an exe build dependency would be enough (forgot how it's called). I mean, perhaps we could add cpphs as such dependency to the .cabal file of the single test in question.?

[ulysses4ever]

@Mikolaj my impression (correct me if I'm wrong) was that those cabal files don't have access to Hackage; see FAQ for the testsuite, Q#3.

[Mikolaj]

Right, I now remember a discussion when somebody wanted the access to check messages from cabal update. How big is cpphs? Would it be cached?

[ulysses4ever]

It's very small, both in terms of dependency footprint and in terms of code size. On my laptop it took several seconds to build. And yes, it'd probably be in the cache all the time.

It's not the build time that I'm concerned about, but just expanding the CI script, and the precedent of doing that for a single test. But maybe it's not worth worrying.

[Mikolaj]

Let's expand this one last time. :)

[AntC2]

(Apologies if I'm talking out of turn here -- just ignore me. By some cosmic coincidence -- spotted by @vmchale -- a few days ago I found cpphs answered a specific need, and for which off-the-shelf cpp wasn't helping.)

Has anyone thought about what would a test amount to?

cpphs is more friendly for Haskell code than cpp -- that is, if you tell it the right way to do things. I wrote up some notes for the cpphs options that might be helpful for a tester.

• A good test would be to expand a multi-line macro into a multi-line Haskell decl with --layout, making sure it indents properly.
• Another good test would be expansions with -XMagicHash; and possibly defining an operator (##). Hashes have special significance within cpp, which make them difficult to use in your Haskell. If you want a test to break something, try those same tests with --hashes set.
• By default, cpphs produces a header line in the output starting #line .... I guess GHC won't know what to do with that(?) Option --noline suppresses it.
• As my note says, cpphs's style for calling macros is myMacro(someArg) -- needs the parens round the arg, not Haskell style myMacro someArg -- whitespace between them. What's bad is that if you use Haskell style, cpphs doesn't shout at you, it'll pass on myMacro someArg into the generated source, where presumably myMacro is not defined. I would include a expected fail test for this, to make sure GHC handles it gracefully.

[Mikolaj]

@AntC2: thank you for the helpful ideas.

@vmchale: could we assist you in any way?

[Kleidukos]

Marking this PR as draft :slightly_smiling_face: