Skip to content

cabal-install: dynExe+profExe requires prof+dyn #10994

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sheaf
Copy link
Collaborator

@sheaf sheaf commented Jun 17, 2025

This commit updates the logic in cabal-install's elaborateInstallPlan function to ensure that we don't try to build a profiled dynamic executable if the compiler doesn't support the profiling dynamic way.

This brings the logic in cabal-install in sync with the Cabal configureProfiling function, which sets withDynExe to false if the user wants a profiled executable but prof+dyn is not supported by the compiler.


Template B: This PR does not modify behaviour or interface

E.g. the PR only touches documentation or tests, does refactorings, etc.

Include the following checklist in your PR:

@sheaf
Copy link
Collaborator Author

sheaf commented Jun 18, 2025

I haven't managed to write a test for this, because the way things currently work, we compute e.g. whether to build a prof+dyn executable in cabal-install, but then when we invoke ./Setup configure we re-compute this information. I tried writing a test that was based off printPlan but that has the same problem, as that calls the computeEffectiveProfiling function from the Cabal library, thus re-computing that field as well.

Thus, the underlying issue only manifests if you directly call the configure function in Cabal from within cabal-install, like I'm doing in #9871. In that case, the problem is detected by the existing ProfShared test (which fails without this change).

I'm not including a changelog entry either as this should be invisible to users.

@sheaf sheaf marked this pull request as ready for review June 18, 2025 10:47
@sheaf sheaf force-pushed the wip/profDyn branch 2 times, most recently from 6a938d6 to e238448 Compare June 18, 2025 10:50
This commit updates the logic in cabal-install's 'elaborateInstallPlan'
function to ensure that we don't try to build a profiled dynamic
executable if the compiler doesn't support the profiling dynamic way.

This brings the logic in cabal-install in sync with the Cabal
'configureProfiling' function, which sets 'withDynExe' to false if the
user wants a profiled executable but prof+dyn is not supported by the
compiler.
@sheaf sheaf added the merge me Tell Mergify Bot to merge label Jun 19, 2025
@mergify mergify bot added ready and waiting Mergify is waiting out the cooldown period merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days labels Jun 19, 2025
Copy link
Contributor

mergify bot commented Jun 21, 2025

This pull request has been removed from the queue for the following reason: pull request branch update failed.

The pull request can't be updated.

You should update or rebase your pull request manually. If you do, this pull request will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue, you can requeue the pull request, without updating it, by posting a @mergifyio requeue comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days merge me Tell Mergify Bot to merge ready and waiting Mergify is waiting out the cooldown period
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants