I sometimes get the following warning during a build:

./Modules/getbuildinfo.c:23:1: warning: "SVNVERSION" redefined
<command line>:1:1: warning: this is the location of the previous definition


It looks like SVNVERSION is defined in getbuildinfo.c, but a value is also passed in on the command-line in the Makefile when the module is built (line 460).  I have no clue why this is being done.

This was broken in

------------------------------------------------------------------------
r55024 | kristjan.jonsson | 2007-04-30 17:17:46 +0200 (Mo, 30 Apr 2007) | 1 line

Complete revamp of PCBuild8 directory.  Use subdirectories for each project under the main pcbuild solution.  Now make extensive use of property sheets to simplify project configuration. x64 build fully supported, and the process for building PGO version (Profiler Guided Optimization) simplified. All projects are now present, except _ssl, which needs to be reimplemented.  Also, some of the projects that require external libraries need extra work to fully compile on x64.
------------------------------------------------------------------------

Kristjan, what was the rationale for making that specific change to getbuildinfo?

The intent was to remove the reliance on the define SUBWCREV so to simplify the actions of make_buildinfo.c.  It then only needs to either copy it or run it through subwcrev.exe.  Now the code autodetects whether it was passed through subwcrev.exe or not by examining if the string was interpolated.  What is the purpose of the SVNVERSION macro passed on the command line?  If it has the same value, then maybe a conditional #define will fix this back?

Otherwise, rolling back the change will mean that make_buildinfo.c will have to create some other info for the subsequent compilation of getbuildinfo.c to define or undefin SVNWCREV

Well, when I remove the command-line def (the entire -D argument when building getbuildinfo.c) I get no subversion number out of sys.subversion.  But when I comment out the SVNVERSION definition instead in getbuildinfo.c I do have the subversion number show up.

And I just noticed that when both are defined I have no subversion number (as is the case in the trunk at the moment).

I just wrapped the SVNVERSION definition in getbuildinfo.c in a #ifndef and that fixed the problem.  but I don't know if doing that will just mask an issue where SVNVERSION should not be defined in getbuildinfo.c at all if it is being passed in during compilation.

SVNVERSION is passed on Unix. On Unix, there is no subwcrev program. Instead, there is an svnversion program which computes the same string as subwcrev, but prints it on stdout (rather than substituting it in some template file).

Therefore, SVNVERSION needs to be passed on the command line.

I figure that much from my experiment.  What I am wondering is if the #define in getbuildinfo.c is needed.  Does the Windows build need it?  Or is it that just to guarantee that the file is self-supported and won't fail to compile if the compile-time define is not passed in?

If the #define in the file is required then I vote for the #ifndef solution with an explanation.

The define is indeed needed on Windows. On a Windows installation, there is typically no svnversion binary. However, TortoiseSVN ships with a similar tool, calls subwcrev.exe, which does string interpolation on a template file. So the define in getbuildinfo is the template, and the link process creates a second C file (getbuildinfo1.c) which is then compiled and linked into the interpreter. As subwcrev.exe might not be installed, the build process used to pass a define SUBWCREV on the command line, which was a condition to the definition of SVNVERSION. As Kristjan removed the condition, the Unix build broke.

Here is a suggested patch.  I don't have a linux setup to test with this, what do you think?
Oh, and sorry for messing this up, I thought subwcrev was used on all platforms :(
File Added: woo.patch

Looks fine to me, please apply.

Applied the patch in change 55821, and backported to 25-maint in 55822