[pacman-dev] [PATCH] makepkg: Place packages symlinks in build dir when DESTDIR is used
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. Signed-off-by: Eric Bélanger <snowmaniscool@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}" 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=$? + fi + if [ $ret -ne 0 ]; then error "$(gettext "Failed to create package file.")" exit 1 # TODO: error code -- 1.6.5.2
On Tue, Nov 3, 2009 at 12:00 AM, Eric Bélanger <snowmaniscool@gmail.com> wrote:
Of course, I meant PKGDEST instead of DESTDIR. ;) I've attached a new patch with fixed commit message. Eric
On Mon, Nov 2, 2009 at 11:00 PM, Eric Bélanger <snowmaniscool@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. the three others as necessary.
Will this work with err traps? I thought you need to be explicit with the if; then; fi blocks.
Dan McGee wrote:
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.
How about testing if $PKGDEST is the same as $startdir and then make the symlink using ln -s ${pkg_file} ${pkgfile/$PKGDEST/$startdir/}
On Tue, Nov 3, 2009 at 12:59 AM, Allan McRae <allan@archlinux.org> wrote:
Actually it was repeated twice (EXT vs. PKGEXT). I removed one of my new variables. See new patch (inline and attached).
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.
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 + if [ $ret -ne 0 ]; then error "$(gettext "Failed to create package file.")" exit 1 # TODO: error code -- 1.6.5.2
2009/11/3 Eric Bélanger <snowmaniscool@gmail.com>
This is really convenient, but would it not be good if the symlink(s) are removed upon --clean?
On Tue, Nov 3, 2009 at 3:13 AM, Ray Rashif <schivmeister@gmail.com> wrote:
Sure, that can be easily done.
On Tue, Nov 3, 2009 at 8:55 AM, Eric Bélanger <snowmaniscool@gmail.com> wrote:
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.
Eric Bélanger wrote:
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. 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? Allan
On Tue, Nov 3, 2009 at 6:46 PM, Allan McRae <allan@archlinux.org> wrote:
I've haven't thought about old symlinks. I'll remove them on --clean.
Maybe a warning would be better.
On Tue, Nov 3, 2009 at 7:34 PM, Eric Bélanger <snowmaniscool@gmail.com> wrote:
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.
On Tue, Nov 3, 2009 at 10:29 PM, Allan McRae <allan@archlinux.org> wrote:
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? Signed-off-by: Eric Bélanger <snowmaniscool@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 @@ -999,21 +1005,22 @@ create_package() { *) warning "$(gettext "'%s' is not a valid archive extension.")" \ "$PKGEXT" ; EXT=$PKGEXT ;; esac - local pkg_file="$PKGDEST/${nameofpkg}-${pkgver}-${pkgrel}-${PKGARCH}${EXT}" + local tar_file="$PKGDEST/${nameofpkg}-${pkgver}-${pkgrel}-${PKGARCH}${EXT}" + local pkg_file="$PKGDEST/${nameofpkg}-${pkgver}-${pkgrel}-${PKGARCH}${PKGEXT}" local ret=0 # when fileglobbing, we want * in an empty directory to expand to # the null string rather than itself shopt -s nullglob - bsdtar -cf - $comp_files * > "$pkg_file" || ret=$? + bsdtar -cf - $comp_files * > "$tar_file" || ret=$? shopt -u nullglob if [ $ret -eq 0 ]; then case "$PKGEXT" in - *tar.gz) gzip -f -n "$pkg_file" ;; - *tar.bz2) bzip2 -f "$pkg_file" ;; - *tar.xz) xz -z -f "$pkg_file" ;; + *tar.gz) gzip -f -n "$tar_file" ;; + *tar.bz2) bzip2 -f "$tar_file" ;; + *tar.xz) xz -z -f "$tar_file" ;; esac ret=$? fi @@ -1022,6 +1029,15 @@ create_package() { error "$(gettext "Failed to create package file.")" exit 1 # TODO: error code fi + + if (( ! ret )) && [[ "$PKGDEST" != "${startdir}" ]]; then + ln -sf "${pkg_file}" "${pkg_file/$PKGDEST/$startdir}" + ret=$? + fi + + if [ $ret -ne 0 ]; then + warning "$(gettext "Failed to create symlink to package file.")" + fi } create_srcpackage() { -- 1.6.5.2
Eric Bélanger wrote:
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.
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
On Wed, Nov 4, 2009 at 1:45 AM, Allan McRae <allan@archlinux.org> wrote:
OK. I'll do that. I was inlining them manually to keep everything in the same thread.
I was following (maybe too closely) the log files removal as template here. I'll change it to pkgname.
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.
Yeah, it probably can be removed.
Sure. I'll use this instead. I don't use the --clean option so I am indifferent. I'll send a new patch tomorrow. Eric
Everything else in the patch is fine.
Allan
On Wed, Nov 4, 2009 at 2:37 AM, Eric Bélanger <snowmaniscool@gmail.com> wrote:
Here's the new patch. I struggled getting git to work so hopefully it's OK. BTW, the 'git send-email' --in-reply-to option asks for an identifier. I can't seem to find it in the patches or email headers in gmail. Signed-off-by: Eric Bélanger <snowmaniscool@gmail.com> --- scripts/makepkg.sh.in | 28 +++++++++++++++++++++++----- 1 files changed, 23 insertions(+), 5 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 92b0454..02ebf16 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -145,6 +145,14 @@ clean_up() { rm -f "${pkg}-${pkgver}-${pkgrel}-${CARCH}-package.log"* done fi + + for pkg in ${pkgname[@]}; do + for file in ${pkg}-*-*-${CARCH}${PKGEXT}; do + if [[ -h $file && ! -e $file ]]; then + rm -f $file + fi + done + done fi fi @@ -999,21 +1007,22 @@ create_package() { *) warning "$(gettext "'%s' is not a valid archive extension.")" \ "$PKGEXT" ; EXT=$PKGEXT ;; esac - local pkg_file="$PKGDEST/${nameofpkg}-${pkgver}-${pkgrel}-${PKGARCH}${EXT}" + local tar_file="$PKGDEST/${nameofpkg}-${pkgver}-${pkgrel}-${PKGARCH}${EXT}" + local pkg_file="$PKGDEST/${nameofpkg}-${pkgver}-${pkgrel}-${PKGARCH}${PKGEXT}" local ret=0 # when fileglobbing, we want * in an empty directory to expand to # the null string rather than itself shopt -s nullglob - bsdtar -cf - $comp_files * > "$pkg_file" || ret=$? + bsdtar -cf - $comp_files * > "$tar_file" || ret=$? shopt -u nullglob if [ $ret -eq 0 ]; then case "$PKGEXT" in - *tar.gz) gzip -f -n "$pkg_file" ;; - *tar.bz2) bzip2 -f "$pkg_file" ;; - *tar.xz) xz -z -f "$pkg_file" ;; + *tar.gz) gzip -f -n "$tar_file" ;; + *tar.bz2) bzip2 -f "$tar_file" ;; + *tar.xz) xz -z -f "$tar_file" ;; esac ret=$? fi @@ -1022,6 +1031,15 @@ create_package() { error "$(gettext "Failed to create package file.")" exit 1 # TODO: error code fi + + if (( ! ret )) && [[ "$PKGDEST" != "${startdir}" ]]; then + ln -sf "${pkg_file}" "${pkg_file/$PKGDEST/$startdir}" + ret=$? + fi + + if [ $ret -ne 0 ]; then + warning "$(gettext "Failed to create symlink to package file.")" + fi } create_srcpackage() { -- 1.6.5.2
In gmail: At the top right corner of the mail for what you'd like to response, down arrow, Show Original, and you will see something similar: <7fcd249b0911041537o3ffec549ke1b9a6318f695994@mail.gmail.com> Best Regards, Laszlo Papp
On Wed, Nov 4, 2009 at 6:44 PM, Laszlo Papp <djszapi@archlinux.us> wrote:
Thanks. I see it now. Eric
On Thu, Nov 5, 2009 at 1:13 AM, Eric Bélanger <snowmaniscool@gmail.com> wrote:
Maybe it is obvious, but since Laszlo managed to miss the most important information, I want to clarify anyway. The important id is this header : Message-ID: <id> Then you can use 'git send-email' --in-reply-to to reply to that message.
On Thu, Nov 5, 2009 at 7:18 AM, Xavier <shiningxc@gmail.com> wrote:
I had seen it. Thanks anyway.
On Thu, Nov 5, 2009 at 1:18 PM, Xavier <shiningxc@gmail.com> wrote:
Yeah, I think so too, if we speak about Message-ID matter, it's very obvious :) Eric could solve the problem 1 day before this message, so I didn't understand how this message is actual, but yes thanks. Best Regards, Laszlo Papp
Eric Bélanger wrote:
Pushed to my working branch. I added a comment above the clean up of symlinks just to document what was happening. Allan
Eric Bélanger wrote:
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
On Tue, Nov 3, 2009 at 7:26 AM, Cedric Staniewski <cedric@gmx.ca> wrote:
I was just following the current style of the function I was working in. Allan, Dan: is the above coding style OK with you?
Eric Bélanger wrote:
Yes, there is going to be a massive patch integrated at some stage that converts [ style tests to [[ style tests. All patches waiting for makepkg are currently sitting on my working branch and I will rebase them and adjusts any tests to this style before they go into the main git repo. This just saves me work. I really should check how that patch is progressing... Allan
Eric Bélanger wrote:
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}"
On Tue, Nov 3, 2009 at 7:36 AM, Allan McRae <allan@archlinux.org> wrote:
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.
Eric Bélanger wrote:
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) Allan
Allan McRae wrote:
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
On Tue, Nov 3, 2009 at 9:20 AM, Allan McRae <allan@archlinux.org> wrote:
Yes, I'll do the change. Don't worry about it. I just wanted to make sure we were on the same page. Eric
On Tue, Nov 3, 2009 at 8:31 AM, Eric Bélanger <snowmaniscool@gmail.com> wrote:
Do I need to do anything? :P -Dan
participants (7)
-
Allan McRae
-
Cedric Staniewski
-
Dan McGee
-
Eric Bélanger
-
Laszlo Papp
-
Ray Rashif
-
Xavier