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

Fix #1810: Unicode-normalized pkg name comparison #2397

Conversation

Blaisorblade
Copy link
Collaborator

(Untested yet).
Potential quick fix to the most urgent part #1810, based on @harendra-kumar's pure Haskell normalization library. A more proper and systematic fix, replacing text with a normalizing version, remains for future work.

Includes one line from normalization-insensitive, pending https://github.com/ppelleti/normalization-insensitive#2—maybe I should add @ppelleti's copyright or is this line too trivial?

@Blaisorblade
Copy link
Collaborator Author

FWIW, the affected integration test appears to still work for me on my OS X machine with this PR. Here's the complete transcript (sorry for the noise), grep for ば日本-4本 for the relevant success.

$ stack exec -- stack ghci stack:stack-integration-test --flag stack:integration-tests --no-build
Populated index cache.
Did not find .cabal file for wreq-0.4.1.0 with Git SHA of 76d8ca4c0629b0e0bec9e23d699f16799f4bc1d2
Did not find .cabal file for servant-client-0.7.1 with Git SHA of fae87d88a9890b1ad3d51c458c76a9de0e0e4d4f
Did not find .cabal file for pipes-safe-2.2.4 with Git SHA of 473334c25715ece2b16ae5fb9a6af2d887961238
Did not find .cabal file for network-transport-0.4.4.0 with Git SHA of 85fb7d392620f289c379c453cac6a50e647a0caf
Did not find .cabal file for monad-http-0.1.0.0 with Git SHA of c59775f9430a8d671169fbe3dd156ee4e2df9918
Did not find .cabal file for enclosed-exceptions-1.0.2 with Git SHA of 828ccfe89d675893f66774c5e9f9b4c64d4ea97f
Did not find .cabal file for amazonka-core-1.4.3 with Git SHA of 4bee585c675127f8ae93dfd6d5ca57634f908a34
The following GHC options are incompatible with GHCi and have not been passed to it: -threaded
Using main module: 1. Package `stack' component test:stack-integration-test with main-is file: /Users/pgiarrusso/AeroFS/Repos/stack-bluevelvet/test/integration/IntegrationSpec.hs
Configuring GHCi with the following packages: stack
GHCi, version 7.10.3: http://www.haskell.org/ghc/  :? for help
[1 of 1] Compiling StackTest        ( /Users/pgiarrusso/AeroFS/Repos/stack-bluevelvet/test/integration/lib/StackTest.hs, interpreted )

/Users/pgiarrusso/AeroFS/Repos/stack-bluevelvet/test/integration/lib/StackTest.hs:3:1: Warning:
    The import of ‘Control.Exception’ is redundant
      except perhaps to import instances from ‘Control.Exception’
    To import instances alone, use: import Control.Exception()

/Users/pgiarrusso/AeroFS/Repos/stack-bluevelvet/test/integration/lib/StackTest.hs:6:1: Warning:
    The import of ‘System.FilePath’ is redundant
      except perhaps to import instances from ‘System.FilePath’
    To import instances alone, use: import System.FilePath()

/Users/pgiarrusso/AeroFS/Repos/stack-bluevelvet/test/integration/lib/StackTest.hs:28:5: Warning:
    This binding for ‘stack’ shadows the existing binding
      defined at /Users/pgiarrusso/AeroFS/Repos/stack-bluevelvet/test/integration/lib/StackTest.hs:32:1

/Users/pgiarrusso/AeroFS/Repos/stack-bluevelvet/test/integration/lib/StackTest.hs:49:5: Warning:
    This binding for ‘stack’ shadows the existing binding
      defined at /Users/pgiarrusso/AeroFS/Repos/stack-bluevelvet/test/integration/lib/StackTest.hs:32:1

/Users/pgiarrusso/AeroFS/Repos/stack-bluevelvet/test/integration/lib/StackTest.hs:70:14: Warning:
    Defined but not used: ‘msg’

/Users/pgiarrusso/AeroFS/Repos/stack-bluevelvet/test/integration/lib/StackTest.hs:120:1: Warning:
    Top-level binding with no type signature: exeExt :: [Char]

/Users/pgiarrusso/AeroFS/Repos/stack-bluevelvet/test/integration/lib/StackTest.hs:123:1: Warning:
    Top-level binding with no type signature: isWindows :: Bool

/Users/pgiarrusso/AeroFS/Repos/stack-bluevelvet/test/integration/lib/StackTest.hs:129:1: Warning:
    Top-level binding with no type signature:
      defaultResolverArg :: [Char]
Ok, modules loaded: StackTest.
[2 of 2] Compiling Main             ( /Users/pgiarrusso/AeroFS/Repos/stack-bluevelvet/test/integration/IntegrationSpec.hs, interpreted )
Ok, modules loaded: StackTest, Main.
*Main StackTest> :cd test/integration/tests/1336-1337-new-package-names/
Warning: changing directory causes all loaded modules to be unloaded,
because the search path has changed.
Prelude> :load Main.hs
[1 of 2] Compiling StackTest        ( /Users/pgiarrusso/AeroFS/Repos/stack-bluevelvet/test/integration/lib/StackTest.hs, interpreted )

/Users/pgiarrusso/AeroFS/Repos/stack-bluevelvet/test/integration/lib/StackTest.hs:3:1: Warning:
    The import of ‘Control.Exception’ is redundant
      except perhaps to import instances from ‘Control.Exception’
    To import instances alone, use: import Control.Exception()

/Users/pgiarrusso/AeroFS/Repos/stack-bluevelvet/test/integration/lib/StackTest.hs:6:1: Warning:
    The import of ‘System.FilePath’ is redundant
      except perhaps to import instances from ‘System.FilePath’
    To import instances alone, use: import System.FilePath()

/Users/pgiarrusso/AeroFS/Repos/stack-bluevelvet/test/integration/lib/StackTest.hs:28:5: Warning:
    This binding for ‘stack’ shadows the existing binding
      defined at /Users/pgiarrusso/AeroFS/Repos/stack-bluevelvet/test/integration/lib/StackTest.hs:32:1

/Users/pgiarrusso/AeroFS/Repos/stack-bluevelvet/test/integration/lib/StackTest.hs:49:5: Warning:
    This binding for ‘stack’ shadows the existing binding
      defined at /Users/pgiarrusso/AeroFS/Repos/stack-bluevelvet/test/integration/lib/StackTest.hs:32:1

/Users/pgiarrusso/AeroFS/Repos/stack-bluevelvet/test/integration/lib/StackTest.hs:70:14: Warning:
    Defined but not used: ‘msg’

/Users/pgiarrusso/AeroFS/Repos/stack-bluevelvet/test/integration/lib/StackTest.hs:120:1: Warning:
    Top-level binding with no type signature: exeExt :: [Char]

/Users/pgiarrusso/AeroFS/Repos/stack-bluevelvet/test/integration/lib/StackTest.hs:123:1: Warning:
    Top-level binding with no type signature: isWindows :: Bool

/Users/pgiarrusso/AeroFS/Repos/stack-bluevelvet/test/integration/lib/StackTest.hs:129:1: Warning:
    Top-level binding with no type signature:
      defaultResolverArg :: [Char]
[2 of 2] Compiling Main             ( Main.hs, interpreted )

Main.hs:2:1: Warning:
    The import of ‘System.Directory’ is redundant
      except perhaps to import instances from ‘System.Directory’
    To import instances alone, use: import System.Directory()

Main.hs:3:1: Warning:
    The import of ‘System.FilePath’ is redundant
      except perhaps to import instances from ‘System.FilePath’
    To import instances alone, use: import System.FilePath()
Ok, modules loaded: StackTest, Main.
*Main> main
Running: /Users/pgiarrusso/AeroFS/Repos/stack-bluevelvet/.stack-work/install/x86_64-osx/lts-6.6/7.10.3/bin/stack new 1234a-4b-b4-abc-12b34
Downloading template "new-template" to create project "1234a-4b-b4-abc-12b34" in 1234a-4b-b4-abc-12b34/ ...

The following parameters were needed by the template but not provided: author-email, author-name, category, copyright, github-username
You can provide them in /Users/pgiarrusso/.stack/config.yaml, like this:
templates:
  params:
    author-email: value
    author-name: value
    category: value
    copyright: value
    github-username: value
Or you can pass each one as parameters like this:
stack new 1234a-4b-b4-abc-12b34 new-template -p "author-email:value" -p "author-name:value" -p "category:value" -p "copyright:value" -p "github-username:value"

Looking for .cabal or package.yaml files to use to init the project.
Using cabal packages:
- 1234a-4b-b4-abc-12b34/1234a-4b-b4-abc-12b34.cabal

Selecting the best among 8 snapshots...

* Matches lts-6.7

Selected resolver: lts-6.7
Initialising configuration using resolver: lts-6.7
Total number of user packages considered: 1
Writing configuration to file: 1234a-4b-b4-abc-12b34/stack.yaml
All done.
doesExist ./1234a-4b-b4-abc-12b34/stack.yaml
Running: /Users/pgiarrusso/AeroFS/Repos/stack-bluevelvet/.stack-work/install/x86_64-osx/lts-6.6/7.10.3/bin/stack new 1234-abc
Expected valid package name, but got: 1234-abc
Package names consist of one or more alphanumeric words separated by hyphens.
To avoid ambiguity with version numbers, each of these words must contain at least one letter.


Usage: stack new PACKAGE_NAME [--bare] [TEMPLATE_NAME] [-p|--param KEY:VALUE]
                 [DIRS] [--solver] [--omit-packages] [--force]
                 [--ignore-subdirs] [--help]
  Create a new project from a template. Run `stack templates' to see available
  templates.
