Date: Sat, 28 Jan 2017 16:32:17 -0800 (PST) From: "Rodney W. Grimes" <freebsd-rwg@pdx.rh.CN85.dnsmgr.net> To: Julian Elischer <julian@freebsd.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org Subject: Re: svn commit: r312871 - stable/11/share/zoneinfo Message-ID: <201701290032.v0T0WHLF032469@pdx.rh.CN85.dnsmgr.net> In-Reply-To: <722a5ab1-f997-fa44-4f67-91430673622d@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
> Hi Rod, Hello Julian, > On 27/1/17 11:25 pm, Rodney W. Grimes wrote: > >> Author: julian > >> Date: Fri Jan 27 09:11:44 2017 > >> New Revision: 312871 > >> URL: https://svnweb.freebsd.org/changeset/base/312871 > >> > >> Log: > >> MFH: r308671 > >> > >> When you select make OLDTIMEZONES=1 then you need a few added directories > >> to be made or the command fails > >> > >> Sponsored by: panzura > >> > >> MFH: r310426 > >> > >> If you are going to be run individually to make a new timezone set > >> then ensure the destination directories exist. > >> Especially if you define OLDTIMEZONES because the mtree pass > >> doesn't do it for you. > > Perhaps it is time to start pre-processing mtree files so this > > would no longer be the case? > A much bigger question than I can handle at this time.. :-) Fair enough. I did run (cd /usr/src; find . -name Makefile | xargs grep mkdir | more) to see how much of this has occured, and was happy to see that only a few places. When I did all the early mtree work I had to remove mkdir/chown/chmod's in the Makefiles. > >> Sponsored by: Panzura > >> > >> Modified: > >> stable/11/share/zoneinfo/Makefile > >> Directory Properties: > >> stable/11/ (props changed) > >> > >> Modified: stable/11/share/zoneinfo/Makefile > >> ============================================================================== > >> --- stable/11/share/zoneinfo/Makefile Fri Jan 27 09:07:11 2017 (r312870) > >> +++ stable/11/share/zoneinfo/Makefile Fri Jan 27 09:11:44 2017 (r312871) > >> @@ -67,6 +67,10 @@ TZBUILDSUBDIRS= \ > >> Pacific \ > >> SystemV > >> > >> +.if defined(OLDTIMEZONES) > >> +TZBUILDSUBDIRS+= US Mexico Chile Canada Brazil > >> +.endif > >> + > >> .if !defined(_SKIP_BUILD) > >> all: zoneinfo > >> .endif > >> @@ -81,6 +85,8 @@ zoneinfo: yearistype ${TDATA} > >> > >> beforeinstall: install-zoneinfo > >> install-zoneinfo: > >> + mkdir -p ${DESTDIR}/usr/share/zoneinfo > >> + cd ${DESTDIR}/usr/share/zoneinfo; mkdir -p ${TZBUILDSUBDIRS} > > This has the failure mode that your current uid and umask, etc are > > used in creating the directories. Please add appropriate chown/chmod > > commands to reflect what mtree would of done had it done this. > This is mostly because it failed when run as an individual operation > to update zoninfo files. I think it would also fail on a complete installworld if you had OLDTIMEZONES set would it not? > which would probably be done as root, or someone who wants the files > owned by them. > You can't write here unless you are root, so I'm not sure what the > right thing to do would be. > and chown doesn't work if you are not root, so what would you suggest? This is an install phase thing, so is only expected to work as root, and hence should be coded assuming it is running as root, so chown/chmod are fine, though still a sic bandaide. > (what is needed for the offline permissions tool)? > should we assume ${DESTDIR}/usr/share exists already? Probably, I dont like the -p on mkdir cause if any of the components are missing you can easy create ones with wrong permissions using this. > > the following might be a very slight improvement, but ... > > mkdir -p -m 755 ${DESTDIR}/usr/share Never, clearly the space of etc/mtree/BSD.usr.dist, and if this directoy does not exist your make install SHOULD fail. > mkdir -p -m 755 ${DESTDIR}/usr/share/zoneinfo Ditto > cd ${DESTDIR}/usr/share/zoneinfo; mkdir -p -m 755 ${TZBUILDSUBDIRS} This one I am ok with. Though I think the code in the Makefile already does a umask override so not sure what is going on now. > -cd ${DESTDIR}/usr/share/zoneinfo; chown -R ${BINOWN}:${BINGRP} . < would fail if you are not root. No need for the -, it should fail if you are not root, running install as non root inside /usr/src has never been supported. Another, simple, and cleaner solution would be to write etc/mtree/BSD.oldtz, properly populate it, and invoke it, when appropriate, from the make hierarchy target. Sorry you stumbled into one of my pedantic things about the src tree not having mkdir -p in the Makefiles. -- Rod Grimes rgrimes@freebsd.org
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201701290032.v0T0WHLF032469>