Skip to content

Add revision information to plan.json #10980

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fgaz
Copy link
Member

@fgaz fgaz commented Jun 8, 2025

Closes #6186


Template Α: This PR modifies behaviour or interface

Include the following checklist in your PR:

QA Notes

  • cd to any project that uses packages from hackage
  • Run cabal build --dry all to produce a plan.json
  • inspect dist-newstyle/cache/plan.json
    • it should contain a pkg-revision field under all remote-repo or secure-repo packages
    • the field should correspond to the latest hackage revision published before the used index-state

@fgaz fgaz added re: cabal-plan Concerning functionality offered by `cabal-plan` type: enhancement labels Jun 8, 2025
@fgaz fgaz force-pushed the plan.json-revisions branch 4 times, most recently from fad4496 to 7c04a29 Compare June 8, 2025 12:36
@fgaz fgaz requested a review from mpickering June 8, 2025 12:39
@fgaz fgaz marked this pull request as ready for review June 8, 2025 12:39
Copy link
Collaborator

@geekosaur geekosaur left a comment

Choose a reason for hiding this comment

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

Tests would be nice, granting that we'd have to find a stable repo that has revision fields.

@fgaz fgaz force-pushed the plan.json-revisions branch from 7c04a29 to 538a5aa Compare June 8, 2025 14:24
@fgaz
Copy link
Member Author

fgaz commented Jun 8, 2025

I'm trying to write one using withRemoteRepo

@fgaz fgaz force-pushed the plan.json-revisions branch from 538a5aa to 6475619 Compare June 8, 2025 14:27
@fgaz fgaz force-pushed the plan.json-revisions branch from 6475619 to e4d9b28 Compare June 8, 2025 14:58
@fgaz
Copy link
Member Author

fgaz commented Jun 8, 2025

Okay, this should work, please have a look ^_^

@fgaz fgaz added the squash+merge me Tell Mergify Bot to squash-merge label Jun 8, 2025
Copy link
Collaborator

@mpickering mpickering left a comment

Choose a reason for hiding this comment

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

Perhaps the revision should be adjacent to "repo" in the RepoTarballPackage but either way it seems good to get the info into the plan.json somehow.

@fgaz fgaz removed the squash+merge me Tell Mergify Bot to squash-merge label Jun 8, 2025
@fgaz
Copy link
Member Author

fgaz commented Jun 8, 2025

I put it there because I thought it didn't apply to all repo types (RepoLocalNoIndex in particular), but indeed it's a bit weird to have it inside repo.
And even though I don't really see a use case for exposing revisions from local repos, I don't see any disadvantage either, so maybe I should just do it.

@geekosaur
Copy link
Collaborator

"If it's there, someone will come up with a use for it". And consistent behavior is generally a good thing.

@fgaz
Copy link
Member Author

fgaz commented Jun 8, 2025

Now I wonder if we should just have it at top level, next to pkg-name and pkg-version.
It's just a field that can be extracted from any cabal file that contains it anyway

@mpickering
Copy link
Collaborator

Now I wonder if we should just have it at top level, next to pkg-name and pkg-version. It's just a field that can be extracted from any cabal file that contains it anyway

That doesn't seem too bad to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
re: cabal-plan Concerning functionality offered by `cabal-plan` type: enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

plan.json should contain the Hackage revision number
4 participants