doesNotExist ./1234-abc/stack.yaml
doesNotExist ./1234-abc
Running: /Users/pgiarrusso/AeroFS/Repos/stack-bluevelvet/.stack-work/install/x86_64-osx/lts-6.6/7.10.3/bin/stack new 1-abc
Expected valid package name, but got: 1-abc
Package names consist of one or more alphanumeric words separated by hyphens.
To avoid ambiguity with version numbers, each of these words must contain at least one letter.


Usage: stack new PACKAGE_NAME [--bare] [TEMPLATE_NAME] [-p|--param KEY:VALUE]
                 [DIRS] [--solver] [--omit-packages] [--force]
                 [--ignore-subdirs] [--help]
  Create a new project from a template. Run `stack templates' to see available
  templates.
Running: /Users/pgiarrusso/AeroFS/Repos/stack-bluevelvet/.stack-work/install/x86_64-osx/lts-6.6/7.10.3/bin/stack new 44444444444444
Expected valid package name, but got: 44444444444444
Package names consist of one or more alphanumeric words separated by hyphens.
To avoid ambiguity with version numbers, each of these words must contain at least one letter.


Usage: stack new PACKAGE_NAME [--bare] [TEMPLATE_NAME] [-p|--param KEY:VALUE]
                 [DIRS] [--solver] [--omit-packages] [--force]
                 [--ignore-subdirs] [--help]
  Create a new project from a template. Run `stack templates' to see available
  templates.
