Skip to content

Conversation

@Bodigrim
Copy link
Collaborator

This is to comply with haskell/core-libraries-committee#292

We could have achieved the immediate goal by replacing combine in Distribution.Compat.Internal.TempFile with something like

    combine a [] = a
    combine a b = case unsnoc a of
        Nothing -> b
        Just (_, lastA)
            | pathSeparator [lastA] -> a ++ b
            | otherwise -> a ++ [pathSeparatorChar] ++ b

but honestly there is no reason to retain openNewBinaryFile implementation at all. Let's just outsource all the work to openBinaryTempFileWithDefaultPermissions from base.

Template B: This PR does not modify behaviour or interface

Include the following checklist in your PR:

  • Patches conform to the coding conventions.
  • Is this a PR that fixes CI? If so, it will need to be backported to older cabal release branches (ask maintainers for directions).

@Bodigrim Bodigrim force-pushed the avoid-init-and-last branch from 1382a57 to e505950 Compare June 14, 2025 22:44
Copy link
Collaborator

@geekosaur geekosaur left a comment

Choose a reason for hiding this comment

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

Modulo the hlint warning, which derives from an import that is no longer conditional. —ETA: and which you just resolved while I was reviewing, lol.

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.

Thanks!

@Bodigrim Bodigrim added the merge me Tell Mergify Bot to merge label Jun 15, 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 15, 2025
This is to comply with haskell/core-libraries-committee#292

We could have achieved the immediate goal by replacing combine
in Distribution.Compat.Internal.TempFile with something like

    combine a [] = a
    combine a b = case unsnoc a of
        Nothing -> b
        Just (_, lastA)
            | pathSeparator [lastA] -> a ++ b
            | otherwise -> a ++ [pathSeparatorChar] ++ b

but honestly there is no reason to retain openNewBinaryFile implementation at all.
Let's just outsource all the work to openBinaryTempFileWithDefaultPermissions from base.
@Mikolaj Mikolaj force-pushed the avoid-init-and-last branch from e505950 to 96a72d2 Compare June 17, 2025 11:05
@mergify
Copy link
Contributor

mergify bot commented Jun 17, 2025

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

The merge conditions cannot be satisfied due to failing checks:

You may have to fix your CI before adding the pull request to the queue again.
If you update this pull request, to fix the CI, it will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a @mergifyio requeue comment.

mergify bot added a commit that referenced this pull request Jun 17, 2025
@mergify mergify bot merged commit b2fa5ec into master Jun 17, 2025
55 of 63 checks passed
@mergify mergify bot deleted the avoid-init-and-last branch June 17, 2025 15:15
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.

4 participants