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

Print build profile in new-build command #4605

Merged
merged 8 commits into from Jul 17, 2017

Conversation

amir
Copy link
Collaborator

@amir amir commented Jul 15, 2017

Please include the following checklist in your PR:

  • 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.

Prints build profile—at the moment only compiler and optimization level. Sample output:

Build profile: with-compiler: ghc-7.10.3, optimization: MaximumOptimisation

With this cabal.project.local:

optimization: 2

This is a naive attempt to address #3945. I'm not only new to Cabal but also to Haskell. I'd gladly follow any hints by a mentor to get this merged but feel free to discard it if there's absolutely no hope.

@mention-bot
Copy link

@amir, thanks for your PR! By analyzing the history of the files in this pull request, we identified @dcoutts and @ezyang to be potential reviewers.

Copy link
Member

@23Skidoo 23Skidoo left a comment

Choose a reason for hiding this comment

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

Looks OK modulo minor comments.

@@ -638,7 +644,7 @@ printPlan verbosity

| otherwise
= noticeNoWrap verbosity $ unlines $
("In order, the following " ++ wouldWill ++ " be built" ++
(showBuildProfile ++ "In order, the following " ++ wouldWill ++ "!" ++ " be built" ++
Copy link
Member

Choose a reason for hiding this comment

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

Why the exclamation sign here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sorry, a debugging artifact which is removed now.

@@ -756,6 +762,9 @@ printPlan verbosity
showMonitorChangedReason MonitorFirstRun = "first run"
showMonitorChangedReason MonitorCorruptCache = "cannot read state cache"

showBuildProfile = "Build profile:\n" ++ (unlines $ map (" " ++) [
Copy link
Member

Choose a reason for hiding this comment

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

Can you please just inline the indentation instead of using map?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done.

@23Skidoo
Copy link
Member

Travis failure looks genuine, you need to update expected output files in the test suite.

@@ -756,6 +762,9 @@ printPlan verbosity
showMonitorChangedReason MonitorFirstRun = "first run"
showMonitorChangedReason MonitorCorruptCache = "cannot read state cache"

showBuildProfile = "Build profile:\n" ++ (unlines [
Copy link
Member

@hvr hvr Jul 16, 2017

Choose a reason for hiding this comment

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

Please try to squeeze this into a single line as I suggested in #3945 (comment); this is mostly redundant/noisy information to see on every invocation, so it better be terse/compact. Note that there's also the proposed cabal show-build-info command whose purpose is to dump a much more verbose version of the build-profile. So the one we emit here can really just be a small glimpse at it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done. comma separated now instead of line separated.

@23Skidoo 23Skidoo merged commit dec3b28 into haskell:master Jul 17, 2017
@23Skidoo
Copy link
Member

Merged, thanks!

@23Skidoo
Copy link
Member

23Skidoo commented Jul 18, 2017

@amir In the interest of brevity and clarity, would it be possible to render optimisation: NormalOptimisation as optimisation: -O1 (and so forth for -O0/-O2/...)?

@hvr
Copy link
Member

hvr commented Jul 18, 2017

@23Skidoo I'd go one step further, why not use the CLI-valid concise representation?

e.g.

Build profile: -w ghc-7.10.3 -O2

that way you can directly paste it on the commandline.

@amir
Copy link
Collaborator Author

amir commented Jul 18, 2017

Sure, I'll prepare a new PR.

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.

None yet

4 participants