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

Failures in Haddock crash dependency installation #7372

Closed
ag-eitilt opened this issue Apr 28, 2021 · 4 comments
Closed

Failures in Haddock crash dependency installation #7372

ag-eitilt opened this issue Apr 28, 2021 · 4 comments

Comments

@ag-eitilt
Copy link
Collaborator

ag-eitilt commented Apr 28, 2021

Describe the bug
When incidentally generating Haddock (i.e. with documentation: True for cabal build, not with cabal haddock), the documentation failing shouldn't have any effect if the rest of the build was successful. This seems to have been fixed for the local package as per #5977, but is still an issue during dependency resolution.

To Reproduce
There are likely quite a few ways to hit this bug; two I have come across are below.

haskell/vector#383 -- since-fixed Haddock bugs still being present when using older compilers:

$ cat libcdio.cabal
...
  build-depends:
...
                  , vector
...
$ cabal v2-build --with-compiler ghc-8.4.4 
Resolving dependencies...
Build profile: -w ghc-8.4.4 -O1
In order, the following will be built (use -v for more details):
 - vector-0.12.3.0 (lib:vector) (requires build)
 - test-0.1.0.0 (lib) (configuration changed)
Starting     vector-0.12.3.0 (all, legacy fallback)
Building     vector-0.12.3.0 (all, legacy fallback)
Haddock      vector-0.12.3.0 (all, legacy fallback)
Warning: Failed to build documentation for vector-0.12.3.0 (which is required
by test-0.1.0.0).
$ echo $?
0

Functions named using Unicode characters on a TTY with fonts limited to (I think) 256 glyphs:

$ locale
LANG=en_US.iso88591
LC_CTYPE="en_US.iso88591"
LC_NUMERIC="en_US.iso88591"
LC_TIME="en_US.iso88591"
LC_COLLATE="en_US.iso88591"
LC_MONETARY="en_US.iso88591"
LC_MESSAGES="en_US.iso88591"
LC_PAPER="en_US.iso88591"
LC_NAME="en_US.iso88591"
LC_ADDRESS="en_US.iso88591"
LC_TELEPHONE="en_US.iso88591"
LC_MEASUREMENT="en_US.iso88591"
LC_IDENTIFICATION="en_US.iso88591"
LC_ALL=
$ cat test.cabal
...
  build-depends:
...
                  , constraints <0.11.2
...
$ cabal build
Resolving dependencies...
Build profile: -w ghc-8.10.4 -O1
In order, the following will be built (use -v for more details):
 - constraints-0.11.1 (lib) (requires download & build)
 - test-0.1.0.0 (lib) (configuration changed)
Downloading  constraints-0.11.1
Downloaded   constraints-0.11.1
Starting     constraints-0.11.1 (lib)
Building     constraints-0.11.1 (lib)
Haddock      constraints-0.11.1 (lib)
Warning: Failed to build documentation for constraints-0.11.1 (which is
required by test-0.1.0.0).
$ echo $?
0

Expected behavior
The warning to be displayed and the Haddock to not be installed, but for the rest of package installation to continue despite that and for the build to progress to the local test package.

System information

  • Linux x86_64 5.4.38-gentoo
  • cabal-install version 3.2.0.0
  • GHC 8.10.4 (normally) and 8.4.4 (according to the specific reproduction case)

Additional context
My guess is that the fixes in #6137 for cabal build were not carried over to cabal install. There might be other commands which likewise cause the generation of Haddock and fail, but I don't immediately know what those may be just reading down the list.

@Mikolaj
Copy link
Member

Mikolaj commented Apr 28, 2021

Hi! Thank you for the report. I'm confused, you say cabal install may be to blame, but I can't see any use of cabal install in your reproduction instructions.

@ag-eitilt
Copy link
Collaborator Author

I'm not familiar with the internals, but was imagining cabal install being used behind the scenes to install the dependencies used by the cabal build call. If that's not the case, then replace that with "the fixes were not carried over to the mechanisms behind dependency resolution". Sorry for the poor wording, I was getting quite tired and just wanted to send the issue note before I fell asleep.

@fgaz
Copy link
Member

fgaz commented Apr 28, 2021

I also am a bit confused: the issue you linked is the opposite to this: haddock failures did not propagate to the whole command. The linked pull request made the command fail on haddock failure.
In this ticket though you seem to want the opposite behavior: making haddock failures not fatal.
But this is not a good idea with cabal-install's nix-like architecture: the fact that you require the documentation to be built is part of the unit id that uniquely identifies that particular instance of the package. Skipping the docs would result in an entirely different id than the one that was requested as dependency.

I'd also say it's more correct to fail early when an intermediate build step failed, rather than carry on and result in a possibly unexpected state.

@ag-eitilt
Copy link
Collaborator Author

I also am a bit confused: the issue you linked is the opposite to this: haddock failures did not propagate to the whole command.

You're right, I meant to link #5232 but copied the number from the wrong tab (see: tiredness). In #5232, the consensus was that cabal build should not fail just because the docs do. As I gather, #5977 was pointing out that the first fix was a bit too far-reaching.

But this is not a good idea with cabal-install's nix-like architecture: the fact that you require the documentation to be built is part of the unit id that uniquely identifies that particular instance of the package. Skipping the docs would result in an entirely different id than the one that was requested as dependency.

Ah, for some reason I was thinking that the Haddock wouldn't be figured into the package ID. With that being the case, I do agree that the dependency resolution should fail. Thanks for your time!

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

No branches or pull requests

3 participants