Skip to content

Replace Version with Git Hash#33

Closed
greymeister wants to merge 5 commits into
1dot13:masterfrom
greymeister:git-hash-version
Closed

Replace Version with Git Hash#33
greymeister wants to merge 5 commits into
1dot13:masterfrom
greymeister:git-hash-version

Conversation

@greymeister
Copy link
Copy Markdown
Contributor

Using a method found here I updated the VS 2017 and 2019 project files to add pre-build events to update a file rev.h to include a macro that contains 10 digits of the git hash of the working copy.

Some notes, this means that older VS projects will not be able to build and thus will need to backport the changes from 2017/2019 and that git must be available when the build runs, else the pre-build event will fail to execute.

@rftrdev
Copy link
Copy Markdown
Contributor

rftrdev commented Nov 28, 2022

It looks like this can be easily applied to the other *.vcxproj files, meaning that VS2003, 2005, and 2008 will be broken. I guess I'm okay with that?

@greymeister
Copy link
Copy Markdown
Contributor Author

It looks like this can be easily applied to the other *.vcxproj files, meaning that VS2003, 2005, and 2008 will be broken. I guess I'm okay with that?

I wouldn't have a problem making those changes but it would take some time setting up systems with those installs.

@Asdow
Copy link
Copy Markdown
Contributor

Asdow commented Dec 2, 2022

Do we even have a reason to keep those old projects around? IMO, we should start moving the code towards making use of modern c++17/20

@greymeister
Copy link
Copy Markdown
Contributor Author

@Asdow @rftrdev After #59 it looks like the GitHub workflows are populating czVersionString with a placeholder. I'm not sure what the best solution is for a non-github and github build to be consistent.

@rftrdev
Copy link
Copy Markdown
Contributor

rftrdev commented Jan 11, 2023

@Build@ and @Version@ are replaced by the build action, so could you do something similar here with the prebuild event?

@greymeister
Copy link
Copy Markdown
Contributor Author

@Build@ and @Version@ are replaced by the build action, so could you do something similar here with the prebuild event?

It looks like the GitHub build does invoke msbuild, so my previous implementation would work assuming it can invoke git as well as part of the solution config for prebuild events. If that is the case then the @Version@ replacement would be redundant, but it looks like it is doing more than just that:

          SOURCE_COMMIT_DATETIME=$(TZ=UTC0 git log -1 --date=iso-strict-local --format=%cd $GITHUB_SHA)
          SOURCE_COMMIT_DATE=$(TZ=UTC0 git log -1 --date=short-local --format=%cd $GITHUB_SHA)
          
          # GAME_VERSION is used to detect outdated save games and is the main version used for tracking
          if [[ "$GITHUB_REF_TYPE" == 'tag' ]]
          then
            # if we build for a specific tag, use that as the game version
            # examples:
            # - v1.3.3
            # - v1.3.2-rc2
            GAME_VERSION="$GITHUB_REF_NAME"
          else
            # tag the very first commit as v1.13, fixes git describe if no tags are around
            if ! git rev-list v1.13 2>/dev/null
            then
              git tag v1.13 $(git rev-list --max-parents=0 HEAD) && git push origin v1.13
            fi
          
            # uses `git describe`, which tries to find a tag in the commit hierarchy
            # example five (5) commits after v1.13:
            # - v1.13-5-7g7ffa
            GAME_VERSION="$(git describe --tags --match='v[0-9]*' $GITHUB_SHA)"
          fi
          # max 15 CHAR8
          GAME_VERSION="${GAME_VERSION:0:15}"

So I guess one solution would be "Capture GAME_VERSION env variable and if blank replace with hash" which should provide something useful for local builds and just subsume what the github workflow is doing?

@greymeister greymeister marked this pull request as draft January 11, 2023 16:44
@CptMoore
Copy link
Copy Markdown
Contributor

several issues

  1. do not invoke git on the command line as it could be missing (others using git usually download a platform independent or dependent git lib automatically and run it during building)
  2. the github workflow replaces version and build information, both are required to uniquely identify the build.
  3. pre and postbuild events usually are a bad idea, syntax usually makes it hard to cross compile on other platforms, and if it changes sources it can trigger recompiles even if nothing changes
  4. we anyway want to switch to cmake hopefully, I would wait until we are there, as it might allow a better solution

@greymeister
Copy link
Copy Markdown
Contributor Author

several issues

1. do not invoke git on the command line as it could be missing (others using git usually download a platform independent or dependent git lib automatically and run it during building)

You are doing that in your GitHub workflow yml.

2. the github workflow replaces version and build information, both are required to uniquely identify the build.

That's fine but building locally should still work and now the version info in game is useless.

3. pre and postbuild events usually are a bad idea, syntax usually makes it hard to cross compile on other platforms, and if it changes sources it can trigger recompiles even if nothing changes

4. we anyway want to switch to cmake hopefully, I would wait until we are there, as it might allow a better solution

That doesn't solve anything.

I'm withdrawing this PR but I think it's a really stupid idea to assume things in code that rely on GitHub as a build system.

@greymeister greymeister deleted the git-hash-version branch January 11, 2023 19:51
@CptMoore
Copy link
Copy Markdown
Contributor

You are doing that in your GitHub workflow yml.

Thats because git is installed by the build machine image thats defined in the workflow yml. One can't demand that git is available on the PATH on every developer machine.

That's fine but building locally should still work and now the version info in game is useless.

No not that, I wanted to emphesize that you need more than a hash. You need to specify where that hash is located, what the repo is called. Build information when built in github includes the name "github" the owner/repo and even the branch if applicable. I would try to include at least some information about the remote (git remote -v) as build information for local builds.

Also now that you mention hash, git descibe would be better anyway, as that properly shows a tag and a commit count. Its the official way from git to descibe a version the current checkout is at.

That doesn't solve anything.

I assume you mean cmake? Once cmake is used, you just search google and find a good solution and use that. So it helps a lot.
First result searching "git cmake version":

Yes it still requires the git exe but it has a fallback implemented. Point is, now its easy to find a solution that covers many cases and you only need to modify it marginally. Well once we have cmake.

I'm withdrawing this PR but I think it's a really stupid idea to assume things in code that rely on GitHub as a build system.

2 things:

  1. Its not a reliance, the code just dont know its version, unless you start commiting a version for every commit. I've seen projects that actually do it and where half the commits were just version bumps, that hides actual work though so kinda bad imo.
  2. github workflow is the only thing setting the version and build information for now, but that can still be changed, I didnt list the issues so that this doesnt get done, just that those issues should be addressed.

If 1dot13 would be a .NET project, I would have said to just add

<PackageReference Include="GitVersion.MsBuild" Version="5.6.10*">
  <PrivateAssets>All</PrivateAssets>
</PackageReference>

to your csproj file and be done with it. That would just do everything automatically: download a git library to use (instead of relying git on path), set the official assembly versions numbers based on the current git state, provide additional git related attributes in the code and metadata. But alas we are in CPP and dont have that option.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants