lspitzner / brittany

haskell source code formatter
GNU Affero General Public License v3.0
690 stars 72 forks source link

cabal new-install brittany fails #162

Closed andreabedini closed 3 years ago

andreabedini commented 6 years ago

The README suggests the following procedure

cabal unpack brittany
cd brittany-0.11.0.0
# cabal new-configure -w $PATH_TO_GHC_8_0
cabal new-build exe:brittany
# and it should be safe to just copy the executable, e.g.
cp `find dist-newstyle/ -name brittany -type f | xargs -x ls -t | head -n1` $HOME/.cabal/bin/

I wondered why cabal new-install would not work and indeed it doesn't

command: cabal new-install -j1 brittany
Resolving dependencies...
Build profile: -w ghc-8.4.3 -O1
In order, the following will be built (use -v for more details):
 - brittany-0.11.0.0 (exe:brittany) (requires build)
Configuring executable 'brittany' for brittany-0.11.0.0..
clang: warning: argument unused during compilation: '-nopie' [-Wunused-command-line-argument]
Preprocessing executable 'brittany' for brittany-0.11.0.0..
Building executable 'brittany' for brittany-0.11.0.0..

src-brittany/Main.hs:7:10: error:
     fatal error: 'prelude.inc' file not found
  |
7 | #include "prelude.inc"
  |          ^
#include "prelude.inc"
         ^~~~~~~~~~~~~
1 error generated.
`clang' failed in phase `C pre-processor'. (Exit code: 1)
Symlinking brittany

Very verbose output available in https://gist.github.com/gwils/918a8c5cf169716e9c5c502c0bb7f7f6

I am not sure if this is a problem in brittany or in cabal but hexagoxel on #hackage suggested I'd open the issue here. I might open it on cabal too.

lspitzner commented 6 years ago

@andreabedini

Could you try the following fix: In brittany.cabal, add

  include-dirs:
    srcinc

to the executable component. Those lines are already present in the library component, but it looks like new-install needs it on all components. Which makes sense, all of them use the include. Kind of curious that this oversight was not noticed sooner :-)

andreabedini commented 6 years ago

Thanks @lspitzner. I did have a go but the problem seems more complex.

1) I am not sure cabal new-install works at all for local packages (so I can't just apply your change to my working copy) 2) @fgaz on IRC pointed me to haskell/cabal#5399 (from which I dug up haskell/cabal#4120, which seems more relevant)

¯_(ツ)_/¯

lspitzner commented 6 years ago

Right, that was an uninformed suggestion. Presuming that it does not noticeably make anything worse I will include the potential fix in the next release (which is not too far away, ghc-8.6 compat). I don't see a need to make a release sooner just for testing this.

Fortunately I think that 4120 is not relevant as brittany does not use data-files.

We can leave this issue open as a reminder to test things after the next release.

hvr commented 6 years ago

@lspitzner btw, adding include-dirs: srcinc to the exe stanza is imo not the proper way to fix this!

The problem is rather that

library 
  install-includes: srcinc/prelude.inc 
  include-dirs: srcinc 

specifies the install-includes with an explicit path, instead of

library 
  install-includes: prelude.inc 
  include-dirs: srcinc 

then things should work, and there'd be no need to change anything in the exe component.

lspitzner commented 6 years ago

@hvr oh, good point. thanks for the correction!

lspitzner commented 6 years ago

although.. i don't really see the need to expose prelude.inc outside of the package. So I guess removing "install-includes" entirely plus per-component "include-dirs" is the way to go?

hvr commented 5 years ago

@lspitzner

So I guess removing "install-includes" entirely plus per-component "include-dirs" is the way to go?

Sorry for the late response. Yes, that sounds even better!

lspitzner commented 5 years ago

Should be fixed on master. At least I managed to get new-install to work once with the change in 784e4d0; it still does not like to overwrite existing symlinks it seems, and generally creates symlinks regardless of whether the build succeeded. When in doubt, delete existing symlink and look at verbose build output.

If this is confirmed to work we probably want a note in the README for this, too.

hvr commented 5 years ago

@lspitzner Have you tried w/ the latest cabal 2.4.1.0 release? that one shouldn't create symlinks on failure and be less silent about the failure, e.g.:

$ /opt/cabal/2.4/bin/cabal v2-install brittany
Warning: Parsing the index cache failed (Unknown encoding for constructor).
Trying to regenerate the index cache...
Resolving dependencies...
Build profile: -w ghc-8.4.4 -O1
In order, the following will be built (use -v for more details):
 - brittany-0.11.0.0 (exe:brittany) (requires build)
Starting     brittany-0.11.0.0 (exe:brittany)
Building     brittany-0.11.0.0 (exe:brittany)
Warning: Some package(s) failed to build. Try rerunning with -j1 if you can't see the error.
andreabedini commented 5 years ago

I just tested with cabal-install 3.0.0.0 (compiled using the same version of the cabal library)

Installing   brittany-0.12.0.0 (exe:brittany)
Completed    brittany-0.12.0.0 (exe:brittany)
cabal: installdir is not defined. Set it in your cabal config file or use
--installdir=<path>

passing installdir works correctly.

[~] cabal install brittany --installdir=$HOME/bin
Resolving dependencies...
Up to date
Symlinking 'brittany'
[~] brittany --version
brittany version 0.12.0.0
Copyright (C) 2016-2018 Lennart Spitzner
There is NO WARRANTY, to the extent permitted by law.

Good work everyone! :heart:

andreabedini commented 5 years ago

I just realised installdir is not defined is because I didn't update .cabal/config to version 3 (not that I was told to :P)

lspitzner commented 5 years ago

Thanks for confirming @andreabedini

The README was very recently updated to mention that flag. Will be in the next release.