svn commit: r304594 - head/Mk

[Bryan Drewery]

On 9/21/2012 11:02 AM, poyopoyo at puripuri.plala.or.jp wrote:
> Hi Bryan,
> 
> At Fri, 21 Sep 2012 07:58:18 -0500,
> Bryan Drewery wrote:
>>
>> On 9/21/2012 1:51 AM, poyopoyo at puripuri.plala.or.jp wrote:
>>> At Thu, 20 Sep 2012 19:51:30 +0000 (UTC),
>>> Beat Gaetzi wrote:
>>>>   - Make 'do-package' atomic while creating the pkg file [4]
>>>
>>>> Modified: head/Mk/bsd.port.mk
>>>> ==============================================================================
>>>> @@ -3911,11 +3905,15 @@ do-package: ${TMPPLIST}
>>>>  			fi; \
>>>>  		fi; \
>>>>  	fi
>>>> -	@if ${PKG_CMD} -b ${PKGNAME} ${PKGFILE}; then \
>>>> +	@TMPPKGFILE=$$(mktemp -t pkg); \
>>>> +	trap "${RM} -f $${TMPPKGFILE} $${TMPPKGFILE}${PKG_SUFX}; exit 1" 1 2 3 5 10 13 15; \
>>>> +	if ${PKG_CMD} -b ${PKGNAME} $${TMPPKGFILE}; then \
>>>> +		${MV} -f $${TMPPKGFILE}${PKG_SUFX} ${PKGFILE}; \
>>>>  		if [ -d ${PACKAGES} ]; then \
>>>>  			cd ${.CURDIR} && eval ${MAKE} package-links; \
>>>>  		fi; \
>>>>  	else \
>>>> +		${RM} -f $${TMPPKGFILE}; \
>>>>  		cd ${.CURDIR} && eval ${MAKE} delete-package; \
>>>>  		exit 1; \
>>>>  	fi
>>>
>>> This does not seem to atomic at all. Well being visible when PKG_SUFX is set.
>>
>> How is a single mv(1) on a tempfile not atomic? The purpose here is to
>> avoid the very long pkg_create(1) call and avoid leaving an invalid
>> package behind on ctrl+c or other interruptions.
> 
> Because temporary file which mktemp creates is not referenced by anyone.
> 
>>>> +	@TMPPKGFILE=$$(mktemp -t pkg); \
> 
> TMPPKGFILE=pkg.XXXXXXXX (here, for example)
> 
>>>> +	if ${PKG_CMD} -b ${PKGNAME} $${TMPPKGFILE}; then \
>>>> +		${MV} -f $${TMPPKGFILE}${PKG_SUFX} ${PKGFILE}; \
> 
> pkg_create creates pkg.XXXXXXXX.tbz and mv it to ${PKGFILE}.
> ${TMPPKGFILE} left untouched. mv is atomic, but pkg_create does not.
> (While mktemp succeed to create pkg.XXXXXXXX, pkg.XXXXXXXX.tbz could be
> a symlink, a directory or something fancy)
> 
> At Fri, 21 Sep 2012 08:22:37 -0500,
> Bryan Drewery wrote:
>> I will also try creating the tempfile in the PKGREPOSITORY, to keep it
>> on the same filesystem :)
> 
> It's nice. I think an easiest way for me to fix this is mktemp -d to
> create own temporary directory and pkg_crate ${PKGNAME} under it, then
> mv it to ${PKGFILE}. Just a thought, BTW.
> 


I was thinking the same.

Mind testing this patch out?

http://people.freebsd.org/~bdrewery/patch-bsd.port.mk-do-package.txt

It will keep the same behavior of unrecognized PKG_SUFX for pkg_create,
not leave any temp files, keeps the tempdir on the save device, and
allows overriding PACKAGES/PKGREPOSITORY/PKGFILE all properly.


-- 
Regards,
Bryan Drewery
bdrewery at freenode/EFNet