CMake: Keep git info up-to-date without reconfiguration #3841

Holzhaus · 2021-05-07T11:54:57Z

Based on #3831.

Hopefully replaces #3837 #3822 #3259 #3832 (EDIT: Also added setting the patched flag from git describe from #3817 in 09e5ee4)

This should avoid reconfiguration due to git state changes reliably:

All git info stuff removed from CMakeLists.txt
Add build time, this always runs a script that configures a file with the git version info as a build job. That file is then included by the source code. This means that the build system can figure out if it needs to rebuild/relink and we don't have any unnecessary rebuilds/reconfigurations.
For CPack, the git information is retrieved at CPack time, not at configuration time. This also ensure that the git info is always up to date.

daschuer · 2021-05-07T15:04:47Z

I am gad we are getting close, thank you. What a work for this tiny flag, not used for DJ-ing :-/ .. but at least interesting to lean cmake ...

daschuer

The big solution ... Thank you.
I have left some comments.

CMakeLists.txt

daschuer · 2021-05-07T15:13:53Z

CMakeLists.txt

@@ -1705,9 +1648,12 @@ if(WIN32)
    "${CMAKE_CURRENT_BINARY_DIR}/src/mixxx.rc.include"
    @ONLY
  )
+  add_dependencies(mixxx mixxx-gitinfo)


We should do this for all targets and remove the git dependency from mixxx-lib.

Did you consider to configure a cpp file instead of a header, like did in the first attempt? This will speed up recompiling even more?

This will speed up recompiling even more?

How so?

I can take care after merge.

CMakeLists.txt

daschuer · 2021-05-07T15:22:41Z

packaging/CPackConfig.cmake

+endif()
+set(CPACK_PACKAGE_FILE_NAME "mixxx-${PACKAGE_VERSION}")
+set(CPACK_SOURCE_PACKAGE_FILE_NAME "${CPACK_PACKAGE_FILE_NAME}-source")
+set(CPACK_DEBIAN_GIT_DESCRIBE "${GIT_DESCRIBE}")


The code patching the source tree with CPACK_DEBIAN_GIT_DESCRIBE in CPackDebUploadPPA.cmake does no longer work.

I hopefully fixed it now. But I can't test (no debian system).

daschuer · 2021-05-07T15:23:59Z

src/gitinfo.h.in

+#define GIT_COMMIT_DATE "@GIT_COMMIT_DATE@"
+#cmakedefine GIT_COMMIT_YEAR @GIT_COMMIT_YEAR@
+#cmakedefine GIT_COMMIT_COUNT @GIT_COMMIT_COUNT@
+#cmakedefine GIT_DIRTY


Ideally this should become a cpp file

No, this file is also used from mixxx.rc.include.template which isn't C/CPP. And I think configuring header files is much cleaner than cpp files. Performance wise I doubt this makes a difference.

src/buildinfo.h.in

daschuer · 2021-05-07T15:38:23Z

Uhhh bad ....

This fails with ninja. Ninja does not recalculate files during build. All files that changes during the build must be added with DEPENDS directives. Some of these do not work for VisualStudio.

Since the dirty flag is only relevant for home builds, this important.

Conclusion:
Let's remove the dirty flag entirely and carry one the important topics, like crash when changing effects on windows or the auto DJ stop issue.

What do you think?

I have given up to make ninja / msbuild and make work in #3837 it is probably not possible with build system independent code.

Holzhaus · 2021-05-07T16:01:14Z

Uhhh bad ....

This fails with ninja. Ninja does not recalculate files during build. All files that changes during the build must be added with DEPENDS directives. Some of these do not work for VisualStudio.

I cannot reproduce this, it works fine here:

$ ninja
[2/6] Update git version information in gitinfo.h
Git describe: 2.3-beta-3890-g836d68bcce
Git worktree modified: no
Git branch: cmake-gitinfo-without-reconfig
Git commit date: 2021-05-07T17:48:21+02:00
Git commit year: 2021
Git commit count: 8173
$ printf "\n" >> ../res/schema.xml
$ ninja
[2/10] Update git version information in gitinfo.h
Git describe: 2.3-beta-3890-g836d68bcce-modified
Git worktree modified: yes
Git branch: cmake-gitinfo-without-reconfig
Git commit date: 2021-05-07T17:48:21+02:00
Git commit year: 2021
Git commit count: 8173
[10/10] Linking CXX executable mixxx-test

Holzhaus · 2021-05-07T19:08:11Z

@daschuer I also want to get this over with, but It works fine for me. Can you re-test or elaborate what exactly doesn't work?

daschuer · 2021-05-07T21:56:57Z

Ninja does not rebuild, mixxx when the git work there becomes dirty due to a file that not triggers a rebuild anyway.

Holzhaus · 2021-05-07T22:43:00Z

Ninja does not rebuild, mixxx when the git work there becomes dirty due to a file that not triggers a rebuild anyway.

Cannot reproduce this, works for me:

$ ninja
[2/6] Update git version information in gitinfo.h
Git describe: 2.3-beta-3897-ga01f34ec79
Git worktree modified: no
Git branch: cmake-gitinfo-without-reconfig
Git commit date: 2021-05-07T18:15:51+02:00
Git commit year: 2021
Git commit count: 8176
$ printf "\n" >> ../res/controllers/Roland_DJ-505-scripts.js
$ ninja
[2/6] Update git version information in gitinfo.h
Git describe: 2.3-beta-3897-ga01f34ec79-modified
Git worktree modified: yes
Git branch: cmake-gitinfo-without-reconfig
Git commit date: 2021-05-07T18:15:51+02:00
Git commit year: 2021
Git commit count: 8176
[6/6] Linking CXX executable mixxx
$ ninja --version
1.10.2

In any case, I think this PR improves the current situation.

daschuer · 2021-05-08T08:32:46Z

I can no longer confirm the ninja build issues.

Holzhaus · 2021-05-08T08:56:51Z

Maybe the worktree was already dirty. In that case, if you change something else, too, it stays dirty and won't recompile unnecessarily.

daschuer

Thank you for this effort. It works now.

Holzhaus added the build label May 7, 2021

Holzhaus added this to the 2.3.0 milestone May 7, 2021

Holzhaus requested a review from daschuer May 7, 2021 11:54

Holzhaus added this to In progress in 2.3 release via automation May 7, 2021

Holzhaus changed the title ~~Cmake gitinfo without reconfig~~ CMake: Keep git info up-to-date without reconfiguration May 7, 2021

Holzhaus mentioned this pull request May 7, 2021

CMake: Reconfigure to update git-dirty version when source files are changed #3832

Closed

Holzhaus force-pushed the cmake-gitinfo-without-reconfig branch 2 times, most recently from 09e5ee4 to 8c78569 Compare May 7, 2021 12:54

Holzhaus moved this from In progress to Needs Review in 2.3 release May 7, 2021

Holzhaus mentioned this pull request May 7, 2021

dirty-flag: avoid always reconfigure #3837

Closed

Holzhaus force-pushed the cmake-gitinfo-without-reconfig branch 2 times, most recently from 76641c5 to d192455 Compare May 7, 2021 14:56

daschuer requested changes May 7, 2021

View reviewed changes

Holzhaus force-pushed the cmake-gitinfo-without-reconfig branch from d192455 to 836d68b Compare May 7, 2021 15:50

Holzhaus added 3 commits May 7, 2021 18:02

CMake: Split version.h into version.h and versiondetails.h

ceccfb9

defs_urls.h: Use MIXXX_VERSION define instead of hardcoding version

c3a52a5

CmdlineArgs: Move hardcoded manual URL to defs_urls.h

0eb2b0e

Holzhaus force-pushed the cmake-gitinfo-without-reconfig branch from 836d68b to dfea4f4 Compare May 7, 2021 16:10

CMake: Fetch latest Git information on build without reconfig

a01f34e

Holzhaus force-pushed the cmake-gitinfo-without-reconfig branch from dfea4f4 to a01f34e Compare May 7, 2021 16:18

Holzhaus mentioned this pull request May 7, 2021

Use CMAKE_PROJECT_VERSION for URLs #3831

Merged

daschuer approved these changes May 8, 2021

View reviewed changes

daschuer merged commit 896b64a into mixxxdj:2.3 May 8, 2021

2.3 release automation moved this from Needs Review to Done May 8, 2021

This was referenced May 8, 2021

use git describe for version #3259

Closed

Set patched flag and build revison from git describe #3817

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CMake: Keep git info up-to-date without reconfiguration #3841

CMake: Keep git info up-to-date without reconfiguration #3841

Holzhaus commented May 7, 2021 •

edited

daschuer commented May 7, 2021

daschuer left a comment

daschuer May 7, 2021

Holzhaus May 7, 2021

daschuer May 8, 2021

daschuer May 7, 2021

Holzhaus May 7, 2021

daschuer May 7, 2021

Holzhaus May 7, 2021

daschuer commented May 7, 2021

Holzhaus commented May 7, 2021

Holzhaus commented May 7, 2021

daschuer commented May 7, 2021 •

edited

Holzhaus commented May 7, 2021

daschuer commented May 8, 2021

Holzhaus commented May 8, 2021

daschuer left a comment

CMake: Keep git info up-to-date without reconfiguration #3841

CMake: Keep git info up-to-date without reconfiguration #3841

Conversation

Holzhaus commented May 7, 2021 • edited

daschuer commented May 7, 2021

daschuer left a comment

Choose a reason for hiding this comment

daschuer May 7, 2021

Choose a reason for hiding this comment

Holzhaus May 7, 2021

Choose a reason for hiding this comment

daschuer May 8, 2021

Choose a reason for hiding this comment

daschuer May 7, 2021

Choose a reason for hiding this comment

Holzhaus May 7, 2021

Choose a reason for hiding this comment

daschuer May 7, 2021

Choose a reason for hiding this comment

Holzhaus May 7, 2021

Choose a reason for hiding this comment

daschuer commented May 7, 2021

Holzhaus commented May 7, 2021

Holzhaus commented May 7, 2021

daschuer commented May 7, 2021 • edited

Holzhaus commented May 7, 2021

daschuer commented May 8, 2021

Holzhaus commented May 8, 2021

daschuer left a comment

Choose a reason for hiding this comment

Holzhaus commented May 7, 2021 •

edited

daschuer commented May 7, 2021 •

edited