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

Cabal interactive init is broken if project names overlap #6861

Closed
emilypi opened this issue May 29, 2020 · 6 comments · Fixed by #7044
Closed

Cabal interactive init is broken if project names overlap #6861

emilypi opened this issue May 29, 2020 · 6 comments · Fixed by #7044

Comments

@emilypi
Copy link
Member

emilypi commented May 29, 2020

Describe the bug
If the directory overlaps with an existing Hackage pacakge, choosing "yes" when presented with Should I generate a simple project with sensible defaults? [default: y] fails to produce a valid build, or even a valid .cabal file.

To Reproduce

λ П(a: A) haskell → mkdir foo
λ П(a: A) haskell → cd foo 
λ П(a: A) foo → cabal init
Should I generate a simple project with sensible defaults? [default: y] y

Guessing dependencies...

Generating LICENSE...
Generating Setup.hs...
Generating CHANGELOG.md...
Generating src/MyLib.hs...
Generating exe/Main.hs...
Generating test/MyLibTest.hs...
Error: no package name provided.
λ П(a: A) foo → ls
CHANGELOG.md LICENSE      Setup.hs     exe          src          test

Expected behavior
If we're going to offer this feature, it should work regardless of Hackage. The counter-intuitive fix is to choose a name directory name that doesn't overlap with Hackage:

λ П(a: A) haskell → mv foo foofoo 
λ П(a: A) haskell → cd foofoo 
λ П(a: A) foofoo → cabal init 
Should I generate a simple project with sensible defaults? [default: y] y

Guessing dependencies...

Generating LICENSE...
Generating Setup.hs...
Generating CHANGELOG.md...
Generating src/MyLib.hs...
Generating exe/Main.hs...
Generating test/MyLibTest.hs...
Generating foofoo.cabal...

Warning: no synopsis given. You should edit the .cabal file and add one.
You may want to edit the .cabal file and add a Description field.
λ П(a: A) foofoo → ls
CHANGELOG.md LICENSE      Setup.hs     exe          foofoo.cabal src          test

However, if I'm a beginner, or offline, I have no way of checking this information. In fact, for a bare install, I have no way of knowing that this is a requirement! If i have a brand new install, then it works (RIP my ~/.cabal):

λ П(a: A) foofoo → rm -rf ~/.cabal
λ П(a: A) foofoo → cd ..
λ П(a: A) haskell → rm -rf foofoo 
λ П(a: A) haskell → mkdir foo
λ П(a: A) haskell → cd foo 
λ П(a: A) foo → cabal init
Config file path source is default config file.
Config file /Users/emilypi/.cabal/config not found.
Writing default configuration to /Users/emilypi/.cabal/config
Warning: The package list for 'hackage.haskell.org' does not exist. Run 'cabal
update' to download it.RemoteRepo {remoteRepoName = "hackage.haskell.org",
remoteRepoURI = http://hackage.haskell.org/, remoteRepoSecure = Just True,
remoteRepoRootKeys =
["fe331502606802feac15e514d9b9ea83fee8b6ffef71335479a2e68d84adc6b0","1ea9ba32c526d1cc91ab5e5bd364ec5e9e8cb67179a471872f6e26f0ae773d42","2c6c3627bd6c982990239487f1abd02e08a02e6cf16edb105a8012d444d870c3","0a5c7ea47cd1b15f01f5f51a33adda7e655bc0f0b0615baa8e271f4c3351e21d","51f0161b906011b52c6613376b1ae937670da69322113a246a09f807c62f6921"],
remoteRepoKeyThreshold = 3, remoteRepoShouldTryHttps = True}

Guessing dependencies...

Generating LICENSE...
Warning: unknown license type, you must put a copy in LICENSE yourself.
Generating Setup.hs...
Generating CHANGELOG.md...
Generating Main.hs...
Generating foo.cabal...

Warning: no synopsis given. You should edit the .cabal file and add one.
You may want to edit the .cabal file and add a Description field.
λ П(a: A) foo → 

System information

  • MacOS
  • Cabal 3.2
  • GHC 8.8.3
@emilypi emilypi changed the title Cabal interactive "sensible" project defaults is broken Cabal interactive "sensible" project defaults is broken if project names overlap May 29, 2020
@emilypi emilypi changed the title Cabal interactive "sensible" project defaults is broken if project names overlap Cabal interactive "sensible" project defaults option is broken if project names overlap May 29, 2020
@phadej phadej changed the title Cabal interactive "sensible" project defaults option is broken if project names overlap Cabal interactive init is broken if project names overlap May 30, 2020
@phadej
Copy link
Collaborator