Running: /Users/pgiarrusso/AeroFS/Repos/stack-bluevelvet/.stack-work/install/x86_64-osx/lts-6.6/7.10.3/bin/stack new abc-1
Expected valid package name, but got: abc-1
Package names consist of one or more alphanumeric words separated by hyphens.
To avoid ambiguity with version numbers, each of these words must contain at least one letter.


Usage: stack new PACKAGE_NAME [--bare] [TEMPLATE_NAME] [-p|--param KEY:VALUE]
                 [DIRS] [--solver] [--omit-packages] [--force]
                 [--ignore-subdirs] [--help]
  Create a new project from a template. Run `stack templates' to see available
  templates.
Running: /Users/pgiarrusso/AeroFS/Repos/stack-bluevelvet/.stack-work/install/x86_64-osx/lts-6.6/7.10.3/bin/stack new 444-ば日本-4本
Expected valid package name, but got: 444-ば日本-4本
Package names consist of one or more alphanumeric words separated by hyphens.
To avoid ambiguity with version numbers, each of these words must contain at least one letter.


Usage: stack new PACKAGE_NAME [--bare] [TEMPLATE_NAME] [-p|--param KEY:VALUE]
                 [DIRS] [--solver] [--omit-packages] [--force]
                 [--ignore-subdirs] [--help]
  Create a new project from a template. Run `stack templates' to see available
  templates.
Running: /Users/pgiarrusso/AeroFS/Repos/stack-bluevelvet/.stack-work/install/x86_64-osx/lts-6.6/7.10.3/bin/stack new ば日本-4本
Downloading template "new-template" to create project "ば日本-4本" in ば日本-4本/ ...

The following parameters were needed by the template but not provided: author-email, author-name, category, copyright, github-username
You can provide them in /Users/pgiarrusso/.stack/config.yaml, like this:
templates:
  params:
    author-email: value
    author-name: value
    category: value
    copyright: value
    github-username: value
Or you can pass each one as parameters like this:
stack new ば日本-4本 new-template -p "author-email:value" -p "author-name:value" -p "category:value" -p "copyright:value" -p "github-username:value"

Looking for .cabal or package.yaml files to use to init the project.
Using cabal packages:
- ば日本-4本/ば日本-4本.cabal

Selecting the best among 8 snapshots...

* Matches lts-6.7

