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

[ re #7173 ] Add the license information more in Paths_*pkgname* #7443

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

L-TChen
Copy link

@L-TChen L-TChen commented Jun 15, 2021

I am trying to resolve #7173 by embedding adding the license information to the module Paths_<pkgname>. There are a few design issues I am not sure about.

  • Should the license type be stored as String or as License type? In the current draft, license :: String is pretty printed using Distribution.Pretty, i.e. Z.zLicense = show $ pretty $ license pkg_descr in Cabal/src/Distribution/Simple/Build/PathsModule.hs. I don't know if there is any possible application requiring License instead of String.

    • Making license of type License means that Distribution.License needs to be imported and thus Paths_<pkgname> depends on the package cabal. That is essentially the reason I am against using License.
    • However, having a separate module like License_<pkgname> may alleviate the need for an extra dependency.
  • Should it be a separate module instead? I plan to embed license files using Template Haskell, but I have the impression that invoking TH slows down the compilation in general.


Update (Jun. 15):

Anything fancier than String results in an additional dependency anyway, including template-haskell. I plan to add the following in Paths_<pkgname>:

  • license :: String
  • licenseFiles :: [FilePath]

where license and licenseFiles correspond to the fields license and license-file in a .cabal file. Since license files are not copied into the destination folder (please correct me if it is not the case), a sensible way to access its content is via Template Haskell to embed its content.


Please include the following checklist in your PR:

Please also shortly describe how you tested your change. Bonus points for added tests!

@L-TChen L-TChen force-pushed the issue-7173-more-data-Paths_pkgname branch from abb7727 to 1056324 Compare June 15, 2021 12:37
@Mikolaj
Copy link
Member

Mikolaj commented Jun 15, 2021

Thank you for the PR. I've just read the issue and one question seems unanswered: what to do when there are multiple licenses. What's your preference there?

@phadej
Copy link
Collaborator

phadej commented Jun 15, 2021

  • At the bare minimum this must be guarded by cabal-version
  • Filepaths is a very bad idea. There are plenty good reasons to not hard code (installation) paths into libraries, don't make it harder to make it happen (at the every least the path getting function should be in IO c.f. getDataDir). Note: original issue is about embedding textual content of them.

EDIT: I'm sure someone would ask to be able to generate&include transitive dependency closure of a given library&executable, so maybe the design should be extended directly to cover that use case. The usefulness of & license (& text) of a single library is not IMHO motivated enough, that's quite static information - isn't it. OTOH, the dependency closure isn't so static nor easy to accumulate (there is cabal-plan license-report but it barely helps with embedding - which BSD-3-Clause 2nd clause requires btw.)

@L-TChen
Copy link
Author

L-TChen commented Jun 15, 2021

Alright, thanks for your reply. I was not so sure how to do it, but I think I can do more than just giving file paths.

What to do when there are multiple licenses. What's your preference there?

I will have [String] instead.

Filepaths is a very bad idea.

You are right. I forgot that I can simply fetch the license file during generating the module Paths_<pkgname>.

the dependency closure isn't so static nor easy to accumulate

The dependency graph will be available after cabal resolves the dependencies, right? Is there any way to fetch that information? Is it sufficient to use the argument of type LocalBuildInfo passed to writeAutogenFiles in Distribution.Simple.Build?

@L-TChen L-TChen force-pushed the issue-7173-more-data-Paths_pkgname branch from 1056324 to 0839b0c Compare June 28, 2021 17:02
@mergify mergify bot added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Sep 1, 2022
@ulysses4ever ulysses4ever removed the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Sep 3, 2022
@wenkokke
Copy link

@L-TChen What still needs to happen before this can be merged?

@L-TChen
Copy link
Author

L-TChen commented May 18, 2023

Sorry for not getting back to you sooner. IIRC, this PR was for some discussion first to figure out the appropriate way to add license information to the module Paths_<pkgname>, and I only tried to locate what files are needed to modify.

I plan to work on this PR this summer.

@L-TChen
Copy link
Author

L-TChen commented Aug 1, 2023

I haven’t figured out how to get the text of the license file of every dependent package when generating the module Paths_<pkgname>. License files are usually placed in the cabal store, but their location does not seem available in the existing records. For installed packages, the type of license is already in the InstalledPackageInfo but the license file might be lost.

My current understanding is that it might be worthwhile to keep the license file as a field of type [Text] in InstalledPackageInfo, so it can be easily accessed from the field installedPkgs :: InstalledPackageIndex of LocalBuildInfo.

Any hint?

@phadej
Copy link
Collaborator

phadej commented Aug 3, 2023

My current understanding is that it might be worthwhile to keep the license file as a field of type

Not a great idea.

My cabal store has 12000 packages currently. Reading its package database (i.e. 12000 InstalledPackageInfos) is already quite slow. Traversing through license text would slow it even further.

If you need a file location(s), add a field for field locations.

@L-TChen L-TChen force-pushed the issue-7173-more-data-Paths_pkgname branch from 0839b0c to 8d01287 Compare August 7, 2023 06:24
@L-TChen
Copy link
Author

L-TChen commented Aug 7, 2023

I hit the issue #701. I am patching the record type InstalledPackage with a new field licenseFiles :: [FilePath].

IIUC, I will need to re-compile the ghc-pkg to get sync with the new field. GHC should use the latest cabal as of the GHC release, so I don’t have to file a PR to GHC, right?

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

Successfully merging this pull request may close these issues.

Feature request: provide text of LICENSE file as String/Text in generated Paths_XYZ
6 participants