Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Track the exact source of ghc-*-options related warnings #5598

Merged

Conversation

nkly
Copy link
Collaborator

@nkly nkly commented Oct 1, 2018

Previously, when running cabal check all the various ghc-*-options flags were merged together, thus losing the information about the exact place of the warning. This PR implements separate checking of ghc-*-options, which allows us to give users more precise warnings.

Fixes #5342

Checklist:

  • Patches conform to the coding conventions.
  • Any changes that could be relevant to users have been recorded in the changelog.
  • The documentation has been updated, if necessary.
  • If the change is docs-only, [ci skip] is used to avoid triggering the build bots.

As for the testing approach, it is the same used for all other tests in the Check package.

@nkly nkly force-pushed the cabal-check-track-options-source-field branch 2 times, most recently from c59b110 to 2b3f0cd Compare October 1, 2018 14:17
haskell-pushbot pushed a commit to haskell-pushbot/cabal-binaries that referenced this pull request Oct 1, 2018
"url":"pull/5598",
"account":"haskell",
"repo":"cabal",
"commit": "f055a5b0e51f6c68f449ad4b8247d95e93e6764f",
"tag":"linux-7.10.3"
}
haskell-pushbot pushed a commit to haskell-pushbot/cabal-binaries that referenced this pull request Oct 1, 2018
"url":"pull/5598",
"account":"haskell",
"repo":"cabal",
"commit": "f055a5b0e51f6c68f449ad4b8247d95e93e6764f",
"tag":"linux-8.0.2"
}
Copy link
Contributor

@quasicomputational quasicomputational left a comment

Choose a reason for hiding this comment

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

LGTM, one quibble aside and pending Travis's thoughts. Thanks a bunch!

@@ -2,6 +2,8 @@

2.6.0.0 (current development version)
* New solver flag: '--reject-unconstrained-dependencies'. (#2568)
* 'check' reports warnings for various ghc-\*-options fields separately
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is affecting Cabal-the-library, the changelog note should go in Cabal/changelog.

haskell-pushbot pushed a commit to haskell-pushbot/cabal-binaries that referenced this pull request Oct 1, 2018
"url":"pull/5598",
"account":"haskell",
"repo":"cabal",
"commit": "f055a5b0e51f6c68f449ad4b8247d95e93e6764f",
"tag":"linux-7.6.3"
}
Previously, when running `cabal check` all the various ghc-*-options
flags were merged together, thus losing the information about the exact
place of the warning. This PR implements separate checking of
ghc-*-options, which allows us to give users more precise warnings.

Fixes haskell#5342
@nkly nkly force-pushed the cabal-check-track-options-source-field branch from 2b3f0cd to 3c1502b Compare October 1, 2018 14:32
haskell-pushbot pushed a commit to haskell-pushbot/cabal-binaries that referenced this pull request Oct 1, 2018
"url":"pull/5598",
"account":"haskell",
"repo":"cabal",
"commit": "f055a5b0e51f6c68f449ad4b8247d95e93e6764f",
"tag":"osx-7.8.4"
}
haskell-pushbot pushed a commit to haskell-pushbot/cabal-binaries that referenced this pull request Oct 1, 2018
"url":"pull/5598",
"account":"haskell",
"repo":"cabal",
"commit": "3c1502bb633b61ae4d24d40898fca66d7f169679",
"tag":"linux-7.6.3"
}
haskell-pushbot pushed a commit to haskell-pushbot/cabal-binaries that referenced this pull request Oct 1, 2018
"url":"pull/5598",
"account":"haskell",
"repo":"cabal",
"commit": "3c1502bb633b61ae4d24d40898fca66d7f169679",
"tag":"linux-7.10.3"
}
haskell-pushbot pushed a commit to haskell-pushbot/cabal-binaries that referenced this pull request Oct 1, 2018
"url":"pull/5598",
"account":"haskell",
"repo":"cabal",
"commit": "3c1502bb633b61ae4d24d40898fca66d7f169679",
"tag":"linux-8.0.2"
}
haskell-pushbot pushed a commit to haskell-pushbot/cabal-binaries that referenced this pull request Oct 1, 2018
"url":"pull/5598",
"account":"haskell",
"repo":"cabal",
"commit": "3c1502bb633b61ae4d24d40898fca66d7f169679",
"tag":"linux-8.2.2"
}
haskell-pushbot pushed a commit to haskell-pushbot/cabal-binaries that referenced this pull request Oct 1, 2018
"url":"pull/5598",
"account":"haskell",
"repo":"cabal",
"commit": "3c1502bb633b61ae4d24d40898fca66d7f169679",
"tag":"linux-8.4.3"
}
haskell-pushbot pushed a commit to haskell-pushbot/cabal-binaries that referenced this pull request Oct 1, 2018
"url":"pull/5598",
"account":"haskell",
"repo":"cabal",
"commit": "3c1502bb633b61ae4d24d40898fca66d7f169679",
"tag":"osx-7.8.4"
}
haskell-pushbot pushed a commit to haskell-pushbot/cabal-binaries that referenced this pull request Oct 1, 2018
"url":"pull/5598",
"account":"haskell",
"repo":"cabal",
"commit": "3c1502bb633b61ae4d24d40898fca66d7f169679",
"tag":"linux-8.4.3-fdebug-expensive-assertions"
}
haskell-pushbot pushed a commit to haskell-pushbot/cabal-binaries that referenced this pull request Oct 1, 2018
"url":"pull/5598",
"account":"haskell",
"repo":"cabal",
"commit": "3c1502bb633b61ae4d24d40898fca66d7f169679",
"tag":"osx-8.0.2"
}
@nkly
Copy link
Collaborator Author

nkly commented Oct 2, 2018

Looks like one of the jobs failed by timeout, and another one got an error during dependency resolution. Both failures are unrelated to the changes in the PR. Is it okay to merge?

@quasicomputational
Copy link
Contributor

I'd say to go for it. The solver error is happening on master, so it's not due to your code, and we bump right up against time limits on the OSX builders so it's inevitable that some builds get unlucky and time out.

@23Skidoo
Copy link
Member

23Skidoo commented Nov 7, 2018

Merged, thanks!

@23Skidoo 23Skidoo merged commit e1ce4b0 into haskell:master Nov 7, 2018
@nkly nkly deleted the cabal-check-track-options-source-field branch November 8, 2018 12:40
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