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

[Allan McRae]

Allan McRae wrote:
> Eric Bélanger wrote:
>> On Tue, Nov 3, 2009 at 7:36 AM, Allan McRae <allan at archlinux.org> wrote:
>>  
>>> 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}" 
>>>>
>>>>
>>>>       
>>> I'll clarify an earlier comment.  For the proposed patches for the 
>>> package
>>> signing work, there is some lines like
>>>
>>> tar_file="$PKGDEST/${nameofpkg}-${pkgver}-${pkgrel}-${PKGARCH}${EXT}"
>>> pkg_file="$PKGDEST/${nameofpkg}-${pkgver}-${pkgrel}-${PKGARCH}${PKGEXT}" 
>>>
>>>
>>> (or at least there will be once Dan adjusts those patches...)  So I 
>>> would
>>> prefer to keep just these two variables as that is all that is 
>>> needed and
>>> the symlink becomes:
>>>
>>> ln -sf "${pkg_file}" "${pkg_file/$PKGDEST/$startdir}"
>>>
>>>     
>>
>> OK.  But you do realize that the current pkg_file is different than
>> your new pkg_file (it's in fact your  tar_file).  So I'll need to
>> rename the references of pkg_file to tar_file.
>
> Yes, that is a bit more in depth than just the quick change...   How 
> about, I make a quick patch changing pkg_file to tar_file (as it 
> points to the tar file...) and adding the new pkg_file (that points to 
> the pkg file...) as the base for your patch and the gpg patch 
> changes.  I can do that first thing tomorrow morning (12pm here)

In fact, looking at this, you only need to change pkg_file to tar_file 
in four places so I am sure you can handle this.

Allan