You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Nathan Hartman <ha...@gmail.com> on 2019/11/19 04:53:34 UTC

Re: Building subversion runs svnversion, which fails

Moving this from users@ to dev@...

Ryan Schmidt <su...@ryandesign.com> wrote:
> The subversion build evidently runs the just-built svnversion at
> some point. This crashes, because the libraries it's linked with
> haven't been installed yet, and I guess it's not setting
> DYLD_LIBRARY_PATH to the path where the built libraries are. I
> assume that the fix will be setting that variable.
>
> This does not cause the build of subversion to fail, but whatever
> it's running svnversion for can't be getting the correct result.
> Whether that's important, I don't know.
>
> The MacPorts maintainer of subversion suggested:
>
>> That's almost certainly from the revision-install target in the
>> top level makefile. If it ran, it would populate
>> ${prefix}/include/subversion-1/svn-revision.txt with "Unversioned
>> directory", since it crashes that file gets populated with
>> "unknown". I don't think it actually matters.

Brane wrote:
> As I wrote earlier, the real problem is that 'revision-install' runs
> before 'local-install' has finished. The binaries have RPATH to the
> install location embedded in them, so clearly the libraries haven't
> been installed at the point where svnversion is run.

I am unable to reproduce this crash on my Mac.

It occurred to me that I should be able to force a failure by running:

$ ./autogen.sh
$ ./configure --with-apr=/usr/local/opt/apr \
              --with-apr-util=/usr/local/opt/apr-util \
              --prefix=/usr/local/Test
$ make
$ make revision-install

Because, as Brane points out, the 'revision-install' target does not
tell make that it depends on 'local-install', this should have failed.
It did not. It ran, did not crash, and installed a perfectly valid
svn-revision.txt file:

$ cat /usr/local/Test/include/subversion-1/svn-revision.txt
1870003M

That 'M' is there because I modified my working copy:

Unmodified, 'revision-install' looks like this:

.
revision-install:
test -d $(DESTDIR)$(includedir)/subversion-1 || \
 $(MKDIR) $(DESTDIR)$(includedir)/subversion-1
(subversion/svnversion/svnversion $(top_srcdir) 2> /dev/null ||  \
svnversion $(top_srcdir) 2> /dev/null ||                        \
echo "unknown";                                                 \
) > $(DESTDIR)$(includedir)/subversion-1/svn-revision.txt
.

Which means that if 'subversion/svnversion/svnversion' fails and an
installed 'svnversion' is found on the PATH, it will run instead. I
removed that line to ensure that I'd run either the built svnversion
or print "unknown" to the file.

Failing to reproduce it, I cannot test whether any change I might
suggest would actually solve the problem, so my confidence in the
following isn't so great:

This one-line change should force 'revision-install' to wait until
'local-install' has completed, but comes with the side effect that if
anyone uses 'make revision-install' to install *just* svn-revision.txt
it will now go ahead and perform a local-install too... Not sure if
that's a problem for anyone.

.
$ svn diff
Index: Makefile.in
===================================================================
--- Makefile.in (revision 1870003)
+++ Makefile.in (working copy)
@@ -493,7 +493,7 @@

 local-install: @INSTALL_RULES@

-revision-install:
+revision-install: local-install
  test -d $(DESTDIR)$(includedir)/subversion-1 || \
   $(MKDIR) $(DESTDIR)$(includedir)/subversion-1
  (subversion/svnversion/svnversion $(top_srcdir) 2> /dev/null ||  \
.

Another alternative might be to eliminate 'revision-install' and move
that to 'install'.

On a somewhat related note (and at risk of blasphemy), if we want to
provide a way for users to know what revision their svn install was
built from, which would work for those who build from tarballs also,
would it make sense to create a header file for this purpose, turn on
svn:keywords for it, and put "$Revision: $" into a string that would
be parsed and printed by 'svn --version' in place of the
"under development" string that currently is changed manually for
releases?

My installed svn, built from a release:

$ svn --version
svn, version 1.12.2 (r1863366)
                    ^^^^^^^^^^

vs the one I built for the above test:

$ ./svn --version
svn, version 1.14.0-dev (under development)
                        ^^^^^^^^^^^^^^^^^^^

Nathan

Re: Building subversion runs svnversion, which fails

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Nathan Hartman wrote on Tue, 19 Nov 2019 18:25 +00:00:
> On Tue, Nov 19, 2019 at 12:04 PM Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> > The concept of having the binary know the `svnversion` info of the
> >  working copy it was built from does make sense, however. We could make
> >  gen-make.py add «-DFOO=`svnversion`» to CFLAGS. (There's already
> >  something like that, «fakedefines», in the Windows-specific part of the
> >  build generator.)
> 
> That wouldn't do what we want either. If you build from a tarball,
> it'll bake "Unversioned directory" into the binaries. Also it would
> mean that you must have svn installed in order to build svn,
> regardless whether the sources were obtained from distribution tarball
> or otherwise.
> 
> So... I guess what we're doing now makes sense.
> 
> Oh well, it was a nice thought.

We could still do:
.
    if any(map(re.compile(r'SVN_VER_TAG\s+" [(]under development[)]"').search, open('subversion/include/svn_version.h'))):
        try:
            fakedefines.append("SVN_VER_TAG_OVERRIDE=" + subprocess.check_output(['svnversion', '-n']))
        except subprocess.CalledProcessError:
            pass
.
and in svn_version.h, surround the definition of SVN_VER with:
.
    #ifdef SVN_VER_TAG_OVERRIDE
    #define SVN_VER_TAG SVN_VER_TAG_OVERRIDE
    #else
    #define SVN_VER_TAG        " (under development)"
    #endif

For this to work, we'll need to make the Unix part of the build generator
support fakedefines too.  Tarball builds will get the revision number from
SVN_VER_TAG, which release.py munges at tagging time; branch@HEAD will get the
revision number if svnversion was available at build time, but not otherwise.

I suppose it's possible to do branch@HEAD builds without having svnversion
installed — for starters, by using https://github.com/apache/subversion/ —
but anyone who does that can just send us a patch to define SVN_VER_TAG_OVERRIDE
accordingly.

Cheers,

Daniel

Re: Building subversion runs svnversion, which fails

Posted by Nathan Hartman <ha...@gmail.com>.
On Tue, Nov 19, 2019 at 12:04 PM Daniel Shahaf <d....@daniel.shahaf.name>
wrote:

> Nathan Hartman wrote on Tue, 19 Nov 2019 04:53 +00:00:
> > Moving this from users@ to dev@...
>
> [Aside: when doing this, it's a good to post a short response on users@
> saying the conversation continues on dev@, for anyone who wants to
> follow along but isn't subscribed to both lists.]
>
> > On a somewhat related note (and at risk of blasphemy), if we want to
> > provide a way for users to know what revision their svn install was
> > built from, which would work for those who build from tarballs also,
> > would it make sense to create a header file for this purpose, turn on
> > svn:keywords for it, and put "$Revision: $" into a string
>
> If we do this, we'll run into this issue:
>
> http://subversion.apache.org/faq#version-value-in-source
>
> tl;dr: Putting $Revision$ in a header file will expand to the last
> changed revision of _that_ header file, not to the last changed revision
> of anything in cwd.
>

Ah, yes. Thanks for pointing that out. That's not what we want.

The concept of having the binary know the `svnversion` info of the
> working copy it was built from does make sense, however.  We could make
> gen-make.py add «-DFOO=`svnversion`» to CFLAGS.  (There's already
> something like that, «fakedefines», in the Windows-specific part of the
> build generator.)
>

That wouldn't do what we want either. If you build from a tarball,
it'll bake "Unversioned directory" into the binaries. Also it would
mean that you must have svn installed in order to build svn,
regardless whether the sources were obtained from distribution tarball
or otherwise.

So... I guess what we're doing now makes sense.

Oh well, it was a nice thought.

Nathan

Re: Building subversion runs svnversion, which fails

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Nathan Hartman wrote on Tue, 19 Nov 2019 04:53 +00:00:
> Moving this from users@ to dev@...

[Aside: when doing this, it's a good to post a short response on users@
saying the conversation continues on dev@, for anyone who wants to
follow along but isn't subscribed to both lists.]

> On a somewhat related note (and at risk of blasphemy), if we want to
> provide a way for users to know what revision their svn install was
> built from, which would work for those who build from tarballs also,
> would it make sense to create a header file for this purpose, turn on
> svn:keywords for it, and put "$Revision: $" into a string

If we do this, we'll run into this issue:

http://subversion.apache.org/faq#version-value-in-source

tl;dr: Putting $Revision$ in a header file will expand to the last
changed revision of _that_ header file, not to the last changed revision
of anything in cwd.

The concept of having the binary know the `svnversion` info of the
working copy it was built from does make sense, however.  We could make
gen-make.py add «-DFOO=`svnversion`» to CFLAGS.  (There's already
something like that, «fakedefines», in the Windows-specific part of the
build generator.)

Cheers,

Daniel