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

Ignore invalid Unicode in pkg-config descriptions (backport #9609) #9658

Merged
merged 1 commit into from
Jan 25, 2024

Conversation

mergify[bot]
Copy link
Contributor

@mergify mergify bot commented Jan 25, 2024

This is an automatic backport of pull request #9609 done by Mergify.
Cherry-pick of 0b34b4e has failed:

On branch mergify/bp/3.10/pr-9609
Your branch is up to date with 'origin/3.10'.

You are currently cherry-picking commit 0b34b4eaa.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   cabal-install-solver/cabal-install-solver.cabal
	modified:   cabal-install-solver/src/Distribution/Solver/Types/PkgConfigDb.hs
	modified:   cabal-testsuite/PackageTests/ExtraProgPath/pkg-config
	new file:   cabal-testsuite/PackageTests/PkgConfigParse/MyLibrary.hs
	new file:   cabal-testsuite/PackageTests/PkgConfigParse/cabal.project
	new file:   cabal-testsuite/PackageTests/PkgConfigParse/my.cabal
	new file:   cabal-testsuite/PackageTests/PkgConfigParse/pkg-config
	new file:   cabal-testsuite/PackageTests/PkgConfigParse/setup.out
	new file:   cabal-testsuite/PackageTests/PkgConfigParse/setup.test.hs
	new file:   changelog.d/pr-9609

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   Cabal/src/Distribution/Simple/Program/Run.hs

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/github/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally


Mergify commands and options

More conditions and actions can be found in the documentation.

You can also trigger Mergify actions by commenting on this pull request:

  • @Mergifyio refresh will re-evaluate the rules
  • @Mergifyio rebase will rebase this PR on its base branch
  • @Mergifyio update will merge the base branch into this PR
  • @Mergifyio backport <destination> will backport this PR on <destination> branch

Additionally, on Mergify dashboard you can:

  • look at your merge queues
  • generate the Mergify configuration with the config editor.

Finally, you can contact us on https://mergify.com

@tomsmeding
Copy link
Collaborator

The diff in the original PR for the conflicted region of Cabal/src/Distribution/Simple/Program/Run.hs is only one added line: https://github.com/haskell/cabal/pull/9609/files#diff-aba8dac0c04a6819b76baa63673020aec53588c6f28245560d1d7ac4f4d084d8R27

So this should be easy to fix for someone with permissions to push to this branch.

Copy link
Member

@Mikolaj Mikolaj left a comment

Choose a reason for hiding this comment

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

With these changes perhaps it'd compile.

Cabal/src/Distribution/Simple/Program/Run.hs Outdated Show resolved Hide resolved
Cabal/src/Distribution/Simple/Program/Run.hs Outdated Show resolved Hide resolved
Comment on lines 30 to 27
IOEncoding (..)
, emptyProgramInvocation
, simpleProgramInvocation
, programInvocation
, multiStageProgramInvocation
, runProgramInvocation
, getProgramInvocationOutput
, getProgramInvocationLBS
, getProgramInvocationLBSAndErrors
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
IOEncoding (..)
, emptyProgramInvocation
, simpleProgramInvocation
, programInvocation
, multiStageProgramInvocation
, runProgramInvocation
, getProgramInvocationOutput
, getProgramInvocationLBS
, getProgramInvocationLBSAndErrors

Copy link
Member

Choose a reason for hiding this comment

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

Doh, and I finally removed the only bit that was important: getProgramInvocationLBSAndErrors. How do I revert suggestions?

Copy link
Member

Choose a reason for hiding this comment

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

I can't find a way, so I will have to check out the branch, fix and push, after all.

* Ignore invalid Unicode in pkg-config descriptions

Previously, if any of the pkg-config packages on the system had invalid
Unicode in their description fields (like the Intel vpl package has at
the time of writing, 2024-01-11, see #9608), cabal would crash because
it tried to interpret the entire `pkg-config --list-all` output as
Unicode.

This change, as suggested by gbaz in
  #9608 (comment)
switches to using a lazy ByteString for reading in the output, splitting
on the first space in byte land, and then parsing only the package
_name_ to a String.

For further future-proofing, package names that don't parse as valid
Unicode don't crash Cabal, but are instead ignored.

* Add changelog entry

* cabal-install-solver: Add bounds on 'text'

* No literal ASCII values, use 'ord'

* Address review comments re invalid unicode from pkg-config

* Add test for invalid unicode from pkg-config

* Compatibility with text-1.2.5.0

* Align imports

* Handle different exception type

* Use only POSIX shell syntax

* Add invalid-input handler in pkg-config shim

This is to appease shellcheck

* Actually implement all required stuff in the pkg-config shim

* Less exception dance

* Fix shebang lines

MacOS doesn't have /usr/bin/sh, and /bin/sh is the standard (for a POSIX
shell) anyway

* Don't expect a particular representation of invalid characters

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit 0b34b4e)
@Mikolaj Mikolaj force-pushed the mergify/bp/3.10/pr-9609 branch from 66568d5 to d4dff63 Compare January 25, 2024 10:00
@Mikolaj Mikolaj added the merge me Tell Mergify Bot to merge label Jan 25, 2024
@mergify mergify bot merged commit 9f9b5bd into 3.10 Jan 25, 2024
41 checks passed
@mergify mergify bot deleted the mergify/bp/3.10/pr-9609 branch January 25, 2024 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport conflicts merge me Tell Mergify Bot to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants