cabal-install-3.10 fails with header files in 'c-sources' #9190
Comments
|
I guess it's a change in behavior, but if the underlying cause is really that .h files were listed in c-sources, then if argue, cabal is actually doing the right thing now and was doing the wrong thing before. Putting .h into c-sources seems more like a user error to me. |
|
Ideally this should be a warning in cabal though, that for now it filtered out .h files from c-sources (did it really?) and that this behavior will cease in a future version; authors are strongly encouraged not to list headers in c-sources; with a reference to a cabal issue to where affected parties can look up the details. |
|
Yes, it can be considered a bug. But the problem is that cabal will stop accepting cabal files that it previously accepted with no transition period whatsoever. It is a breaking change. |
I'm afraid you're conflating languages like Haskell they have no concept of separate "code" and "header" files, and C where both .c and .h are very natural - and often (though not always, I concede) reside in the same directory. The new behavior is an unwelcome undesirable unproductive breaking change. |
|
Also, for what it's worth, as a user I personally think "C sources" is a reasonable catch-all term to include header files, so I find it confusing that headers would not be valid to include in a "C sources" list just from a terminology standpoint. If I have files |
|
It looks like cabal-install-3.8 has the same behaviour of 3.6 so this is indeed a change in 3.10. It's also present in cabal-head. |
|
I still consider this a bug. One that should not have been a breaking change, but come with a proper deprecation cycle (again, including a ticket where this change of behaviour could be debated, and maybe even lead to the acceptance of the old behaviour). So why do I think this is a bug? C has source files ( The cabal documentation, also only lists I'll remain that the old behaviour is a bug. The breaking change is a defect, as it abruptly breaks the behaviour, and should be rectified to a warning. Ultimately I consider listing header files under |
For you, maybe - but not for the majority of the users. So, I insist that it isn't. You say that .c and .h must be treated differently. I say that view is wrong, at least in this context where Cabal is supposed to treat them all as a "blob" to pass along for processing. Not to mention that .h files sometimes do contain actual code rather than just definitions, and they can be compiled - though majority do not split their code that way. Everybody, please consider; what's the purpose and the (main) use case for |
Correct. The standard is somewhat vague on this but also indicates that headers can sometimes be source files. But not always. So |
|
My uneducated guess would be that c-sources are c source files to compile before linking them to the haskell code. From what I managed to understand today, cabal does indeed call ghc to compile each file in c-sources. |
I used to think that cabal invokes C compiler directly for files in c-sources? |
|
These seem to be the relevant lines cabal/Cabal/src/Distribution/Simple/GHC.hs Line 847 in 41225bb I cannot do that now but this can be easily checked with |
|
Ok, how do we proceed? Is this just a matter of emitting a warning if a header file is found and not forwarding it to the underlying C compilation process? |
|
@jtdaugherty I'm trying to compile vty-unix with GHC 9.2.8 and cabal 3.10.1.0 on commit ad31125f but I'm getting an error: Is this normal? Is there a commit that compiles that I can try the patch on? |
|
I'm confused. There's so many aspects to that. Does cabal fail currently, because it tries to feed a .h file to the compiler and the compiler rejects it or because cabal sees a .h files and aborts? Is the intention of the user that each listed file should be fed to the compiler, e.g., even a .h file, because it contains actual code? Is that the most reasonable semantics of the field? Does What was cabal doing previously? Just filtering out the .h files? What about .cxx files? |
|
I think the first step is to figure out where exactly this broke. There must be a commit to point at, and say this commit changed the previous behavior. And then go from there. |
|
This is a release blocker for 3.10.2.0, and unless someone's figuring out the origin in the next couple of days, we'll have to go with the patch at #9200. |
|
The only recent change to that logic seems to be #9061. I am going to read that again. |
|
@Kleidukos AFAICT vty-unix master depends on a pre-release version of vty. I can reproduce with similar instructions to what Julian has posted EDIT NOTE: |
|
@andreabedini Thanks, I ended up stripping the cabal file from most of its content to get faster build times :P |
|
The plot thickens. With a minimal example, I cannot reproduce it: only |
|
ok let me push a reproducible repo: https://github.com/Kleidukos/cabal-9190-repro |
|
The first bad commit seems to be 6cf9e13 from #8499. Ping @gbaz |
|
Thanks @andreabedini for the sleuthing. In my mind I think this confirms that there's no bug. The files were never supposed to be there, and through a combination of accidents things sometimes worked. Prior to that PR, files were nubbed by name (not including extension) alphabetically. Subsequent to that PR, files are nubbed by name (not including extension) in an order preserving manner. So when you have two files with the same name -- say a c and a header file, before the c always came alphabetically first. Now, it depends on the ordering. But the nubbing is not a "feature" that filters .h files. Its just an accidental artifact that caused .h files to be dropped only when they had a precisely corresponding c file. So if you had a header file with no c file "masking" it, this would always have been a problem. Putting h files in the c sources field was never supported, never in spec, and only sometimes, accidentally worked. The only "real" fix in my mind would be to make cabal nub by filename and extension both -- at which point we would uniformly instead of occasionally fail. (and just to underline: the documentation never said putting header files in c sources was acceptable or correct -- the packages doing this always went against the documentation) |
Co-authored-by: Andrea Bedini <[email protected]> fixes #9190
|
Incredible set of circumstances, thank you @gbaz for the explanation |
|
Thanks @gbaz
I am curious about this, why would we not include the extension? |
|
Oh you're right, its more complicated in fact! The nubbing wasn't at issue, since its by filepath, which does take into account extension. Its really just the reordering, not the nubbing, that induced this issue. So it must be something with the order of the files passed to the c compiler mattering, but now I'm not sure how... |
|
Leaving some notes here for the future. My minimal reproducible example is: Last night I ran git bisect recompiling cabal-install everytime and it took ages. I realise now I could have been smarter: this is purely a Cabal problem, so we can find a way to use only Cabal. Use this script (e.g. and call it like: This only recompiles If you want to pin down ghc and cabal-install versions you can use |
|
[Edit: nothing to see here, please move along.] |
|
why is this issue still open? i thought it was fixed by #9200 ... |
|
Oh, hah, and I was the only person to review the PR. Let me remove the incriminating comment, in that case, and close this issue. :) The PR is mentioned in this issue, just a bit far from here, so I hope everybody interested had a look. If not, please feel free to comment now and open a new issue, if needed (or re-open, or ask me to re-open). |
|
@ulysses4ever just noticed that the PRs quick-fixing this issue are not on master. So perhaps I closed this issue prematurely, after all. However, nobody objected nor asked me to re-open since 3 weeks ago, so perhaps the quick-fix is an acceptable solution, after all? If so, let's forwardport from branch 3.10 to branch master the #9200 fix and it's companion #9285 and be done with it. Thoughts? Objections? |
|
I'm trying to build Am I right that this is the same issue? |
|
@vaibhavsagar yes indeed, would you mind trying with 3.10.2.1 (which has just been published)? |
|
We hit the same issue building And regression when building |
This looks suspiciously similar to #9334 |
|
@hasufell no, it's because the linker can't deal with non-C sources that are in the c-sources stanza |
|
Is there supposed to be an option for Obj-C sources (.m)? https://github.com/composewell/streamly/blob/master/streamly.cabal#L459 |
|
Hi -- It is currently broken with the nightly stackage snapshots. Please either allow |
|
I believe this should be fixed by #9285 which changed the filtering logic from only keeping .c files to only excluding .h files. This change made it into 3.10.2.1 : https://github.com/haskell/cabal/blob/master/release-notes/Cabal-3.10.2.1.md |
3.10.2.1 still thinks .m files are headers. That's the version I've been trying out in Homebrew/homebrew-core#154836 and ended up having to do a dance of using 3.10.2.1 to install 3.10.1.0 in order to build |
|
The linked PR should clearly fix the behavior you describe, so this confuses me. Note that it is a PR to Cabal the library, not cabal-install the executable. Is it possible you are using a version 3.10.2.1 of cabal-install, but linked against a prior version of the Cabal library? |
Retrying locally, it may due to bundled Cabal 3.10.2.0 in GHC 9.8. Increasing verbosity, I see: What would be the best way to avoid this? A manual |
You could just bump the Cabal lowerbound in cabal-install.cabal, or use --constraint sure. |
Uh oh!
There was an error while loading. Please reload this page.
Reproduce:
git clone -b refactor/vty-crossplatform https://github.com/hasufell/brick.gitcd brickghcup run --cabal 3.10 -- cabal buildghcup run --cabal 3.6 -- cabal buildOriginally spotted here: jtdaugherty/vty#260 (comment)
Fixed here by changing the cabal file: jtdaugherty/vty-unix#1
This is a regression.
The text was updated successfully, but these errors were encountered: