[pacman-dev] [PATCH] makepkg: Place packages symlinks in build dir when DESTDIR is used

[Cedric Staniewski]

Eric Bélanger wrote:
> On Tue, Nov 3, 2009 at 12:59 AM, Allan McRae <allan at archlinux.org> wrote:
>> Dan McGee wrote:
>>> On Mon, Nov 2, 2009 at 11:00 PM, Eric Bélanger <snowmaniscool at gmail.com>
>>> wrote:
>>>
>>>> When DESTDIR is used, symlinks to the packages will be put in the build
>>>> directory.  This combines the convenience of a global package cache with the
>>>> ease of having a package (i.e. a symlink) in the build directory for testing
>>>> and installation purpose.
>>>>
>>> This seems really reasonable. It gets a +1 from me for the idea; I
>>> didn't test the code or anything though.
>>>
>>>
>>>> Signed-off-by: Eric Bélanger <snowmaniscool at gmail.com>
>>>> ---
>>>>  scripts/makepkg.sh.in |    6 ++++++
>>>>  1 files changed, 6 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
>>>> index 92b0454..dc9124b 100644
>>>> --- a/scripts/makepkg.sh.in
>>>> +++ b/scripts/makepkg.sh.in
>>>> @@ -1000,6 +1000,8 @@ create_package() {
>>>>               "$PKGEXT" ; EXT=$PKGEXT ;;
>>>>       esac
>>>>       local
>>>> pkg_file="$PKGDEST/${nameofpkg}-${pkgver}-${pkgrel}-${PKGARCH}${EXT}"
>>>> +       local
>>>> pkglinks_name="${startdir}/${nameofpkg}-${pkgver}-${pkgrel}-${PKGARCH}${PKGEXT}"
>>>> +       local
>>>> pkglinks_target="$PKGDEST/${nameofpkg}-${pkgver}-${pkgrel}-${PKGARCH}${PKGEXT}"
>>>>
>>> Since we repeat the same
>>> "${nameofpkg}-${pkgver}-${pkgrel}-${PKGARCH}${PKGEXT}" part three
>>> times, we should really pull that into a local variable and then set
>>> the three others as necessary.
>>>
> 
> Actually it was repeated twice (EXT vs. PKGEXT).  I removed one of my
> new variables. See new patch (inline and attached).
> 
>> On the gpg branch, there is (going to be...) a patch creating "tar_file"
>> (=current pkg_file)  and pkg_file (pkglinks_target here).  I think we can
>> get away with just those two varabiles...  see below.
>>
>>>>       local ret=0
>>>>
>>>> @@ -1018,6 +1020,10 @@ create_package() {
>>>>               ret=$?
>>>>       fi
>>>>
>>>> +       if [ $ret -eq 0 ]; then
>>>> +               [ ! -f "${pkglinks_name}" ] && ln -s "${pkglinks_target}"
>>>> "${pkglinks_name}" && ret=$?
>>>>
>>> Will this work with err traps? I thought you need to be explicit with
>>> the if; then; fi blocks.
>>>
> 
> I don't really know how err traps works but I've made the if statement
> explicit as it seemed that's what you prefered.
> 
>> How about testing if $PKGDEST is the same as $startdir and then make the
>> symlink using
>> ln -s ${pkg_file} ${pkgfile/$PKGDEST/$startdir/}
>>
> 
> Thanks. I did that in my new patch.  I also added the -f option to ln.
>  So if you switch to undefined PKGDEST to defined PKGDEST, the package
> in the build directory will be overwritten by the symlink instead of
> having the ln error out.
> 
> Eric
> 
> =======================
> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> index 92b0454..e2b28c1 100644
> --- a/scripts/makepkg.sh.in
> +++ b/scripts/makepkg.sh.in
> @@ -1000,6 +1000,7 @@ create_package() {
>  		"$PKGEXT" ; EXT=$PKGEXT ;;
>  	esac
>  	local pkg_file="$PKGDEST/${nameofpkg}-${pkgver}-${pkgrel}-${PKGARCH}${EXT}"
> +	local pkglinks_target="${startdir}/${nameofpkg}-${pkgver}-${pkgrel}-${PKGARCH}${PKGEXT}"
> 
>  	local ret=0
> 
> @@ -1018,6 +1019,13 @@ create_package() {
>  		ret=$?
>  	fi
> 
> +	if [ $ret -eq 0 ]; then
> +		if [ "$PKGDEST" != "${startdir}" ]; then
> +			ln -sf "${pkg_file/$EXT/$PKGEXT}" "$pkglinks_target"
> +		fi
> +		ret=$?
> +	fi
> +


I really like your idea of having a symlink to the package. Regarding the patch, I think we should stick with the "new" coding style and also merge the two if statements; could look like this:

if (( ! ret )) && [[ $PKGDEST != $startdir ]]; then
	ln -sf "${pkg_file/$EXT/$PKGEXT}" "$pkglinks_target"
	ret=$?
fi


>  	if [ $ret -ne 0 ]; then
>  		error "$(gettext "Failed to create package file.")"
>  		exit 1 # TODO: error code
> 
> 
> ------------------------------------------------------------------------
> 
>