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

VersionInfo: Mimic github's hash styling #7694

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

Conversation

tresf
Copy link
Member

@tresf tresf commented Feb 8, 2025

The problem:

  • Our installers are versioned with git hashes that are invalid due to a leading g.
  • The hashes used do not reference the latest PR commit, but rather a merge commit.

For example, this is what our software may be versioned:

  • gb353cf1e7 (taken from lmms-1.3.0-alpha.1.760+pr7693.gb353cf1e7-linux-aarch64.AppImage)

This is the commit hash that it references:

Or put simpler:

gb353cf1e7
_b353cf1__

This makes it visually more difficult to correlate a build to a commit hash.

This PR simply shortens the hash to match github and snags the commit hash from the merge commit description.

Adjacent tasks:

@tresf tresf marked this pull request as draft February 8, 2025 22:50
@tresf
Copy link
Member Author

tresf commented Feb 8, 2025

Hmm... Something about VersionInfo.cmake doesn't seem to be adding the commit id that I would expect... Investigating.

Edit: Yeah, it's because PRs use merge-commits. I'm not sure this can be easily avoided.

@tresf
Copy link
Member Author

tresf commented Feb 9, 2025

So this does as described, but the merge commit ends up in the file (whereas the PR commit ends up on the download button). It would be really nice to have these match so that we're not trying to cross reference merge-commits to their respective branch commits.

@tresf
Copy link
Member Author

tresf commented Feb 11, 2025

So this does as described, but the merge commit ends up in the file (whereas the PR commit ends up on the download button). It would be really nice to have these match so that we're not trying to cross reference merge-commits to their respective branch commits.

I believe I found a way to get them to match. 🤞

@tresf
Copy link
Member Author

tresf commented Feb 11, 2025

I believe I found a way to get them to match. 🤞

Nope 😆

@tresf
Copy link
Member Author

tresf commented Feb 11, 2025

Got it! Next I'll test the behavior with a force-commit by squashing everything down. Edit: Works.

@tresf tresf marked this pull request as ready for review February 11, 2025 05:56
tresf added a commit to LMMS/lmms.io that referenced this pull request Feb 11, 2025
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.

1 participant