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

Fix wrong base tag used for deployment and displayed in About dialog #4070

Merged
merged 1 commit into from
Jul 8, 2021

Conversation

Holzhaus
Copy link
Member

@Holzhaus Holzhaus commented Jul 7, 2021

By default, git describe shows the number of commits from the latest
tag in the git history. In our case we don't want that, because this can
cause problems:

        2.4-alpha              Expected: 2.4-alpha-N-abcdef123
main ---+X---------------+---> Actual: 2.3.0-M-abcdef456
         \              /
          X------------X--->
         2.3-beta     2.3.0

Hence, we need to add the --first-parent flag to git describe, so that
it only follows the first parent of a merge and does not pull in tags
from another branch.

Fixes mixxxdj/website#248.

See the issue for details: mixxxdj/website#248

By default, `git describe` shows the number of commits from the latest
tag in the git history. In our case we don't want that, because this can
cause problems:

            2.4-alpha              Expected: 2.4-alpha-N-abcdef123
    main ---+X---------------+---> Actual: 2.3.0-M-abcdef456
             \              /
              X------------X--->
             2.3-beta     2.3.0

Hence, we need to add the `--first-parent` flag to git describe, so that
it only follows the first parent of a merge and does not pull in tags
from another branch.

Fixes mixxxdj/website#248.

See the issue for details: mixxxdj/website#248
@Holzhaus
Copy link
Member Author

Holzhaus commented Jul 7, 2021

Note that this is based on the 2.3 branch and we might consider merging it into 2.3 instead of main to prevent being bitten by this when releasing 2.3.1 or something.

@uklotzde uklotzde changed the base branch from main to 2.3 July 7, 2021 23:26
Copy link
Contributor

@uklotzde uklotzde left a comment

Choose a reason for hiding this comment

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

Works locally. Please verify, confirm, and merge.

@daschuer
Copy link
Member

daschuer commented Jul 7, 2021

I can confirm that this issue is fixed. However the branch is displayed wrong (HEAD) in the github builds.
I think this should be git-describe-first-parent or the PR number or something that you can find the source of this binary later.

@daschuer
Copy link
Member

daschuer commented Jul 7, 2021

Do you like to fix it here or shall we track this as bug?

@Be-ing
Copy link
Contributor

Be-ing commented Jul 8, 2021

If this introduces a regression in the stable branch, that should be fixed in the same PR.

@Holzhaus
Copy link
Member Author

Holzhaus commented Jul 8, 2021

Is it actually a regression? We should check if this was also the case before this PR.

@daschuer
Copy link
Member

daschuer commented Jul 8, 2021

I am pretty sure that this is not a regression.

@Holzhaus
Copy link
Member Author

Holzhaus commented Jul 8, 2021

I am pretty sure that this is not a regression.

Yes, looks like this problem was present before: https://github.com/mixxxdj/mixxx/runs/3013968489#step:20:387 (build log from unrelated qml library PR)

I think the missing branch name comes from github creating a merge commit without fetching branch names. But this should be taken care of in another PR.

@daschuer
Copy link
Member

daschuer commented Jul 8, 2021

I have filed a bug: https://bugs.launchpad.net/mixxx/+bug/1934976

@daschuer
Copy link
Member

daschuer commented Jul 8, 2021

Thank you for the fix.

@daschuer daschuer merged commit b978775 into mixxxdj:2.3 Jul 8, 2021
@uklotzde uklotzde added this to the 2.3.1 milestone Jul 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

macOS / windows dev snapshot is named '2.3' while it is 'main'
4 participants