Skip to content

Comments

record Git revision in devel builds#10541

Merged
mergify[bot] merged 1 commit intohaskell:masterfrom
geekosaur:git-revs
Dec 22, 2024
Merged

record Git revision in devel builds#10541
mergify[bot] merged 1 commit intohaskell:masterfrom
geekosaur:git-revs

Conversation

@geekosaur
Copy link
Collaborator

cabal --version in such builds will include the Git commit and branch (if not master).

I've already been informed that this is not appropriate, but maybe someone will find something worth salvaging in it. It is also incomplete insofar as it needs a changelog entry and such (but probably not a test).

Consider this my last submission.

Template Α: This PR modifies behaviour or interface

Include the following checklist in your PR:

@geekosaur geekosaur linked an issue Nov 9, 2024 that may be closed by this pull request
@geekosaur geekosaur force-pushed the git-revs branch 2 times, most recently from c00f498 to 39b5fae Compare November 10, 2024 08:18
@geekosaur
Copy link
Collaborator Author

hilfy «cabal:git-revs@39b5fae1a$» Z$ $(cabal list-bin cabal) --version
cabal-install version 3.15.0.0 (commit 39b5fae on git-revs, Sun Nov 10 03:13:17 2024 -0500)
compiled using version 3.15.0.0 of the Cabal library (in-tree)

I did come up with a reasonable situation where Cabal could have different git information than cabal-install, so I added that back in. (Building a pre-release cabal-install package with a pre-release ghc, not in-tree, such that ghc's libraries/cabal is used.) I did simplify the output if it's the same, as shown above.

@ulysses4ever
Copy link
Collaborator

Hey @geekosaur! Do you have any plans around this one? It'd be great to have the SHA one way or another...

@geekosaur
Copy link
Collaborator Author

Also, someone checked "significant" in the checklist. The thing is, this only changes behavior for development builds; it won't be in release builds, because we can't use TH in those (consider that there's a Debian maintainer making unregisterised ghc builds for s390x and ppc64le among others), so I'm not sure it needs to be in release notes at all.

Copy link
Collaborator

@ulysses4ever ulysses4ever left a comment

Choose a reason for hiding this comment

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

I don't think we need separate git info machinery for the library and the tool since they live in the same repository. Maybe some corner cases could benefit from it but I doubt they appear in practice often enough to justify the sensitive cost of CPP (and code duplication between cabalGitInfo and cabalInstallGitInfo). I also don't see much value in the (in tree) label: I'd be happier with a binary choice --- SHA or no SHA depending on the flag --- if this saves on the duplication and the CPP.

I'd also try to avoid adding any more CPP at all costs. The usual hack with two modules conditionally employed in the cabal file based on the flag would be my preferred way.

But as discussed elsewhere, we really need this feature ASAP, so I don't want to hold this any longer.

Copy link
Member

@Mikolaj Mikolaj left a comment

Choose a reason for hiding this comment

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

LGTM

@geekosaur geekosaur added merge me Tell Mergify Bot to merge and removed attention: needs-review labels Dec 19, 2024
@mergify mergify bot added the ready and waiting Mergify is waiting out the cooldown period label Dec 19, 2024
@mergify mergify bot added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Dec 22, 2024
`cabal --version` in such builds will include the Git commit and
branch (if not `master`).
@mergify mergify bot merged commit d764489 into haskell:master Dec 22, 2024
48 checks passed
@Mikolaj
Copy link
Member

Mikolaj commented Mar 27, 2025

@mergify backport 3.14

@mergify
Copy link
Contributor

mergify bot commented Mar 27, 2025

backport 3.14

✅ Backports have been created

Details

@Mikolaj
Copy link
Member

Mikolaj commented Mar 27, 2025

There are conflicts and this is not an urgent fix, so I propose to skip the 3.14 backport.

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

Labels

attention: needs-backport 3.12 merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days merge me Tell Mergify Bot to merge ready and waiting Mergify is waiting out the cooldown period

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cabal --version on a non-release build should include a Git commit

3 participants