Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 5 Mar 2017 13:39:06 -0800 (PST)
From:      "Rodney W. Grimes" <freebsd@pdx.rh.CN85.dnsmgr.net>
To:        Bryan Drewery <bdrewery@FreeBSD.org>
Cc:        rgrimes@FreeBSD.org, Baptiste Daroussin <bapt@FreeBSD.org>, freebsd-hackers@FreeBSD.org
Subject:   Re: svn commit: r314693 - head/usr.sbin/rmt
Message-ID:  <201703052139.v25Ld6iR084880@pdx.rh.CN85.dnsmgr.net>
In-Reply-To: <1283ca30-a827-8b32-3021-658548447c22@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
> On 3/5/17 8:26 AM, Rodney W. Grimes wrote:
> > Moved thread to -hackers for a quick discussion.
> > 
> >> On Sun, Mar 05, 2017 at 05:19:28AM -0800, Rodney W. Grimes wrote:
> >>> -- Start of PGP signed section.
> >>>> On Sun, Mar 05, 2017 at 04:09:18AM +0000, Rodney W. Grimes wrote:
> >>>>> Author: rgrimes
> >>>>> Date: Sun Mar  5 04:09:18 2017
> >>>>> New Revision: 314693
> >>>>> URL: https://svnweb.freebsd.org/changeset/base/314693
> >>>>>
> >>>>> Log:
> >>>>>   Change /etc/rmt symlink from absolute to relative path,
> >>>>>   correcting the mistake made in r6499
> >>>>>   
> >>>>>   Approved by:	grehan
> >>>>>   MFC after:	1 week
> >>>>>
> >>>>> Modified:
> >>>>>   head/usr.sbin/rmt/Makefile
> >>>>>
> >>>>> Modified: head/usr.sbin/rmt/Makefile
> >>>>> ==============================================================================
> >>>>> --- head/usr.sbin/rmt/Makefile	Sun Mar  5 04:02:47 2017	(r314692)
> >>>>> +++ head/usr.sbin/rmt/Makefile	Sun Mar  5 04:09:18 2017	(r314693)
> >>>>> @@ -7,6 +7,6 @@ MAN=	rmt.8
> >>>>>  # called from /usr/src/etc/Makefile
> >>>>>  etc-rmt:
> >>>>>  	rm -f ${DESTDIR}/etc/rmt
> >>>>> -	ln -s ${BINDIR}/rmt ${DESTDIR}/etc/rmt
> >>>>> +	ln -s ..${BINDIR}/rmt ${DESTDIR}/etc/rmt
> >>>>
> >>>> I think this should be ${INSTALL_RSYMLINK} ${BINDIR}/rmt ${DESTDIR}/etc/rmt
> >>>
> >>> find /usr/src | xargs grep INSTALL_RSYM
> >>> (no results)
> >>>
> >>> Sorry, no prior work does this, perhaps once I get done sweeping the
> >>> absolutes out of the tree (about 10 or 15 IIRC) a pass can be made to
> >>> sweep all ln -s out and propage this internal bsd.lib.mk function out
> >>> to the rest of the source tree?
> >>
> >> There is also no Makefiles that do ls -sf directly beside that one.
> > Unless I have missed a commit:
> > ./crypto/openssh/contrib/cygwin/Makefile:       cd $(DESTDIR)$(mandir)/man1 && ln -s ssh.1.gz slogin.1.gz
> > ./usr.sbin/sendmail/Makefile:   ln -sf ${.ALLSRC} ${.TARGET}
> > ./usr.sbin/rmt/Makefile:        ln -s ${BINDIR}/rmt ${DESTDIR}/etc/rmt
> > ...
> 
> Keep in mind that INSTALL_*SYMLINK should only be used for *installing*
> a symlink, not for intermediate build files.  All of the direct 'ln'
> usage in the tree should be not installed.  Brooks and I and others have
> done passes before to ensure that any installed symlink uses
> INSTALL_*SYMLINK.  The reasoning is that the -DNO_ROOT build requires
> that 'install' be used since it is logging the file in a meta log that
> is later used to build an image from.  This is also important for the
> DIRDEPS_BUILD feature.

In those several passes you have missed at least this one here in rmt
that has been there since the refer commit of r6499.  This is not a
new link someone added recently.  I simply corrected the arguments to
the command so that we no longer have an absolute link inside
of ${DESTDIR}.

Let me review my other 10 or so pending commits again, but I think all
of those are errors in SYMLINKS=.  Bapt did not answer my question
on how to deal with SYMLINKS hardcoded to use INSTALL_SYMLINKS but
I well need it to use INSTALL_RSYMLINKS for some of these corrections.

For now I have just feed the proper arguments to SYMLINKS so that it
creates proper relative paths.  

> > A summary is there are 50 instances of ln -sf,  28 other variants of ln -s,
> > and 5 ln -fs.  I did not search for other permutaions of ln and s f options.
...
REVIEWING my patches I see this:
--- share/termcap/Makefile      (revision 314708)
+++ share/termcap/Makefile      (working copy)
@@ -24,6 +24,6 @@
        cap_mkdb ${CAP_MKDB_ENDIAN} -f ${.TARGET:R} ${.ALLSRC}

 etc-termcap:
-       ${INSTALL_SYMLINK} ${BINDIR}/misc/termcap ${DESTDIR}/etc/termcap
+       ${INSTALL_SYMLINK} ..${BINDIR}/misc/termcap ${DESTDIR}/etc/termcap

 .include <bsd.prog.mk>

I'll convert that to INSTALL_RSYMLINK, all others are in
SYMLINK lists.  That should get handled by your .mk modifications to
do the right thing.


-- 
Rod Grimes                                                 rgrimes@freebsd.org



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201703052139.v25Ld6iR084880>