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

Detect unused packages with new-build #5331

Open
4 tasks
vmchale opened this issue May 19, 2018 · 16 comments
Open
4 tasks

Detect unused packages with new-build #5331

vmchale opened this issue May 19, 2018 · 16 comments

Comments

@vmchale
Copy link
Contributor

vmchale commented May 19, 2018

I would like cabal to support detection of redundant packages with cabal new-build, like weeder or packunused. This would entail:

  • Detecting redundant packages in the .cabal file
  • Making a command-line flag to toggle detection of redundant packages
  • Making a field in cabal.project files to toggle detection of redundant packages
  • Print out warnings/errors and set exit code when redundant packages are found

I can do all of the work, but I would like to know that this is a welcome feature before getting started.

@angerman
Copy link
Collaborator

I for one would like to see this. Even though I like the Unix approach of one tool for one task, having this in cabal without the need for a secondary tool would get a lot more usage than what wieder or packunused get today.

I would also argue we have precedence for this in cabal already with the missing module listing warnings.

@gbaz
Copy link
Collaborator

gbaz commented May 19, 2018

prior ticket on this: #1820

i'd be all for this.

@hvr
Copy link
Member

hvr commented May 19, 2018

Fwiw, it was always the plan for me to eventually integrate packunused into cabal, but this is missing an important feature from GHC that didn't make it for GHC 8.4 unfortunately (i.e. having GHC dump the module import dependency graph in a useful way; we already had a diff for that on phabricator -- it's a trivial patch); as the approach packunused is using is fragile/broken/hacky, and so is not suitable for integrating into cabal.

@Yuras
Copy link
Contributor

Yuras commented Oct 28, 2018

I think we can teach GHC to warn about unused dependencies. It already has everything on hands to do the check.

GHC loads modules lazyly, so at the end of compilation it has a list of actually used packages, see eps_PIT in ExternalPackageState. We can compare it with the list of --package-ids passed by cabal. Seems to be a simple patch, almost a no-brainer. @hvr, do you think it will work?

@hvr
Copy link
Member

hvr commented Oct 28, 2018

@Yuras I'd like to have such a warning in GHC but I don't think this is future-proof if I understand your suggestion correctly as it relies on using ghc --make, i.e. letting GHC be in charge of compiling the whole component-unit. However, we have this long-standing plan to move away from ghc --make towards having cabal orchestrate compilations with single module granularity to allow for more control for better parallelism utilization and other potential benefits (that's one of the other motivations for the phabricator diff).

That being said, if it's a simple patch, I'd strongly suggest to implement it and try it; it may serve as a very useful short-term stopgap solution which I'd love to use if it works.

@Yuras
Copy link
Contributor

Yuras commented Oct 30, 2018

@hvr I made a patch, see https://ghc.haskell.org/trac/ghc/ticket/15838

@Yuras
Copy link
Contributor

Yuras commented Nov 1, 2018

@hvr The approach I described doesn't work for indirect dependencies. Simply because they are actually required for compilation. E.g. if we specify text and bytestring in build-depends, but import only modules from text, then bytestring will not be reported as unused because text depends on bytestring. Knowing that, I still find the approach useful, though other people may find it not worthy.

@emilypi
Copy link
Member

emilypi commented May 26, 2021

I'm in favor of this. PR's welcome, and I'm available for review.

@phadej
Copy link
Collaborator

phadej commented May 26, 2021

https://downloads.haskell.org/ghc/latest/docs/html/users_guide/using-warnings.html#ghc-flag--Wunused-packages, i.e. having

package my-package
  ghc-options: -Wunused-packages

should work

@Mikolaj
Copy link
Member

Mikolaj commented May 26, 2021

This modern technology is astounding. So what do we do? Dear issue participants, can you verify this works as required and recommend the closure of this issue?

@Yuras
Copy link
Contributor

Yuras commented May 27, 2021

@Mikolaj The -Wunused-packages warns when package is not needed to build the code. But even when package is needed to build the code, it might still not be needed in cabal file, so the warning doesn't solve the issue completely. (I'm not against closing the issue though if you don't expect any work toward the full solution in a near future.)

@hasufell
Copy link
Member

hasufell commented May 27, 2021

@Mikolaj The -Wunused-packages warns when package is not needed to build the code. But even when package is needed to build the code, it might still not be needed in cabal file, so the warning doesn't solve the issue completely. (I'm not against closing the issue though if you don't expect any work toward the full solution in a near future.)

It also is very confusing output, because there is no location info and it just dumps several checks for every library and executable you have.

And when you have a compilation failure, the output is outright wrong.

I also have the impression I get incorrect suggestions when using if-conditions in cabal.

So after trying this for half an hour, I gave up.

@phadej
Copy link
Collaborator

phadej commented May 27, 2021

And when you have a compilation failure, the output is outright wrong.

Is there a GHC ticket about that?

@Yuras
Copy link
Contributor

Yuras commented May 27, 2021

@hasufell Sorry for your bad experience with this flag. Let's see what we can improve there.

because there is no location info

Yep. The warning is about command line options, so there is no location. And GHC doesn't know anything about cabal file.

it just dumps several checks for every library and executable you have

That might be cabal issue. E.g. it's running GHC multiple times (compilation and linking), and AFAIK there is no way in cabal to specify GHC options only for compilation run.

And when you have a compilation failure, the output is outright wrong.

I guess it should be possible to suppress it in that case. But why do you care about this warning in case of build failure? I mean, just ignore it's output.

I also have the impression I get incorrect suggestions when using if-conditions in cabal.

It might be cabal issue. Could you please check whether cabal runs GHC with the conditioned-out dependencies?

@ulysses4ever
Copy link
Collaborator

IMO it’d be strange for cabal to reimplement and maintain a feature that is already available in GHC albeit with a suboptimal UI. The right direction would be to improve the GHC feature rather than duplicate.

@ulysses4ever
Copy link
Collaborator

No objections, so optimistically closing. Feel free to reopen if disagree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests