You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@subversion.apache.org by Ryan Schmidt <su...@ryandesign.com> on 2019/11/17 19:47:15 UTC
Building subversion runs svnversion, which fails
Hi, I noticed this bug in subversion on macOS. I reported it here:
https://trac.macports.org/ticket/59712
But it's really an upstream issue so I would like to report it here as well.
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.
Nevertheless, fixing the build so that it does not run a program that crashes would be good.
Re: Building subversion runs svnversion, which fails
Posted by Branko Čibej <br...@apache.org>.
On 17.11.2019 22:24, Nathan Hartman wrote:
> On Sun, Nov 17, 2019 at 2:47 PM Ryan Schmidt <subversion-
> 2019@ryandesign.com <ma...@ryandesign.com>> wrote:
> > Hi, I noticed this bug in subversion on macOS. I reported it here:
> >
> > https://trac.macports.org/ticket/59712
> >
> > But it's really an upstream issue so I would like to report it here
> > as well.
> >
> > 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.
>
> I am not an expert on this subject but IIRC setting DYLD_LIBRARY_PATH
> will *not* help because of something called SIP (System Integrity
> Protection) on macOS.
SIP only affects how DYLD_LIBRARY_PATH works for system binaries, not
stuff you build yourself. For example, our test suite runs just fine in
the build directory with DYLD_LIBRARY_PATH set, but the Python (2)
bindings test suite does not, because it's run by the system Python binary.
Anyway: the short-term solution would be to run 'make install' with
'-j1' so that local-install runs before revision-install. The long-term
solution is to either remove that useless stanza from the Makefile, or
add proper dependencies between those two targets.
-- Brane
Re: Building subversion runs svnversion, which fails
Posted by Nathan Hartman <ha...@gmail.com>.
On Sun, Nov 17, 2019 at 4:24 PM Nathan Hartman <ha...@gmail.com>
wrote:
> On Sun, Nov 17, 2019 at 2:47 PM Ryan Schmidt <subversion-
> 2019@ryandesign.com> wrote:
> > 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.
>
> Wait a minute.
>
> "Unversioned directory" is one of the possible outputs of svnversion.
>
(snip)
That tells me that it must be running, not crashing.
>
> Of course, I could be mistaken.
>
I was mistaken.
You wrote that the file contains "unknown" not "Unversioned directory"; I
misread that earlier. In this case, yes, it is crashing. The "unknown" is
produced by the makefile if all else fails. The OR logic is also the reason
why 'make' doesn't quit at this point.
I am not at my computer right now so I cannot test why this is failing. It
might have something to do with order of operations. I'll investigate
further...
Thanks for your report...
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
Re: Building subversion runs svnversion, which fails
Posted by Nathan Hartman <ha...@gmail.com>.
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 Branko Čibej <br...@apache.org>.
On 18.11.2019 16:14, Ryan Schmidt wrote:
> On Nov 18, 2019, at 08:44, Ryan Schmidt wrote:
>
>> The relevant part in the log is where it says:
>>
>> (subversion/svnversion/svnversion . 2> /dev/null || \
>> svnversion . 2> /dev/null || \
>> echo "unknown"; \
>> ) > /opt/local/var/macports/build/_opt_bblocal_var_buildworker_ports_build_ports_devel_subversion/subversion/work/destroot/opt/local/include/subversion-1/svn-revision.txt
>> /bin/sh: line 1: 61814 Abort trap: 6 subversion/svnversion/svnversion . 2> /dev/null
> And just to be clear, it should work if it were changed so that it ran:
>
> (DYLD_LIBRARY_PATH=where/the/built/libs/are subversion/svnversion/svnversion . 2> /dev/null || \
> svnversion . 2> /dev/null || \
> echo "unknown"; \
> ) > /opt/local/var/macports/build/_opt_bblocal_var_buildworker_ports_build_ports_devel_subversion/subversion/work/destroot/opt/local/include/subversion-1/svn-revision.txt
>
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.
-- Brane
Re: Building subversion runs svnversion, which fails
Posted by Ryan Schmidt <su...@ryandesign.com>.
On Nov 18, 2019, at 08:44, Ryan Schmidt wrote:
> The relevant part in the log is where it says:
>
> (subversion/svnversion/svnversion . 2> /dev/null || \
> svnversion . 2> /dev/null || \
> echo "unknown"; \
> ) > /opt/local/var/macports/build/_opt_bblocal_var_buildworker_ports_build_ports_devel_subversion/subversion/work/destroot/opt/local/include/subversion-1/svn-revision.txt
> /bin/sh: line 1: 61814 Abort trap: 6 subversion/svnversion/svnversion . 2> /dev/null
And just to be clear, it should work if it were changed so that it ran:
(DYLD_LIBRARY_PATH=where/the/built/libs/are subversion/svnversion/svnversion . 2> /dev/null || \
svnversion . 2> /dev/null || \
echo "unknown"; \
) > /opt/local/var/macports/build/_opt_bblocal_var_buildworker_ports_build_ports_devel_subversion/subversion/work/destroot/opt/local/include/subversion-1/svn-revision.txt
Re: Building subversion runs svnversion, which fails
Posted by Ryan Schmidt <su...@ryandesign.com>.
On Nov 17, 2019, at 15:24, Nathan Hartman wrote:
> On Sun, Nov 17, 2019 at 2:47 PM Ryan Schmidt wrote:
>
> > Hi, I noticed this bug in subversion on macOS. I reported it here:
> >
> > https://trac.macports.org/ticket/59712
> >
> > But it's really an upstream issue so I would like to report it here
> > as well.
> >
> > 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.
>
> I am not an expert on this subject but IIRC setting DYLD_LIBRARY_PATH
> will *not* help because of something called SIP (System Integrity
> Protection) on macOS.
On newer versions of macOS, DYLD_* environment variables are no longer passed down to subshells. This merely means that you need to set DYLD_LIBRARY_PATH at the right time, in other words right as you're running svnversion, and not in some top-level script that will eventually after calling various sub scripts run svnversion.
> But I wonder, why does this fail? Subversion's test suite (which runs
> the svn binaries) runs just fine. It stands to reason that svnversion
> should run just fine, too.
I have not looked at your build system, but it fails for the reason I stated.
> That tells me that it must be running, not crashing.
It is crashing. The operating system crash logs (attached to the MacPorts ticket) were what alerted me to the problem.
> Of course, I could be mistaken. Please could you show us more output
> from 'make install' than the 3 lines in the MacPorts Trac issue?
Sure, here's one of the logs from our build system:
https://build.macports.org/builders/ports-10.14_x86_64-builder/builds/44459/steps/install-port/logs/stdio
The relevant part in the log is where it says:
(subversion/svnversion/svnversion . 2> /dev/null || \
svnversion . 2> /dev/null || \
echo "unknown"; \
) > /opt/local/var/macports/build/_opt_bblocal_var_buildworker_ports_build_ports_devel_subversion/subversion/work/destroot/opt/local/include/subversion-1/svn-revision.txt
/bin/sh: line 1: 61814 Abort trap: 6 subversion/svnversion/svnversion . 2> /dev/null
> Also, are you building from a distribution tarball or from a working
> copy checked out from Subversion's source repository?
Distribution tarball.
Re: Building subversion runs svnversion, which fails
Posted by Nathan Hartman <ha...@gmail.com>.
On Sun, Nov 17, 2019 at 2:47 PM Ryan Schmidt <subversion-
2019@ryandesign.com> wrote:
> Hi, I noticed this bug in subversion on macOS. I reported it here:
>
> https://trac.macports.org/ticket/59712
>
> But it's really an upstream issue so I would like to report it here
> as well.
>
> 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.
I am not an expert on this subject but IIRC setting DYLD_LIBRARY_PATH
will *not* help because of something called SIP (System Integrity
Protection) on macOS.
But I wonder, why does this fail? Subversion's test suite (which runs
the svn binaries) runs just fine. It stands to reason that svnversion
should run just fine, too.
> 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.
Wait a minute.
"Unversioned directory" is one of the possible outputs of svnversion.
$ svnversion --help
usage: svnversion [OPTIONS] [WC_PATH [TRAIL_URL]]
Subversion working copy identification tool.
(snip)
If WC_PATH is an unversioned path, the program will output
'Unversioned directory' or 'Unversioned file'. If WC_PATH is
an added or copied or moved path, the program will output
'Uncommitted local addition, copy or move'.
That tells me that it must be running, not crashing.
Of course, I could be mistaken. Please could you show us more output
from 'make install' than the 3 lines in the MacPorts Trac issue?
Also, are you building from a distribution tarball or from a working
copy checked out from Subversion's source repository?
Thanks,
Nathan