Skip to content

Commit

Permalink
Revert "init: check pkg name and .cabal file name match"
Browse files Browse the repository at this point in the history 
This reverts commit 63c7f48.

@harendra-kumar The reverted commit seems to break the
1336-1337-new-package-names integration test on Mac OS X.  I did
a bit of debugging and it appears that, somewhere along the way,
some unicode characters are changing (they look the same when
rendered, but the UTF-8 bytes are different) and that is causing
the comparison to fail.

For example:

```
$ stack new ば日本-4本
[...]
Package name as defined in the .cabal file must match the .cabal file name.
Please fix the following packages and try again:
- ば日本-4本/ば日本-4本.cabal
```

`show (FP.takeBaseName $ toFilePath fp,show $ gpdPackageName gpd)`
in this case gives me
`("\12399\12441\26085\26412-4\26412","\12400\26085\26412-4\26412")`;
note the first two bytes in the fst are different than the first byte
in the snd.
  • Loading branch information
@borsboom
borsboom committed Feb 19, 2016
1 parent d71b34e commit cf18703
Showing 1 changed file with 6 additions and 14 deletions.
20 changes: 6 additions & 14 deletions src/Stack/Solver.hs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -515,23 +515,15 @@ cabalPackagesCheck cabalfps noPkgMsg dupErrMsg = do
(warnings, gpds) <- fmap unzip (mapM readPackageUnresolved cabalfps)
zipWithM_ (mapM_ . printCabalFileWarning) cabalfps warnings

-- package name cannot be empty or missing otherwise
-- it will result in cabal solver failure.
-- stack requires packages name to match the cabal file name
-- Just the latter check is enough to cover both the cases

let packages = zip cabalfps gpds
getNameMismatchPkg (fp, gpd)
| (show . gpdPackageName) gpd /= (FP.takeBaseName . toFilePath) fp
= Just fp
getEmptyNamePkg (fp, gpd)
| ((show . gpdPackageName) gpd) == "" = Just fp
| otherwise = Nothing
nameMismatchPkgs = mapMaybe getNameMismatchPkg packages
emptyNamePkgs = mapMaybe getEmptyNamePkg packages

when (nameMismatchPkgs /= []) $ do
rels <- mapM makeRelativeToCurrentDir nameMismatchPkgs
error $ "Package name as defined in the .cabal file must match the \
\.cabal file name.\n\
\Please fix the following packages and try again:\n"
when (emptyNamePkgs /= []) $ do
rels <- mapM makeRelativeToCurrentDir emptyNamePkgs
error $ "Please assign a name to the following package(s):\n"
<> (formatGroup rels)

let dupGroups = filter ((> 1) . length)
Expand Down

9 comments on commit cf18703

@borsboom
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@harendra-kumar I guess when I say "byte" above I should say "codepoint". I wouldn't normally revert right away, but I'm trying to get a release out the door and this looks pretty safe to revert without affecting anything else.

@harendra-kumar
Copy link
Collaborator

Choose a reason for hiding this comment

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

Its ok to revert this one. We can fix it post release. I will take a look what's going on.

@harendra-kumar
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is stack build working fine once this package gets created?

@harendra-kumar
Copy link
Collaborator

Choose a reason for hiding this comment

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

I raised #1810 to track this.

@borsboom
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually didn't test that.

@harendra-kumar
Copy link
Collaborator

Choose a reason for hiding this comment

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

If its not just a problem with the comparison, meaning stack new is really creating a filename which is different than the package name then stack build will fail because it performs the same check. In that case this revert will push the problem to later which is in fact a tad worse than pre-revert.

@borsboom
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I just tested and stack build does indeed fail for this case.

@borsboom
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Uncicode is fun. I suspect the problem is somewhere in String->Text->String conversion, since Text handles unicode differently than String.

manny@eb-mbp14a:~/fpco/tmp/ば日本-4本$ stack build
cabal file path /Users/manny/fpco/tmp/ば日本-4本/ば日本-4本.cabal does not match the package name it defines.
Please rename the file to: ば日本-4本.cabal
For more information, see: https://github.com/commercialhaskell/stack/issues/317
manny@eb-mbp14a:~/fpco/tmp/ば日本-4本$ mv ば日本-4本.cabal ば日本-4本.cabal
manny@eb-mbp14a:~/fpco/tmp/ば日本-4本$ stack build
cabal file path /Users/manny/fpco/tmp/ば日本-4本/ば日本-4本.cabal does not match the package name it defines.
Please rename the file to: ば日本-4本.cabal
For more information, see: https://github.com/commercialhaskell/stack/issues/317

@borsboom
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've un-reverted this change in master, since it is just making a pre-existing problem happen sooner. Since it causes integration tests to fail on Mac OS X, I'm giving this priority P1: Must. Ideally, the old bug should also have caused integration tests to fail.

Please sign in to comment.