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.
Better than the old code anyway.
Okay
