You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Shahaf <da...@elego.de> on 2013/06/21 12:29:37 UTC

Re: svn commit: r1495329 - /subversion/trunk/Makefile.in

philip@apache.org wrote on Fri, Jun 21, 2013 at 08:35:37 -0000:
> Author: philip
> Date: Fri Jun 21 08:35:37 2013
> New Revision: 1495329
> 
> URL: http://svn.apache.org/r1495329
> Log:
> Fix the svnmucc symlink in svn-tools so it doesn't to link to DESTDIR.
> The user can configure $(bindir) but $(toolsdir) is always a subdir
> of that so a relative symlink should be OK.
> 
> Found by: Nico Kadel-Garcia <nkadel{_AT_}gmail.com>
> 
> * Makefile.in
>   (INSTALL_EXTRA_TOOLS): Use a relative symlink.
> 
> Modified:
>     subversion/trunk/Makefile.in
> 
> Modified: subversion/trunk/Makefile.in
> URL: http://svn.apache.org/viewvc/subversion/trunk/Makefile.in?rev=1495329&r1=1495328&r2=1495329&view=diff
> ==============================================================================
> --- subversion/trunk/Makefile.in (original)
> +++ subversion/trunk/Makefile.in Fri Jun 21 08:35:37 2013
> @@ -912,5 +912,5 @@ INSTALL_EXTRA_TOOLS=\
>    test -n "$$SVN_SVNMUCC_IS_SVNSYITF" && \
>    ln -sf svnmucc$(EXEEXT) $(DESTDIR)$(bindir)/svnsyitf$(EXEEXT); \
>    if test "$(DESTDIR)$(bindir)" != "$(DESTDIR)$(toolsdir)"; then \
> -    ln -sf $(DESTDIR)$(bindir)/svnmucc$(EXEEXT) $(DESTDIR)$(toolsdir)/svnmucc$(EXEEXT); \
> +    ln -sf ../svnmucc$(EXEEXT) $(DESTDIR)$(toolsdir)/svnmucc$(EXEEXT); \

Shouldn't this read:
> +    ln -sf $(bindir)/svnmucc$(EXEEXT) $(DESTDIR)$(toolsdir)/svnmucc$(EXEEXT); \
?   Otherwise, if someone changes 'toolsdir' or 'bindir', or makes
'svn-tools' a symlink, the symlink would be wrong or dangling.

Daniel

>    fi
> 
> 

Re: svn commit: r1495329 - /subversion/trunk/Makefile.in

Posted by Daniel Shahaf <da...@elego.de>.
Philip Martin wrote on Fri, Jun 21, 2013 at 12:16:02 +0100:
> Daniel Shahaf <da...@elego.de> writes:
> 
> >> --- subversion/trunk/Makefile.in (original)
> >> +++ subversion/trunk/Makefile.in Fri Jun 21 08:35:37 2013
> >> @@ -912,5 +912,5 @@ INSTALL_EXTRA_TOOLS=\
> >>    test -n "$$SVN_SVNMUCC_IS_SVNSYITF" && \
> >>    ln -sf svnmucc$(EXEEXT) $(DESTDIR)$(bindir)/svnsyitf$(EXEEXT); \
> >>    if test "$(DESTDIR)$(bindir)" != "$(DESTDIR)$(toolsdir)"; then \
> >> -    ln -sf $(DESTDIR)$(bindir)/svnmucc$(EXEEXT) $(DESTDIR)$(toolsdir)/svnmucc$(EXEEXT); \
> >> +    ln -sf ../svnmucc$(EXEEXT) $(DESTDIR)$(toolsdir)/svnmucc$(EXEEXT); \
> >
> > Shouldn't this read:
> >> +    ln -sf $(bindir)/svnmucc$(EXEEXT) $(DESTDIR)$(toolsdir)/svnmucc$(EXEEXT); \
> > ?   Otherwise, if someone changes 'toolsdir' or 'bindir', or makes
> > 'svn-tools' a symlink, the symlink would be wrong or dangling.
> 
> Change in what way?  The user can configure a different bindir but
> toolsdir is hardcoded as bindir/svn-tools.  There is no way for a user

If someone changes the hardcoded values, then the relative symlink will
break.  It'd be more robust for the symlink to work without assuming
a particular relationship between $toolsdir and $bindir (namely, that
the latter is a child of the former).

> to change toolsdir to something other than bindir/svn-tools.  If we
> change the hardcoded svn-tools subdir then the symlink will break but I
> don't think that matters.

My point wasn't that _we_ change the hard-coded value, it was the case
that DESTDIR == "" and $bindir/svn-tools is a symlink.  In that case the
new code would install a broken symlink.

> The real solution is for the svn-tools subdir
> to go away and for install-tools to install the tools into bindir.

I'm not sure we can do that in a patch release, so we still need
another solution for 1.8.1.

> Why did we ever think a subdir of bindir was a good idea?
> 
> I suppose the user could set up a symlink for bindir/svn-tools before
> running 'make install-tools', but the most likely symlink would be to
> bindir itself and then no symlink is possible.

Fair point, but it's orthogonal to what the destination of the symlink
is decided to be.

Cheers,

Daniel

Re: svn commit: r1495329 - /subversion/trunk/Makefile.in

Posted by Philip Martin <ph...@wandisco.com>.
Daniel Shahaf <da...@elego.de> writes:

>> --- subversion/trunk/Makefile.in (original)
>> +++ subversion/trunk/Makefile.in Fri Jun 21 08:35:37 2013
>> @@ -912,5 +912,5 @@ INSTALL_EXTRA_TOOLS=\
>>    test -n "$$SVN_SVNMUCC_IS_SVNSYITF" && \
>>    ln -sf svnmucc$(EXEEXT) $(DESTDIR)$(bindir)/svnsyitf$(EXEEXT); \
>>    if test "$(DESTDIR)$(bindir)" != "$(DESTDIR)$(toolsdir)"; then \
>> -    ln -sf $(DESTDIR)$(bindir)/svnmucc$(EXEEXT) $(DESTDIR)$(toolsdir)/svnmucc$(EXEEXT); \
>> +    ln -sf ../svnmucc$(EXEEXT) $(DESTDIR)$(toolsdir)/svnmucc$(EXEEXT); \
>
> Shouldn't this read:
>> +    ln -sf $(bindir)/svnmucc$(EXEEXT) $(DESTDIR)$(toolsdir)/svnmucc$(EXEEXT); \
> ?   Otherwise, if someone changes 'toolsdir' or 'bindir', or makes
> 'svn-tools' a symlink, the symlink would be wrong or dangling.

Change in what way?  The user can configure a different bindir but
toolsdir is hardcoded as bindir/svn-tools.  There is no way for a user
to change toolsdir to something other than bindir/svn-tools.  If we
change the hardcoded svn-tools subdir then the symlink will break but I
don't think that matters. The real solution is for the svn-tools subdir
to go away and for install-tools to install the tools into bindir.  Why
did we ever think a subdir of bindir was a good idea?

I suppose the user could set up a symlink for bindir/svn-tools before
running 'make install-tools', but the most likely symlink would be to
bindir itself and then no symlink is possible.

-- 
Philip Martin | Subversion Committer
WANdisco | Non-Stop Data
www.wandisco.com