Fix incremental updating of build version information

[Zopolis4]

Previously, on CMake builds, version information would not be updated on subsequent rebuilds without a full clearing of the build dir. Now, version information will be updated whenever the git revision changes.

How it works:

On the first build, DOLPHIN_WC_REVISION_CACHE will not have been set thus will not be equal to DOLPHIN_WC_REVISION. The version information will be generated, and then DOLPHIN_WC_REVISION_CACHE will be set.

On subsequent builds, DOLPHIN_WC_REVISION_CACHE is set to the revision of the previous build. If it differs from the current revision (DOLPHIN_WC_REVISION), version information will be re-generated.

[phire]

I'm confused, what was causing it to fail before?

Looks like if dolphin_git_head_symbolic_filename or dolphin_git_head_filename changed, it would re-run the whole configure, which should re-generate the variables, right?
This PR changes nothing in that regard, it just wraps a cache variable around it and actually executes less code on most re-executions. I must be missing something.

Slight tangent, but is configure time really the best place to do this? Seems like it might be better to create an add_custom_command script that runs at build time. Make it depends on those git head files so the the build system (make or ninja) will automatically run it at the correct time.

[Zopolis4]

It would re-run the configure, yes, but the configure just takes the pre-existing variables. The variables are only set in this CMake file, and thus would only be called once. That's what it looks like to me, anyway.

[Zopolis4]

I looked into add_custom_command, but I didn't think it was worth it.

[phire]

It's unwise to make a change when we aren't sure why it was previously broken, and why this change "fixes it".

Hell, I've never noticed this bug, so does it only happen under some configurations? Is it some form of undefined behaviour in cmake? If so, this change might make it worse on other configurations.

I suspect it might actually be the set(... CACHE ... FORCE) that is fixing it, but I don't know cmake well enough to know.

I looked into add_custom_command, but I didn't think it was worth it.

The advantage of add_custom_command, is that it's easier to understand and reason about (at least for me, maybe someone else knows what's going wrong here)

[Zopolis4]

dolphin_git_head_filename will almost never change. It will always be .git/HEAD on any typical git configuration. Run git rev-parse --git-path HEAD if you want to check.

It's a very similar situation with dolphin_git_head_symbolic_filename. dolphin_git_head_symbolic will be the same (i.e. refs/heads/master) unless you switch a branch. dolphin_git_head_symbolic_filename will be very similar (e.g. .git/refs/heads/nlob. If you want to test, run git rev-parse --symbolic-full-name HEAD and then run git rev-parse --git-path using the output of the previous command as the argument.

Neither of these variables change with the revision, whereas the new variables set here do.

[Zopolis4]

In regards to the actual bug, not sure. You'd have to ask AdmiralCurtiss, as they were the one encountering the issue.

[AdmiralCurtiss]

I have never investigated why this happens either, but when using Visual Studio's built-in CMake support (the Open Folder thing) it generates the scmrev.h once and then never again -- my current build's header is stuck on a branch I worked on months ago. This may also result from a combination of using both CMake and the VS project files? Unsure.

I asked Jos on IRC and they said it never updates for them either, so I assumed it to be a general issue with the CMake files.

I do agree that doing it at build time rather than at configure time makes more sense -- in particular because it's not even guaranteed you'll even do a reconfigure after switching commit/branch. The way I've done this in the past on another project is by using add_custom_target() that calls ${CMAKE_COMMAND} on a separate CMake script and specifies a BYPRODUCT of the actual header file. The program consuming that header would then depend on that command via add_dependencies(). The called CMake script would generate a header file in a temporary location, and then copy it to its 'real' location only if the generated result was different to the currently existing header (if any). I do not know whether that's the best way, but I know it works.

[Zopolis4]

Weird-- I failed to encounter this issue on either master or this pr. Admittedly, I was using Linux, and was calling make to test rather than reconfiguring. Both still worked for me.

[JosJuice]

For the record, I was also using Visual Studio's built-in CMake support.

[AdmiralCurtiss]

So I debugged this now and, this PR does not fix the issue -- because the code actually worked fine as-is. Turns out the issue is elsewhere. My guess here is correct:

This may also result from a combination of using both CMake and the VS project files?

The VS project files generate the scmrev.h file inside the source tree. This is not cleaned up when switching to CMake, so you now have two scmrev.h files in the build tree, and by include directory order it prefers the one in the source tree rather than the one in the build directory.

I suspect a add_custom_command()/add_custom_target()-based approach might still be preferable, but I guess it's okay as-is. The CMake script probably should check for and remove the scmrev.h in the source tree if it exists though so all Windows devs don't run into this once we drop VS.

[Zopolis4]

So it was a VS skill issue all along.