Selected resolver: lts-6.7
Initialising configuration using resolver: lts-6.7
Total number of user packages considered: 1
Writing configuration to file: ば日本-4本/stack.yaml
All done.
Running: /Users/pgiarrusso/AeroFS/Repos/stack-bluevelvet/.stack-work/install/x86_64-osx/lts-6.6/7.10.3/bin/stack new אבהץש
Downloading template "new-template" to create project "אבהץש" in אבהץש/ ...

The following parameters were needed by the template but not provided: author-email, author-name, category, copyright, github-username
You can provide them in /Users/pgiarrusso/.stack/config.yaml, like this:
templates:
  params:
    author-email: value
    author-name: value
    category: value
    copyright: value
    github-username: value
Or you can pass each one as parameters like this:
stack new אבהץש new-template -p "author-email:value" -p "author-name:value" -p "category:value" -p "copyright:value" -p "github-username:value"

Looking for .cabal or package.yaml files to use to init the project.
Using cabal packages:
- אבהץש/אבהץש.cabal

Selecting the best among 8 snapshots...

* Matches lts-6.7

Selected resolver: lts-6.7
Initialising configuration using resolver: lts-6.7
Total number of user packages considered: 1
Writing configuration to file: אבהץש/stack.yaml
All done.
Running: /Users/pgiarrusso/AeroFS/Repos/stack-bluevelvet/.stack-work/install/x86_64-osx/lts-6.6/7.10.3/bin/stack new ΔΘΩϬ
Downloading template "new-template" to create project "ΔΘΩϬ" in ΔΘΩϬ/ ...

The following parameters were needed by the template but not provided: author-email, author-name, category, copyright, github-username
You can provide them in /Users/pgiarrusso/.stack/config.yaml, like this:
templates:
  params:
    author-email: value
    author-name: value
    category: value
    copyright: value
    github-username: value
Or you can pass each one as parameters like this:
stack new ΔΘΩϬ new-template -p "author-email:value" -p "author-name:value" -p "category:value" -p "copyright:value" -p "github-username:value"

Looking for .cabal or package.yaml files to use to init the project.
Using cabal packages:
- ΔΘΩϬ/ΔΘΩϬ.cabal

Selecting the best among 8 snapshots...

* Matches lts-6.7

Selected resolver: lts-6.7
Initialising configuration using resolver: lts-6.7
Total number of user packages considered: 1
Writing configuration to file: ΔΘΩϬ/stack.yaml
All done.
doesExist ./ΔΘΩϬ/stack.yaml
doesExist ./ΔΘΩϬ/ΔΘΩϬ.cabal
*Main>
Leaving GHCi.

@@ -560,8 +562,10 @@ cabalPackagesCheck cabalfps noPkgMsg dupErrMsg = do
-- Just the latter check is enough to cover both the cases

let packages = zip cabalfps gpds
-- XXX taken from https://github.com/ppelleti/normalization-insensitive, see #1810
Copy link
Collaborator

@harendra-kumar harendra-kumar Jul 21, 2016

Choose a reason for hiding this comment

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

Its just a String to Text conversion and back. I don't think you need this comment here. You can call it normalizeString to indicate that this is for String.

@Blaisorblade
Copy link
Collaborator Author

I'm on holiday, but I'll address the comments and rebase the PR next week.

The filename gets normalized to NFD on OS X causing spurious failures.

Let's normalize both sides to NFC in case the Cabal file is in some
other normalization for any reason.

This is a slight hack.
… X, for now (commercialhaskell#1810)"

This reverts commit 87fec57, now that
we have a proper fix.
@Blaisorblade Blaisorblade force-pushed the 1810-unicode-normalized-pkg-name-comparison branch from 65c4a37 to ccb120e Compare August 2, 2016 14:31
@Blaisorblade
Copy link
Collaborator Author

I just ran the integration tests locally on OS X (ahem, with LTS-6.6), tests passed, and it seems uncontroversial enough. Hence I'm merging the PR (this seems OK for stack, though discouraged for other projects). Thanks to @harendra-kumar for his comments!

@Blaisorblade Blaisorblade merged commit ccc7229 into commercialhaskell:master Aug 2, 2016
@Blaisorblade Blaisorblade deleted the 1810-unicode-normalized-pkg-name-comparison branch August 2, 2016 17:58
@Blaisorblade Blaisorblade restored the 1810-unicode-normalized-pkg-name-comparison branch August 2, 2016 21:28
@Blaisorblade Blaisorblade deleted the 1810-unicode-normalized-pkg-name-comparison branch August 3, 2016 09:30
@borsboom
Copy link
Contributor

borsboom commented Aug 4, 2016

Hurrah!

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