Skip to content

Conversation

@sol
Copy link
Member

@sol sol commented Dec 8, 2025

There are two things that I lack the context and background to comprehend.

  1. For what reason would these pragmas be needed in the first place?
  2. What was the rational for changing these in f47768a? Was it simply an oversight, or are we at the point where "a subjective urge for tidiness" takes precedences over providing value to the user and backwards compatibility?
  • squash commits (I did this through GitHub UI from mobile..)

@sol
Copy link
Member Author

sol commented Dec 8, 2025

Just to be clear, if this was an honest mistake, then that's OK with me. Nobody's perfect and mistakes do happen. That's normal.

But if this was intentional, then I don't have much sympathy for it.

@Bodigrim
Copy link
Collaborator

  1. For what reason would these pragmas be needed in the first place?

If a package is compiled with, say, -Werror -Weverything, Paths_*.hs must forcefully disable some warnings so that this autogenerated file does not cause the entire build to fail.

It could probably bluntly set -Wwarn -Wno-everything, but what's exactly the issue with the current approach?

@sol
Copy link
Member Author

sol commented Dec 12, 2025

  1. For what reason would these pragmas be needed in the first place?

If a package is compiled with, say, -Werror -Weverything, Paths_*.hs must forcefully disable some warnings so that this autogenerated file does not cause the entire build to fail.

My naïve assumption was that the -w that's already there is enough to achieve this. But maybe I am mistaken.

what's exactly the issue with the current approach?

It needlessly breaks GHC 7.10.3.

https://github.com/hspec/setup-haskell/actions/runs/19809279822/job/56748489581#step:6:25

Now, if you would get anything in return for making things less compatible than that could be a worthwhile trade-off.

But changing

{-# OPTIONS_GHC -fno-warn-missing-import-lists #-}
{-# OPTIONS_GHC -w #-}

to

{-# OPTIONS_GHC -Wno-missing-import-lists #-}
{-# OPTIONS_GHC -w #-}

?

That's strictly less compatible, no? Is there any justification to do that?

@sol sol requested a review from philderbeast December 12, 2025 21:51
@Bodigrim
Copy link
Collaborator

It needlessly breaks GHC 7.10.3.

Cabal stopped testing compatibility with GHC 7.10 quite some time ago (modern Linux distributives can no longer run old GHC binaries and macOS on ARM even less so), so that's why no one noticed it. But sure, -fno-warn instead of -Wno should be fine. In the interest to get things done without a prolonged discussion, I'd suggest to update the PR to do just that.

@sol
Copy link
Member Author

sol commented Dec 12, 2025

In the interest to get things done without a prolonged discussion, I'd suggest to update the PR to do just that.

I'm not a big fan of cargo culting.

Is there a reason -w alone isn't sufficient?

@Bodigrim
Copy link
Collaborator

Dunno, I can see #7352 introducing -w with a stated goal to quash -Wno-prepositive-qualified-module (but still retaining -fno-warn-missing-import-lists!), and then #8336 a year later, which added -Wno-prepositive-qualified-module explicitly.

@sol
Copy link
Member Author

sol commented Dec 13, 2025

then #8336 a year later, which added -Wno-prepositive-qualified-module explicitly

Even adding a test that doesn't seem to fail without the corresponding change…

@Bodigrim
Copy link
Collaborator

Given that the tests still pass and that #7352 / #8336 does not mention any specific reason why -w would be insufficient to be used instead of any other -Wno / -fno-warn, I suppose this is good to go.

Copy link
Collaborator

@ulysses4ever ulysses4ever left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like a good cleanup of a somewhat messy history of drive-by edits... Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants