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

Add git revision info to player version #2774

Merged
merged 9 commits into from
May 2, 2022

Conversation

carstene1ns
Copy link
Member

Replaces #2070, fixes #87.

Works both on git checkouts and release tarballs.
A checked out git tag behaves the same like a release tarball.
make distcheck works

For our builds we can easily inject a tag (like CI, PR, AUR, OBS) with
-DPLAYER_VERSION_APPEND for cmake or --enable-append-version for autotools.

@carstene1ns carstene1ns added this to the 0.7.1 milestone Apr 26, 2022
@carstene1ns carstene1ns requested a review from Ghabry April 26, 2022 00:55
@Ghabry
Copy link
Member

Ghabry commented Apr 26, 2022

Great work!

What doesn't work is regenerating the Git-Stuff in CMake after a commit happened or a tag was added. This uses then the stale data. Though this is probably hard to fix. Not sure how to do this properly. So can be omitted for now.


Minor improvement idea (see patch): GetVersionString could be added to version.cpp/.h and then used in Player. This makes GetFullVersionString so simple that PrintVersionString can be removed. (Maybe also move GetFullVersionString to version.cpp/.h?

Also added the git-version to Window_About. The append-part does currently not fit. Can be solved later by changing the layout of that scene (moving the menu to the top and using a horizontal menu).

diff --git a/src/player.cpp b/src/player.cpp
index 56458492..2fe1a69b 100644
--- a/src/player.cpp
+++ b/src/player.cpp
@@ -690,7 +690,7 @@ Game_Config Player::ParseCommandLine(int argc, char *argv[]) {
 		}
 #endif
 		if (cp.ParseNext(arg, 0, "--version", 'v')) {
-			PrintVersion();
+			std::cout << GetFullVersionString() << "\n";
 			exit(0);
 			break;
 		}
@@ -1321,19 +1321,7 @@ std::string Player::GetEncoding() {
 }
 
 std::string Player::GetFullVersionString() {
-	std::stringstream version;
-	version << "EasyRPG Player " << Version::STRING;
-	if (std::strlen(Version::GIT) > 0) {
-		version << " " << Version::GIT;
-	}
-	if (std::strlen(Version::APPEND) > 0) {
-		version << " " << Version::APPEND;
-	}
-	return version.str();
-}
-
-void Player::PrintVersion() {
-	std::cout << GetFullVersionString() << std::endl;
+	return std::string(GAME_TITLE) + " " + Version::GetVersionString();
 }
 
 void Player::PrintUsage() {
diff --git a/src/player.h b/src/player.h
index da4b43f8..4010d820 100644
--- a/src/player.h
+++ b/src/player.h
@@ -267,9 +267,6 @@ namespace Player {
 	/** @return full version string */
 	std::string GetFullVersionString();
 
-	/** Output program version on stdout */
-	void PrintVersion();
-
 	/** Output program usage information on stdout */
 	void PrintUsage();
 
diff --git a/src/version.cpp b/src/version.cpp
index 2b16a27f..696cef0a 100644
--- a/src/version.cpp
+++ b/src/version.cpp
@@ -47,4 +47,14 @@ namespace Version {
 	const char GIT[] = EP_VERSION_GIT;
 	const char APPEND[] = EP_VERSION_APPEND;
 
+	std::string GetVersionString(bool with_git, bool with_append) {
+		std::string ver = Version::STRING;
+		if (with_git && std::strlen(GIT) > 0) {
+			ver += std::string(" ") + GIT;
+		}
+		if (with_append && std::strlen(APPEND) > 0) {
+			ver += std::string(" ") + APPEND;
+		}
+		return ver;
+	}
 }
diff --git a/src/version.h b/src/version.h
index 063995a6..7b42a2d6 100644
--- a/src/version.h
+++ b/src/version.h
@@ -18,6 +18,9 @@
 #ifndef EP_VERSION_H
 #define EP_VERSION_H
 
+#include <cstring>
+#include <string>
+
 /**
  * Version of Player.
  */
@@ -30,6 +33,13 @@ namespace Version {
 	extern const char GIT[];
 	extern const char APPEND[];
 
+	/**
+	 * Generates a version string.
+	 *
+	 * @param with_git Append git information (if available)
+	 * @param with_append Append additional information (if available, by default the build date)
+	 */
+	std::string GetVersionString(bool with_git = true, bool with_append = true);
 }
 
 /**
diff --git a/src/window_about.cpp b/src/window_about.cpp
index 9eb0bf3d..d3191a72 100644
--- a/src/window_about.cpp
+++ b/src/window_about.cpp
@@ -36,7 +36,7 @@ void Window_About::Refresh() {
 		"interpreter.",
 		"Licensed under the GPLv3",
 		"",
-		"Version: " + std::string(Version::STRING),
+		"Version: " + Version::GetVersionString(true, false),
 		"Website: easyrpg.org",
 		"",
 		"Contact us:",
-- 
2.35.1

0001-Bla.zip

carstene1ns and others added 5 commits April 27, 2022 02:10
Regenerate once changed
Use version namespace
Use a convenience lib for autotools to not pollute definitions
autotools regenerates only the version library, cmake the whole project
@carstene1ns carstene1ns force-pushed the feature/git-revision branch from a56aec0 to cdfaff0 Compare April 27, 2022 00:27
@carstene1ns
Copy link
Member Author

Do not merge yet.

- simplify version setting
- use macro from autoconf archive
Warn when no build version is defined - to convince Makefile users ;)
@@ -1159,9 +1174,11 @@ set(DOXYGEN_STATUS "Unavailable")
if(DOXYGEN_FOUND)
set(DOXYGEN_STATUS "Available (target \"doc\")")
# fake autotools variables
set(PACKAGE_VERSION ${PLAYER_VERSION})
Copy link
Member Author

@carstene1ns carstene1ns May 1, 2022

Choose a reason for hiding this comment

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

This has been successively broken in 2016 and 2018 m(

doxygen used the version of the expat package here (because of config file providing PACKAGE_VERSION)

Always use shortest version identifier possible
Generate version and date for most platforms
Apply git versioning for homebrew
Update ignores and release-helper script
(stop treating Windows as first-class :P)
@carstene1ns carstene1ns force-pushed the feature/git-revision branch from 330abe0 to 7796bcb Compare May 1, 2022 17:02
@carstene1ns
Copy link
Member Author

This is ready now.

@carstene1ns carstene1ns merged commit 8ef2455 into EasyRPG:master May 2, 2022
@carstene1ns carstene1ns deleted the feature/git-revision branch May 3, 2022 00:10
@Ghabry
Copy link
Member

Ghabry commented May 3, 2022

Not a full review due to lack of time.
Wow you really pushed this forward. Even updates all the version information in all of the resource files etc. Great job!

Also the oldest issue closed 🎉

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

Successfully merging this pull request may close these issues.

Use "git describe" to display a nice rev-number
3 participants