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

Git info store object #3848

Merged
merged 12 commits into from
May 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 19 additions & 5 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1124,7 +1124,7 @@ endif()
# The mixxx executable
add_executable(mixxx WIN32 src/main.cpp)
set_target_properties(mixxx-lib PROPERTIES CXX_CLANG_TIDY "${CLANG_TIDY}")
target_link_libraries(mixxx PRIVATE mixxx-lib)
target_link_libraries(mixxx PRIVATE mixxx-lib mixxx-gitinfostore)

#
# Installation and Packaging
Expand Down Expand Up @@ -1500,7 +1500,7 @@ add_executable(mixxx-test
src/test/wwidgetstack_test.cpp
)
set_target_properties(mixxx-test PROPERTIES AUTOMOC ON)
target_link_libraries(mixxx-test PRIVATE mixxx-lib gtest gmock)
target_link_libraries(mixxx-test PRIVATE mixxx-lib mixxx-gitinfostore gtest gmock)

#
# Benchmark tests
Expand Down Expand Up @@ -1607,16 +1607,27 @@ get_target_property(MIXXX_BUILD_FLAGS mixxx-lib COMPILE_OPTIONS)
# uses CMAKE_PROJECT_VERSION MIXXX_VERSION_PRERELEASE MIXXX_BUILD_FLAGS
configure_file(src/version.h.in src/version.h @ONLY)

if(GIT_COMMIT_DATE AND NOT GIT_COMMIT_DATE MATCHES "^[0-9]*-[0-9]*-[0-9]*T[0-9]*\\:[0-9]*\\:[0-9]*[+-][0-9]*\\:[0-9]*$")
message(FATAL_ERROR "GIT_COMMIT_DATE requires strict ISO 8601 format %Y-%m-%dT%H:%M:%SZ")
endif()

add_custom_target(mixxx-gitinfo
COMMAND ${CMAKE_COMMAND}
-DGIT_DESCRIBE="${GIT_DESCRIBE}"
-DGIT_COMMIT_DATE="${GIT_COMMIT_DATE}"
Comment on lines +1616 to +1617
Copy link
Member

Choose a reason for hiding this comment

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

What about CPackConfig? I think you need to copy these variables into the CPack domain as well, and set them before including the GitInfo module.

Copy link
Member

Choose a reason for hiding this comment

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

@daschuer ping.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh yes, we should allow to use the override values in cpack as well.

-DINPUT_FILE="${CMAKE_CURRENT_SOURCE_DIR}/src/gitinfo.h.in"
-DOUTPUT_FILE="${CMAKE_CURRENT_BINARY_DIR}/src/gitinfo.h"
-P "${CMAKE_CURRENT_SOURCE_DIR}/cmake/scripts/gitinfo.cmake"
COMMENT "Update git version information in gitinfo.h"
BYPRODUCTS "${CMAKE_CURRENT_BINARY_DIR}/src/gitinfo.h"
WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}"
)
add_dependencies(mixxx-lib mixxx-gitinfo)

add_library(mixxx-gitinfostore STATIC EXCLUDE_FROM_ALL
Copy link
Member

Choose a reason for hiding this comment

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

What's the reason for making this a separate library? Why not add it to mixxx-lib instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

To improve build time. Re-linking it to mixxx-lib takes long, even if just the git info changes.
From this point of view it is better to have more lbs than less. This will also reduce the memory requirements on Windows.

src/util/gitinfostore.cpp
)
target_include_directories(mixxx-gitinfostore PUBLIC src ${CMAKE_BINARY_DIR}/src)
add_dependencies(mixxx-gitinfostore mixxx-gitinfo)

# Windows-only resource file
if(WIN32)
Expand All @@ -1639,8 +1650,8 @@ if(WIN32)

# uses MIXXX_YEAR MIXXX_FILEVERSION MIXXX_PRODUCTVERSION MIXXX_VERSION MIXXX_DEBUG MIXXX_PRERELEASE
configure_file(
"${CMAKE_CURRENT_SOURCE_DIR}/src/mixxx.rc.include.template"
"${CMAKE_CURRENT_BINARY_DIR}/src/mixxx.rc.include"
"src/mixxx.rc.include.in"
"src/mixxx.rc.include"
@ONLY
)
add_dependencies(mixxx mixxx-gitinfo)
Expand Down Expand Up @@ -2646,6 +2657,9 @@ set(CPACK_RESOURCE_FILE_README "${CMAKE_CURRENT_SOURCE_DIR}/README")
set(CPACK_STRIP_FILES ON)
set(CPACK_CREATE_DESKTOP_LINKS "mixxx")
set(CPACK_MIXXX_VERSION "${MIXXX_VERSION}")
# Save GIT values just in case they have been set manual via cmake
set(CPACK_GIT_DESCRIBE "${GIT_DESCRIBE}")
set(CPACK_GIT_COMMIT_DATE ${GIT_COMMIT_DATE})

# Detailed version information, git info and package file name are set from
# CPackConfig.cmake, not here.
Expand Down
117 changes: 65 additions & 52 deletions cmake/modules/GitInfo.cmake
Original file line number Diff line number Diff line change
@@ -1,65 +1,78 @@
# Get the current commit ref
execute_process(
COMMAND git describe --tags --always --dirty=-modified
WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}"
OUTPUT_VARIABLE GIT_DESCRIBE
OUTPUT_STRIP_TRAILING_WHITESPACE
ERROR_QUIET
)
if(NOT GIT_DESCRIBE)
message(NOTICE "Git describe: unknown")
else()
message(NOTICE "Git describe: ${GIT_DESCRIBE}")
string(REGEX MATCH "-modified$" GIT_DIRTY "${GIT_DESCRIBE}")
if (GIT_DIRTY)
message("Git worktree modified: yes")
set(GIT_DIRTY 1)
execute_process(
COMMAND git describe --tags --always --dirty=-modified
WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}"
OUTPUT_VARIABLE GIT_DESCRIBE
OUTPUT_STRIP_TRAILING_WHITESPACE
ERROR_QUIET
)
if(NOT GIT_DESCRIBE)
message(NOTICE "Git describe is unknown, you may set it via GIT_DESCRIBE")
else()
message(NOTICE "Git worktree modified: no")
set(GIT_DIRTY 0)
endif()
endif()
message(NOTICE "Git describe: ${GIT_DESCRIBE}")
string(REGEX MATCH "-modified$" GIT_DIRTY "${GIT_DESCRIBE}")
if (GIT_DIRTY)
message("Git worktree modified: yes")
set(GIT_DIRTY 1)
else()
message(NOTICE "Git worktree modified: no")
set(GIT_DIRTY 0)
endif()

# Get the current working branch
execute_process(
COMMAND git rev-parse --abbrev-ref HEAD
WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}"
OUTPUT_VARIABLE GIT_BRANCH
OUTPUT_STRIP_TRAILING_WHITESPACE
ERROR_QUIET
)
if(NOT GIT_BRANCH)
message(NOTICE "Git branch: unknown")
else()
message(NOTICE "Git branch: ${GIT_BRANCH}")
endif()

# Get the current working branch
execute_process(
COMMAND git rev-parse --abbrev-ref HEAD
WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}"
OUTPUT_VARIABLE GIT_BRANCH
OUTPUT_STRIP_TRAILING_WHITESPACE
ERROR_QUIET
)
if(NOT GIT_BRANCH)
message(NOTICE "Git branch: unknown")
# Get the number of commits on the working branch
execute_process(
COMMAND git rev-list --count --first-parent HEAD
WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}"
OUTPUT_VARIABLE GIT_COMMIT_COUNT
OUTPUT_STRIP_TRAILING_WHITESPACE
ERROR_QUIET
)
if(NOT GIT_COMMIT_COUNT)
message(NOTICE "Git commit count: unknown")
else()
message(NOTICE "Git commit count: ${GIT_COMMIT_COUNT}")
endif()
endif()
else()
message(NOTICE "Git branch: ${GIT_BRANCH}")
message(NOTICE "Git describe: ${GIT_DESCRIBE}")
if(NOT GIT_COMMIT_DATE)
message(NOTICE "Git commit date unknown, using current time for GIT_COMMIT_DATE")
# use current date in case of tar ball builds
string(TIMESTAMP GIT_COMMIT_DATE "%Y-%m-%dT%H:%M:%SZ" UTC)
endif()
endif()

