On Wed, May 23 2018, Marc Espie <[email protected]> wrote: > On Tue, May 22, 2018 at 11:50:56PM +0200, Klemens Nanni wrote: >> On Mon, May 21, 2018 at 07:22:13PM +0200, Marc Espie wrote: >> > > @@ -3434,17 +3434,17 @@ show: >> > > # du fails if it can't access everything >> > > show-size: >> > > @if du -ks ${WRKDIR} 2>/dev/null >${WRKDIR}/wrkdir-size; then \ >> > > - cat ${WRKDIR}/wrkdir-size && rm -f >> > > ${WRKDIR}/wrkdir-size; \ >> > > + cat ${WRKDIR}/wrkdir-size && ${_PBUILD} rm -f >> > > ${WRKDIR}/wrkdir-size; \ >> > > else \ >> > > - chmod -R u+rX ${WRKDIR}; \ >> > > + ${_PBUILD} chmod -R u+rX ${WRKDIR}; \ >> > > du -ks ${WRKDIR}; \ >> > > fi >> > > >> > > show-fake-size: >> > > @if du -ks ${WRKINST} 2>/dev/null >${WRKINST}/wrkdir-size; then >> > > \ >> > > - cat ${WRKINST}/wrkdir-size && rm -f >> > > ${WRKINST}/wrkdir-size; \ >> > > + cat ${WRKINST}/wrkdir-size && ${_PBUILD} rm -f >> > > ${WRKINST}/wrkdir-size; \ >> > > else \ >> > > - chmod -R u+rX ${WRKINST}; \ >> > > + ${_PBUILD} chmod -R u+rX ${WRKINST}; \ >> > > du -ks ${WRKINST}; \ >> > > fi >> > > >> > Those should definitely be done differently... Namely adding a file under >> > WRKDIR seems like overkill, stuff could go to a tmp file >> > and also it's probably more useful to always go _PBUILD for the du instead >> > of >> > chmod'ing. >> Here's a diff that just runs `du' as _pbuild which may fail and show >> inaccessible files as are. >> >> For bad permissions in WRKDIR, espie (recently) introduced >> FIX_EXTRACT_PERMISSIONS[0], which already does >> >> 2505 # run as _pbuild >> 2506 _post-extract-finalize: >> ... >> 2512 .if ${FIX_EXTRACT_PERMISSIONS:L} == "yes" >> 2513 › @chmod -R a+rX ${WRKDIR} >> >> >> and for bad permissions in WRKINST, there's >> >> 2789 ${_FAKE_COOKIE}: ${_BUILD_COOKIE} ${_FAKESUDO_CHECK_COOKIE} >> ... >> 2802 .if ${FAKE_AS_ROOT:L} != "yes" >> 2803 › @${_FAKESUDO} chmod -R a+rX ${WRKINST} >> >> >> Thus, by the time `show-size' or `show-fake-size' runs, >> `_post-extract-finalize' or `${_FAKE_COOKIE}' respectively will have >> adjusted permissions already. >> >> Scenarios where permissions change in between running these targets are >> not covered, but `show(-fake)-size' is the wrong place to take care of >> this anyway imho. >> >> With this in mind, I don't see why we should still bother with stashing >> the output and rerunning `du'. >> >> Feedback? >> >> 0: https://marc.info/?l=openbsd-ports&m=151266048117858&w=2 >> >> Index: bsd.port.mk >> =================================================================== >> RCS file: /cvs/ports/infrastructure/mk/bsd.port.mk,v >> retrieving revision 1.1403 >> diff -u -p -r1.1403 bsd.port.mk >> --- bsd.port.mk 21 May 2018 21:26:39 -0000 1.1403 >> +++ bsd.port.mk 22 May 2018 20:32:11 -0000 >> @@ -3430,23 +3430,12 @@ show: >> @echo ${${_s}:Q} >> .endfor >> >> -# avoid sudo and avoid modifying dir (if possible): >> # du fails if it can't access everything >> show-size: >> - @if du -ks ${WRKDIR} 2>/dev/null >${WRKDIR}/wrkdir-size; then \ >> - cat ${WRKDIR}/wrkdir-size && rm -f ${WRKDIR}/wrkdir-size; \ >> - else \ >> - chmod -R u+rX ${WRKDIR}; \ >> - du -ks ${WRKDIR}; \ >> - fi >> + @${_PBUILD} du -ks ${WRKDIR} >> >> show-fake-size: >> - @if du -ks ${WRKINST} 2>/dev/null >${WRKINST}/wrkdir-size; then \ >> - cat ${WRKINST}/wrkdir-size && rm -f ${WRKINST}/wrkdir-size; \ >> - else \ >> - chmod -R u+rX ${WRKINST}; \ >> - du -ks ${WRKINST}; \ >> - fi >> + @${_PBUILD} du -ks ${WRKINST} >> >> verbose-show: >> .for _s in ${verbose-show} > > Well, I think you make sense.
Same here. > Better than the old code anyway. > > Okay ok jca@ -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
