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

[Allan McRae]

Eric Bélanger wrote:
> On Tue, Nov 3, 2009 at 10:29 PM, Allan McRae <allan at archlinux.org> wrote:
>   
>> Eric Bélanger wrote:
>>     
>>> On Tue, Nov 3, 2009 at 7:34 PM, Eric Bélanger <snowmaniscool at gmail.com>
>>> wrote:
>>>
>>>       
>>>> On Tue, Nov 3, 2009 at 6:46 PM, Allan McRae <allan at archlinux.org> wrote:
>>>>
>>>>         
>>>>> Eric Bélanger wrote:
>>>>>
>>>>>           
>>>>>> On Tue, Nov 3, 2009 at 8:55 AM, Eric Bélanger <snowmaniscool at gmail.com>
>>>>>> wrote:
>>>>>>
>>>>>>
>>>>>>             
>>>>>>> On Tue, Nov 3, 2009 at 3:13 AM, Ray Rashif <schivmeister at gmail.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>>
>>>>>>>               
>>>>>>>> 2009/11/3 Eric Bélanger <snowmaniscool at gmail.com>
>>>>>>>>
>>>>>>>>     This is really convenient, but would it not be good if the
>>>>>>>> symlink(s) are
>>>>>>>> removed upon --clean?
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>                 
>>>>>>> Sure, that can be easily done.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>               
>>>>>> I'm not sure if removing the symlinks if --clean is used is a good
>>>>>> idea after all.  After a successful build, you would probably want to
>>>>>> have the package's symlink to be still there so you can test/install
>>>>>> the package.
>>>>>>
>>>>>>
>>>>>>             
>>>>> I agree that keeping the current symlink is good, but then do you have
>>>>> to
>>>>> remove old symlinks manually?  I think this is a situation with no best
>>>>> answer, but removing symlinks on --clean may be the better one.
>>>>>
>>>>>           
>>>> I've haven't thought about old symlinks. I'll remove them on --clean.
>>>>
>>>>
>>>>         
>>>>> And here is another thought I just had.  Do we want to error out if the
>>>>> symlinnk creation fails but the building of the package is successful?
>>>>>  Or
>>>>> jsut print a warning?
>>>>>
>>>>>
>>>>>           
>>>> Maybe a warning would be better.
>>>>
>>>>
>>>>         
>>> I added a warning. BTW, should the tar_file and pkg_file be local
>>> variables?  I'll submit anew patch once I get an answer.
>>>
>>>       
>> Yes they should.
>>
>> Allan
>>
>>     
>
> Here's the latest patch. I hope everything is correct.
>
> BTW, do you want me to continue sending patches as attachment too?  I
> see that 'git send-email' doesn't so I guess you can grab the inline
> patch or get it directly from git.  Is that correct?
>   

The way I handle this is to just send them directly with git send-email 
using the "In reply to" value to keep the thread... um... threaded.  Any 
comments about the patch and what you have changed to fix things can go 
under the "---" after the sign-off.  Of course, you can inline them as 
you have and push them to a git repo somewhere and we can just pull from 
there.

Also, can you insert newlines into you commit message to keep them at 
about 80 characters per line.

>
> Signed-off-by: Eric Bélanger <snowmaniscool at gmail.com>
> ---
>  scripts/makepkg.sh.in |   26 +++++++++++++++++++++-----
>  1 files changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> index 92b0454..aaf576b 100644
> --- a/scripts/makepkg.sh.in
> +++ b/scripts/makepkg.sh.in
> @@ -138,11 +138,17 @@ clean_up() {
>  		if [ -n "$pkgbase" ]; then
>  			# Can't do this unless the BUILDSCRIPT has been sourced.
>  			rm -f "${pkgbase}-${pkgver}-${pkgrel}-${CARCH}-build.log"*
> +			[[ "$PKGDEST" != "${startdir}" ]] \
> +			    && rm -f "${pkgbase}-${pkgver}-${pkgrel}-${CARCH}${PKGEXT}"
>  			if [ "$PKGFUNC" -eq 1 ]; then
>  				rm -f "${pkgbase}-${pkgver}-${pkgrel}-${CARCH}-package.log"*
> +				[[ "$PKGDEST" != "${startdir}" ]] \
> +				    && rm -f "${pkgbase}-${pkgver}-${pkgrel}-${CARCH}${PKGEXT}"
>  			elif [ "$SPLITPKG" -eq 1 ]; then
>  				for pkg in ${pkgname[@]}; do
>  					rm -f "${pkg}-${pkgver}-${pkgrel}-${CARCH}-package.log"*
> +					[[ "$PKGDEST" != "${startdir}" ]] \
> +					    && rm -f "${pkg}-${pkgver}-${pkgrel}-${CARCH}${PKGEXT}"
>  				done
>  			fi
>  		fi

I am not happy with this way of cleaning up the symlinks to packages.  
If the package is not a split package, then the package name will be 
$pkgname-...   and not $pkgbase (although, they are likely the same 
thing).  I am also not sure what the second clean-up (in the "$PKGFUNC 
-eq 1" test) is doing that has not already by the one above it. 

However, thinking about this more, we do not remove old packages when 
using --clean so why remove symlinks to old packages.  Only when package 
symlinks are pointing to packages that no longer exist in PKGDEST have 
we made a mess that needs cleaned up.  So how about something like:

for pkg in ${pkgname[@]}; do
  for file in ${pkg}-*-*-${CARCH}${PKGEXT}; do
    if [[ -h $file & ! -e $file ]]; then
      rm -f $file
    fi
  done
fi


Everything else in the patch is fine.

Allan