Topic: The stored revision can become internally inconsistent

[ACarlotti]

At the moment (as far as I can tell) the revision string is #defined into many files, which are not necessarily recompiled when running make, even if the revision string has changed, unless a clean build is chosen deliberately. The result of this is that testing network play requires either producing clean builds each time the revision changes, or excluding/hardcoding the revision string. I think it ought to be possible to change the revision without having to do a clean build. A quick test shows that even with the current code, recompiling only the files affected by a revision change is more than an order or magnitude faster than compiling a clean build. Furthermore, since the revision string is rarely accessed, I don't see any need for it to be #defined everywhere. It could instead be defined into a single-purpose .cc file which is always rebuilt, with every other bit of code accessing it via the corresponding .h file.

What do you think? I realise that it is still possible to 'shoot yourself in the foot' by building with a dirty working tree, but I think that's much less problematic than having a build based upon a 'correct' working tree having traces of older revision numbers in it. This is a problem that caught me out earlier, although I didn't realise what had happened until later, after I started thinking about how the revision string was updated.

[Ters]

At work, test builds are always clean builds. (Developer builds are not, but they never leave the developers machine and only need to be compatible with itself.) Doing clean builds after checking out new code is therefore second nature to me, even if it is perhaps somewhat paranoid. There may be other quirks in the build system that can cause complete rebuilds to be necessary outside of changes in the code files being compiled. Revision number is also not the only thing being passed into to the code from outside cc and h files. Some even come from outside Simutrans' build system, like the stuff sdl-config/sdl2-config, allegro-config, freetype-config and png-config wants to add to your compiler arguments.

I guess one of the reasons the define is used all over the place is because compile time string concatenation is easier to write safely than runtime string concatenation. At least when Simutrans avoids/avoided std::string. (It also turns it into a nice little constant, rather than something on the heap, although I doubt the effect is noticeable in a big application such as Simutrans.)

[ACarlotti]

I was making what you describe as a "developer build" at the time - it just happened that my testing was using two separate builds with different configuration, which were not previously on the same version.

I am aware that the revision number is not the only thing being passed from outside .cc/.h files. However, it differs from the other things you mention in that it changes frequently and is relevant to synchronisation of network play. It could also be separated out without too much difficulty.

As far as I can tell there is little motivation to change it, nor much reason not to change it. So if I were to change the system so that the revision is written to revision.cc, and accessed by including revision.h, would there by any objections to including that change? Or any better ideas about how to do it? (It would probably also replace parts of the existing system for handling the revision number.)

[TurfIt]

is relevant to synchronisation of network play.

How so?  The only thing the revision is used for is the server list, and filtering out 'incompatible' versions. A production server list and development builds don't go together. i.e. Don't announce dev servers.


How do you propose creating this revision.cc?  Note the current system is portable to the many build systems in play, even with a hack to make it work with MSVC.
I also note the current system has issues with dependencies for files modified by the build - e.g. the whole winres simres.rc thing on Windows.

[ACarlotti]

How so?  The only thing the revision is used for is the server list, and filtering out 'incompatible' versions. A production server list and development builds don't go together. i.e. Don't announce dev servers.

I quite agree. However, what ought to go together fairly easily is multiple dev builds at the same time (e.g. Windows/Linux, or a command-line only server), which is the sort of thing I have been trying to test with. Inconsistent revisions, even when the code is otherwise identical, prevent testing the server/client in this case, and might be a confusing error if the client is displaying the correct revision in the title bar but is using a different one to check whether a server is running the 'same' revision.

How do you propose creating this revision.cc?  Note the current system is portable to the many build systems in play, even with a hack to make it work with MSVC.

At the moment I think the MSVC hack writes a revision.h file - this could just directly write a revision.cc file instead. For most other systems the code in the Makefile that currently writes to CFLAG could write the revision to a file instead. I'm not aware of any case where neither of these approaches is viable.

An added benefit of this approach could be that the revision could be included when building from a zip download by just having the file revision.cc be part of the download. However, this wouldn't work automatically downloading it straight from version control, but could be added in for other sources.

I also note the current system has issues with dependencies for files modified by the build - e.g. the whole winres simres.rc thing on Windows.

The stuff in simres.rc does not appear to include the revision number (which is defined by the source control), but rather just the version information that is defined in source files themselves. Therefore it is something that changes much less frequently, and can (at least in theory) be kept in sync by other means (if Make and/or MSVC know about the relevant dependencies).

[TurfIt]

Inconsistent revisions, even when the code is otherwise identical, prevent testing the server/client in this case, and might be a confusing error if the client is displaying the correct revision in the title bar but is using a different one to check whether a server is running the 'same' revision.

Confusing, sure, but nothing is prevented. You can connect to any server with any revision you want. If there's changes in sync critical code, you'll desync, and you can't cheat since the server is still running it's own code, but if you want to run a custom UI or something, no problem. The embedded revision is irrelevant for connecting.

[ACarlotti]

Confusing, sure, but nothing is prevented. You can connect to any server with any revision you want.

The 'join game' button is disabled if both server and client are built with a revision and these revisions don't match.

[TurfIt]

So, direct connect.

[ACarlotti]

Do you mean this?

You can have this even easier, since the special file name "net:IP:port" will connect to any server. So maybe start the server with a batch file "simutrans -server -objects pak64.japan", then you can connect with the other "simutrans -objects pak64.japan -load net:192.165.1.44" or whatever your IP is.

It took some searching to find this - it's completely undocumented in the command line help text, and I only found it because DrSuperGood referred earlier in the same thread to "direct connection", by which he means exactly the thing I can't do if the revisions don't match.
Anyway, I shall begin using that from now - it's better than the alternative of loading a small game with default settings just to access the network gui.

[prissi]

Back to the original topic. At least for standard, after a change of the SVN only the changed files and the ones affected by the new revision are built. So it works as expected.

The nightlies are clean builds anyway, since after building the Window version, there is a Linux build in the same directory ...

[Ters]

At least for standard, after a change of the SVN only the changed files and the ones affected by the new revision are built. So it works as expected.

Last time I paid attention, only the changed files were rebuilt when using make. Files that have not changed, but which depend on revision number, are not rebuilt. So I have had builds which show the wrong revision number in the title bar, since simsys_w wasn't rebuilt. For me, this was expected, but expectations are subjective.

Maybe Visual Studio is different. revision.jse seems to be set up to run at the start of every build. Visual Studio may then detect if its contents changes, and rebuild all files including it accordingly.

[ACarlotti]

Yes, the behaviour is different between using Visual Studio and Make. I think it would be sensible to change the behaviour using Make - we could switch it to using a similar system to how Visual Studio incorporates the revision already (write to revision.h). This would mean that files that depend on the revision number are rebuilt whenever this changes.

The alternative that I originally suggested is to use both a revision.h and a revision.cc - this would be more complicated, but would have the benefit that the .cc files that use the version don't need to be rebuilt every time the version changes.