# Get the current commit date
execute_process(
COMMAND git show --quiet --format=%cI --date=short
WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}"
OUTPUT_VARIABLE GIT_COMMIT_DATE
OUTPUT_STRIP_TRAILING_WHITESPACE
ERROR_QUIET
)
if(NOT GIT_COMMIT_DATE)
message(NOTICE "Git commit date: unknown")
else()
message(NOTICE "Git commit date: ${GIT_COMMIT_DATE}")
string(REGEX MATCH "^[0-9][0-9][0-9][0-9]" GIT_COMMIT_YEAR "${GIT_COMMIT_DATE}")
message(NOTICE "Git commit year: ${GIT_COMMIT_YEAR}")
execute_process(
COMMAND git show --quiet --format=%cI --date=short
WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}"
OUTPUT_VARIABLE GIT_COMMIT_DATE
OUTPUT_STRIP_TRAILING_WHITESPACE
ERROR_QUIET
)
endif()

# Get the number of commits on the working branch
execute_process(
COMMAND git rev-list --count --first-parent HEAD
WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}"
OUTPUT_VARIABLE GIT_COMMIT_COUNT
OUTPUT_STRIP_TRAILING_WHITESPACE
ERROR_QUIET
)
if(NOT GIT_COMMIT_COUNT)
message(NOTICE "Git commit count: unknown")
if(NOT GIT_COMMIT_DATE)
message(NOTICE "Git commit date unknown, using current time for GIT_COMMIT_DATE")
# use current date in case of tar ball builds
string(TIMESTAMP GIT_COMMIT_DATE "%Y-%m-%dT%H:%M:%SZ" UTC)
else()
message(NOTICE "Git commit count: ${GIT_COMMIT_COUNT}")
message(NOTICE "Git commit date: ${GIT_COMMIT_DATE}")
endif()
string(REGEX MATCH "^[0-9][0-9][0-9][0-9]" GIT_COMMIT_YEAR "${GIT_COMMIT_DATE}")
8 changes: 2 additions & 6 deletions packaging/CPackConfig.cmake
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# unlike CMakeLists.txt this file is include at cpack time, once per generator after CPack has set CPACK_GENERATOR
# to the actual generator being used. It allows per-generator setting of CPACK_* variables at cpack time.
set(GIT_DESCRIBE "${CPACK_GIT_DESCRIBE}")
set(GIT_COMMIT_DATE ${CPACK_GIT_COMMIT_DATE})
set(CMAKE_CURRENT_SOURCE_DIR "${CPACK_SOURCE_DIR}")
list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake/modules")
include(GitInfo)
Expand All @@ -11,12 +13,6 @@ else()
endif()
set(CPACK_PACKAGE_FILE_NAME "mixxx-${PACKAGE_VERSION}")
set(CPACK_SOURCE_PACKAGE_FILE_NAME "${CPACK_PACKAGE_FILE_NAME}-source")
set(CPACK_DEBIAN_GIT_BRANCH ${GIT_BRANCH})
set(CPACK_DEBIAN_GIT_COMMIT_COUNT ${GIT_COMMIT_COUNT})
set(CPACK_DEBIAN_GIT_COMMIT_DATE ${GIT_COMMIT_DATE})
set(CPACK_DEBIAN_GIT_COMMIT_YEAR ${GIT_COMMIT_YEAR})
set(CPACK_DEBIAN_GIT_DESCRIBE "${GIT_DESCRIBE}")
set(CPACK_DEBIAN_GIT_DIRTY ${GIT_DIRTY})

# The upstream version must not contain hyphen
# . for nomal versioning + for advance and ~ for decline the version
Expand Down
17 changes: 2 additions & 15 deletions packaging/CPackDebUploadPPA.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -55,21 +55,8 @@ execute_process(
WORKING_DIRECTORY ${CPACK_TOPLEVEL_DIRECTORY}
)

set(CMAKE_PROJECT_VERSION_MAJOR "@CMAKE_PROJECT_VERSION_MAJOR@")
set(CMAKE_PROJECT_VERSION_MINOR "@CMAKE_PROJECT_VERSION_MINOR@")
set(CMAKE_PROJECT_VERSION_PATCH "@CMAKE_PROJECT_VERSION_PATCH@")
set(MIXXX_VERSION_PRERELEASE "@MIXXX_VERSION_PRERELEASE@")
set(MIXXX_BUILD_FLAGS "@MIXXX_BUILD_FLAGS@")
configure_file(
${CPACK_TOPLEVEL_DIRECTORY}/${CPACK_PACKAGE_FILE_NAME}/src/version.h.in
${CPACK_TOPLEVEL_DIRECTORY}/${CPACK_PACKAGE_FILE_NAME}/src/version.h.in
@ONLY)
set(GIT_BRANCH ${CPACK_DEBIAN_GIT_BRANCH})
set(GIT_DESCRIBE ${CPACK_DEBIAN_GIT_DESCRIBE})
set(GIT_COMMIT_DATE ${CPACK_DEBIAN_GIT_COMMIT_DATE})
set(GIT_COMMIT_YEAR ${CPACK_DEBIAN_GIT_COMMIT_YEAR})
set(GIT_COMMIT_COUNT ${CPACK_DEBIAN_GIT_COMMIT_COUNT})
set(GIT_DIRTY ${CPACK_DEBIAN_GIT_DIRTY})
# Save Git info from original working tree to allow building without git
# uses GIT_BRANCH GIT_DESCRIBE GIT_COMMIT_DATE GIT_COMMIT_YEAR GIT_COMMIT_COUNT GIT_DIRTY
configure_file(
${CPACK_TOPLEVEL_DIRECTORY}/${CPACK_PACKAGE_FILE_NAME}/src/gitinfo.h.in
${CPACK_TOPLEVEL_DIRECTORY}/${CPACK_PACKAGE_FILE_NAME}/src/gitinfo.h.in
Expand Down
Empty file removed src/buildinfo.h.in
Empty file.
5 changes: 4 additions & 1 deletion src/dialog/dlgaboutdlg.ui
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@
<item row="1" column="0">
<widget class="QLabel" name="label_2">
<property name="text">
<string>Git Commit Date:</string>
<string>Date:</string>
</property>
</widget>
</item>
Expand Down Expand Up @@ -136,6 +136,9 @@
<property name="text">
<string>Unknown</string>
</property>
<property name="textInteractionFlags">
<set>Qt::TextSelectableByKeyboard|Qt::TextSelectableByMouse</set>
</property>
</widget>
</item>
</layout>
Expand Down
4 changes: 2 additions & 2 deletions src/gitinfo.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
// files that include this will be recompile very often. Therefore, this should
// only be included from versionstore.cpp.

#ifndef VERSION_STORE
#error "Include only from versionstore.cpp and mixxx.rc.include to avoid unnecessary recompile on every commit."
#ifndef GIT_INFO
#error "Include only from gitinfostore.cpp and mixxx.rc.include to avoid unnecessary recompile on every commit."
#endif

#define GIT_BRANCH "@GIT_BRANCH@"
Expand Down
2 changes: 1 addition & 1 deletion src/mixxx.rc.include.template → src/mixxx.rc.include.in
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#define VERSION_STORE
#define GIT_INFO
#include "gitinfo.h"

#ifdef GIT_COMMIT_YEAR
Expand Down
37 changes: 37 additions & 0 deletions src/util/gitinfostore.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
#include "util/gitinfostore.h"

#define GIT_INFO
#include "gitinfo.h"

//static
const char* GitInfoStore::branch() {
return GIT_BRANCH;
};

//static
const char* GitInfoStore::describe() {
return GIT_DESCRIBE;
};

//static
const char* GitInfoStore::date() {
return GIT_COMMIT_DATE;
};

//static
int GitInfoStore::commitCount() {
#ifdef GIT_COMMIT_COUNT
return GIT_COMMIT_COUNT;
#else
return 0;
Copy link
Member

Choose a reason for hiding this comment

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

-1 maybe? or use std::optional?

Copy link
Member Author

Choose a reason for hiding this comment

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

0 fits, because we have 0 commits if we are off the git worktree

#endif
};

//static
bool GitInfoStore::dirty() {
#ifdef GIT_DIRTY
return true;
#else
return false;
#endif
};
10 changes: 10 additions & 0 deletions src/util/gitinfostore.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#pragma once

class GitInfoStore {
public:
static const char* branch();
static const char* describe();
static const char* date();
static int commitCount();
static bool dirty();
};
18 changes: 7 additions & 11 deletions src/util/versionstore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,15 @@
#include <taglib/taglib.h>
#include <vorbis/codec.h>

#include "util/gitinfostore.h"
#include "version.h"
#define VERSION_STORE
#include "gitinfo.h"

namespace {

const QVersionNumber kMixxxVersionNumber = QVersionNumber(
MIXXX_VERSION_MAJOR, MIXXX_VERSION_MINOR, MIXXX_VERSION_PATCH);
const QString kMixxxVersionSuffix = QString(MIXXX_VERSION_SUFFIX);
const QString kMixxx = QStringLiteral("Mixxx");
const QString kGitBranch = QString(GIT_BRANCH);
const QString kGitDescribe = QString(GIT_DESCRIBE);
const QDateTime kGitCommitDate = QDateTime::fromString(GIT_COMMIT_DATE, Qt::ISODate);
const QString kBuildFlags = QString(MIXXX_BUILD_FLAGS);

} // namespace
Expand All @@ -65,7 +61,7 @@ QString VersionStore::versionSuffix() {
}

QDateTime VersionStore::date() {
return kGitCommitDate;
return QDateTime::fromString(GitInfoStore::date(), Qt::ISODate);
}

// static
Expand Down Expand Up @@ -114,24 +110,24 @@ QString VersionStore::platform() {

// static
QString VersionStore::gitBranch() {
return kGitBranch;
return GitInfoStore::branch();
}

// static
QString VersionStore::gitDescribe() {
return kGitDescribe;
return GitInfoStore::describe();
}

// static
QString VersionStore::gitVersion() {
QString gitVersion = VersionStore::gitDescribe();
QString gitVersion = GitInfoStore::describe();
if (gitVersion.isEmpty()) {
gitVersion = QStringLiteral("unknown");
}

QString gitBranch = VersionStore::gitBranch();
if (!gitBranch.isEmpty()) {
gitVersion.append(QStringLiteral(" (") + gitBranch + QStringLiteral(" branch)"));
if (!gitBranch.isEmpty() && !gitVersion.startsWith(gitBranch)) {
gitVersion.append(QStringLiteral(" (") + gitBranch + QChar(')'));
Copy link
Member

Choose a reason for hiding this comment

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

I'm okay with this change to make the about window a bit less wide.

Copy link
Member Author

Choose a reason for hiding this comment

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

we can even remove the branch name if it matches the version

Copy link
Member

Choose a reason for hiding this comment

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

We may, but I don't think it's necessary. In any case this should happen in the cpp code, not in the CMake code.

Copy link
Member

Choose a reason for hiding this comment

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

ah you already did.

}

return gitVersion;
Expand Down