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

feat(manager): add Cabal/Haskell manager using Hackage/PVP #33142

Merged
merged 19 commits into from
Jan 13, 2025

Conversation

ysangkok
Copy link
Contributor

@ysangkok ysangkok commented Dec 16, 2024

Changes

The Haskell programming language has a package manager called cabal-install, commonly referred to as 'Cabal'. It's package definition files have the extension .cabal.

This PR contains code to extract build-depends fields from .cabal files.

The build-depends field syntax is documented here.

This was tested on the repo https://github.com/ysangkok/haskell-tz (as you can see in the pull requests tab)

Context

For a discussion on this particular feature, see discussion at

Note that this doesn't contain support for package.yaml files which are used
with hpack. package.yaml files are used to generate the cabal files.
So .cabal file support could be considered more general,
since it covers all packages uploaded to Hackage.

However, .cabal files do use a custom syntax which requires a bit of work to parse.
That is the bulk of this PR.

This PR also doesn't contain code to bump e.g. git commit references in
stack.yaml which is mentioned in issue #8187.
This is already implemented in projects like stack-lint-extra-deps,
so I consider it less pressing.

Even though these are largely orthogonal features, they can reuse the
Hackage/PVP code previously added.

Documentation (please check one with an [x])

  • I have updated the documentation, or
  • No documentation update is required

How I've tested my work (please select one)

I have verified these changes via:

  • Code inspection only, or
  • Newly added/modified unit tests, or
  • No unit tests but ran on a real repository, or
  • Both unit tests + ran on a real repository

@rarkins rarkins requested a review from zharinov December 17, 2024 05:17
Copy link
Collaborator

@zharinov zharinov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks good to me, but please rewrite regex

lib/modules/manager/haskell-cabal/extract.ts Fixed Show resolved Hide resolved
Packages cannot start with a hyphen.

But they can start with a number:
- https://hackage.haskell.org/package/3d-graphics-examples

They can be a single uppercase character:
- https://hackage.haskell.org/package/H

They can be a single lowercase character:
- https://hackage.haskell.org/package/j
@ysangkok
Copy link
Contributor Author

@zharinov I have replaced the regex with validation that rejects more invalid package names.

@zharinov zharinov self-requested a review December 17, 2024 19:03
@zharinov
Copy link
Collaborator

Could you provide couple repos for testing? For example, fork of some existing one that have many deps, with Renovate configured for it

@ysangkok
Copy link
Contributor Author

@zharinov Yes, this is the configuration file that I use for testing, named default.json5:

{
  "$schema": "https://docs.renovatebot.com/renovate-schema.json",
  "persistRepoData": true,
  "onboarding": false,
  "enabledManagers": ["haskell-cabal"],
  "packageRules":
    [{ "matchDatasources": ["hackage"]
     , "rangeStrategy": "widen"
    }],
  "platform": "github", // Could also be 'local', but then no PRs are made
  //"dryRun": "full",
  "token": "<paste your github token here>",
  "repositories": [{ repository: 'ysangkok/diagrams-builder' }, { repository: 'ysangkok/haskell-tz'} ],
}

I compile renovate using npm run compile:ts and then execute, in a sibling directory:

RENOVATE_REQUIRE_CONFIG=ignored RENOVATE_CONFIG_FILE=$PWD/default.json5 LOG_LEVEL=debug node ../renovate/dist/renovate.js

I just ran this, and it made PRs:

@ysangkok
Copy link
Contributor Author

Sorry to have invalidated the review, but this should now be ready for review again. Per the Slack discussion in #general, I have improved the handling of rangeStrategy. rangeStrategy is converted from auto before it reaches versioning, so it doesn't make sense for versioning to handle only auto. I have corrected this mistake in PVP, and I have made the new manager default to widen, which is the only strategy that PVP supports right now.

@ysangkok ysangkok requested a review from zharinov December 20, 2024 13:41
@ysangkok
Copy link
Contributor Author

ysangkok commented Jan 2, 2025

@secustor You requested in #31434 that I split up the work in at least three PRs. I have done this, and this constitutes the third PR in the series. I am planning further work in separate PRs though, like e.g. adding support for rangeStrategy=pin in the PVP versioning module. But that will be in an upcoming PR, because it is somewhat orthogonal to this PR.

@secustor secustor self-requested a review January 2, 2025 20:34
lib/modules/manager/haskell-cabal/extract.spec.ts Outdated Show resolved Hide resolved
lib/modules/manager/haskell-cabal/extract.spec.ts Outdated Show resolved Hide resolved
lib/modules/manager/haskell-cabal/extract.spec.ts Outdated Show resolved Hide resolved
lib/modules/manager/haskell-cabal/extract.ts Outdated Show resolved Hide resolved
lib/modules/versioning/pvp/index.ts Show resolved Hide resolved
@ysangkok ysangkok requested a review from secustor January 5, 2025 17:19
Copy link
Collaborator

@secustor secustor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a reference to the real repo you have used to test this manager.

lib/modules/manager/haskell-cabal/index.spec.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@secustor secustor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

else LGTM

lib/modules/manager/haskell-cabal/index.ts Outdated Show resolved Hide resolved
@rarkins rarkins requested a review from secustor January 8, 2025 12:26
@ysangkok
Copy link
Contributor Author

@secustor Thanks for all your comments, I am not sure how to further advance this. Not sure if Rhys is waiting for the green check mark?

@secustor
Copy link
Collaborator

I had to do a small change, but else LGTM.

Thanks for the patience and contribution!

@secustor secustor added this pull request to the merge queue Jan 13, 2025
Merged via the queue into renovatebot:main with commit 3c52395 Jan 13, 2025
37 of 38 checks passed
@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 39.107.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@ysangkok ysangkok deleted the haskell-cabal-manager branch January 13, 2025 22:28
@wolfgangwalther
Copy link

@ysangkok I have an example where the manager breaks some of the surrounding comments:

https://github.com/PostgREST/postgrest/pull/3861/files

It also adds a trailing comma, which is invalid syntax.

@ysangkok
Copy link
Contributor Author

@wolfgangwalther Thank you for the bug report regarding the comment and the trailing comma, I'll look into that. I'll add it to my issue tracking in the discussion page.

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

Successfully merging this pull request may close these issues.

6 participants