phadej commented May 30, 2020

cc @m-renaud

@m-renaud
Copy link
Collaborator

This appears to be broken regardless of the init flow, so I suspect this bug has been here a while:

$ mkdir foo
$ cd foo
$ cabal init --non-interactive --exe
$ ls
CHANGELOG.md  exe  Setup.hs

Interestingly, if you explicitly specify the package name with -p you get a useful error message:

$ cabal init --non-interactive --exe -p foo
This package name is already used by another package on hackage. Do you want to choose a different name? [default: y] 

I can take a look this week.

@jhrcek
Copy link
Contributor

jhrcek commented Jun 28, 2020

I tried with master and the problem lies in this function:
The package name is taken from 3 potential sources:

  1. --package-name=PACKAGE
  2. --package-dir=DIRECTORY (used when 1 not provided)
  3. name of the current working directory (used when 1 and 2 not provided)

Using an already existing package name only 1 leads to desirable fail-fast behavior with understandable warning. 2 and 3 are silently translated to Nothing (

let guess' | isPkgRegistered guess = Nothing
) leading to Error: no package name provided. further down the line.

I think it should fail fast with a warning also in cases 2 & 3 instead of continuing without setting a name. Would you agree?
Please let me take a stab at this issue.

@m-renaud
Copy link
Collaborator

m-renaud commented Jul 2, 2020

Thanks for looking into this @jhrcek, and I agree with your assessment. If you put together a PR feel free to add me as reviewer.

@jhrcek
Copy link
Contributor

jhrcek commented Jul 3, 2020

@m-renaud @phadej I definitely want to contribute the fix, but I want to avoid just monkey patching.
But the fix won't be as easy as I thought. There are some issues how the code is currently structured

  1. there are cases when interactive prompt is shown even when cabal is invoked with --non-interative
    then promptYesNo promptOtherNameMsg (Just True)
  2. The file generation is not "transactional" - we should either write all the files or none of the files. That would require some validation BEFORE the project files are written not just erroring out when writing cabal file (when most other project files have been already written).

Unfortunately the current approach (composing bunch of InitFlags -> IO InitFlags functions in the IO monad

extendFlags :: InstalledPackageIndex -> SourcePackageDb -> InitFlags -> IO InitFlags
extendFlags pkgIx sourcePkgDb =
getSimpleProject
>=> getLibOrExec
>=> getCabalVersion
>=> getPackageName sourcePkgDb
>=> getVersion
>=> getLicense
>=> getAuthorInfo
>=> getHomepage
>=> getSynopsis
>=> getCategory
>=> getExtraSourceFiles
>=> getAppDir
>=> getSrcDir
>=> getGenTests
>=> getTestDir
>=> getLanguage
>=> getGenComments
>=> getModulesBuildToolsAndDeps pkgIx
)) doesn't give much flexibility for failing fast.

Would you be open to switching from plain IO to something like ExceptT InitError a?
Or would you be ok with calling some kind of die function from within getPackageName? Please suggest your favorite approach while I'll research the existing test suite code for how best to test this

PS.: What behavior would you expect from cabal init --non-interactive with project name that exists? Should the project be generated regardless of that with just a warning that that name is already taken (kind of like current behavior of cabal init --non-interactive --project-name=base and then answering "no" to the prompt about choosing different name)? Or should it just fail fast without generating any files?

@m-renaud
Copy link
Collaborator

So sorry for the very slow reply, I've been really busy lately.

Would you be open to switching from plain IO to something like ExceptT InitError a?

Yeah this approach sounds good to me as its a bit more flexible that the die approach and allows you to separate the "an error has occurred" from the handling of the error, allowing you to do something different in --interactive vs --non-interactive. Which leads me to your next question:

What behavior would you expect from cabal init --non-interactive with project name that exists?

Hmm, I think that --non-interactive should imply that we never ask for any information part way through in the command, so the only valid behaviour would be to not write any of the files and return a non-zero error code. That being said, we should make it very clear in the error message what happened and how to resolve (use a different package name with --package-name). If we wanted to allow this from the non-interactive version we could introduce a --allow-existing-package-name flag (to bikeshed) to not produce an error in that case, but this may be overboard since in general you shouldn't re-use a package name since you'll run into other issues down the line.

fendor pushed a commit to fendor/cabal that referenced this issue Feb 22, 2021
fendor pushed a commit to fendor/cabal that referenced this issue Feb 22, 2021
hololeap pushed a commit to hololeap/cabal that referenced this issue Apr 24, 2022
hololeap pushed a commit to hololeap/cabal that referenced this issue Apr 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants