[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:
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
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.
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}" 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
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.
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.
+ fi + if [ $ret -ne 0 ]; then error "$(gettext "Failed to create package file.")" exit 1 # TODO: error code -- 1.6.5.2
Dan McGee wrote:
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.
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}"
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.
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.
How about testing if $PKGDEST is the same as $startdir and then make the symlink using ln -s ${pkg_file} ${pkgfile/$PKGDEST/$startdir/}
+ 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:59 AM, Allan McRae <allan@archlinux.org> wrote:
Dan McGee wrote:
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.
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}"
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 + 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>
Dan McGee wrote:
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.
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
+ local
On Tue, Nov 3, 2009 at 12:59 AM, Allan McRae <allan@archlinux.org> wrote: pkglinks_name="${startdir}/${nameofpkg}-${pkgver}-${pkgrel}-${PKGARCH}${PKGEXT}" 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 + if [ $ret -ne 0 ]; then error "$(gettext "Failed to create package file.")" exit 1 # TODO: error code -- 1.6.5.2
This is really convenient, but would it not be good if the symlink(s) are removed upon --clean?
Eric Bélanger wrote:
On Tue, Nov 3, 2009 at 12:59 AM, Allan McRae <allan@archlinux.org> wrote:
Dan McGee wrote:
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.
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}"
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
------------------------------------------------------------------------
Eric Bélanger wrote:
On Tue, Nov 3, 2009 at 12:59 AM, Allan McRae <allan@archlinux.org> wrote:
Dan McGee wrote:
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.
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}"
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}"
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
------------------------------------------------------------------------
On Tue, Nov 3, 2009 at 3:13 AM, Ray Rashif <schivmeister@gmail.com> wrote:
2009/11/3 Eric Bélanger <snowmaniscool@gmail.com>
Dan McGee wrote:
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.
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
+ local
On Tue, Nov 3, 2009 at 12:59 AM, Allan McRae <allan@archlinux.org> wrote: pkglinks_name="${startdir}/${nameofpkg}-${pkgver}-${pkgrel}-${PKGARCH}${PKGEXT}" 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 + if [ $ret -ne 0 ]; then error "$(gettext "Failed to create package file.")" exit 1 # TODO: error code -- 1.6.5.2
This is really convenient, but would it not be good if the symlink(s) are removed upon --clean?
Sure, that can be easily done.
On Tue, Nov 3, 2009 at 7:26 AM, Cedric Staniewski <cedric@gmx.ca> wrote:
Eric Bélanger wrote:
On Tue, Nov 3, 2009 at 12:59 AM, Allan McRae <allan@archlinux.org> wrote:
Dan McGee wrote:
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.
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
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:
On Tue, Nov 3, 2009 at 7:26 AM, Cedric Staniewski <cedric@gmx.ca> wrote:
Eric Bélanger wrote:
On Tue, Nov 3, 2009 at 12:59 AM, Allan McRae <allan@archlinux.org> wrote:
Dan McGee wrote:
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.
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
I was just following the current style of the function I was working in. Allan, Dan: is the above coding style OK with you?
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
On Tue, Nov 3, 2009 at 7:36 AM, Allan McRae <allan@archlinux.org> wrote:
Eric Bélanger wrote:
On Tue, Nov 3, 2009 at 12:59 AM, Allan McRae <allan@archlinux.org> wrote:
Dan McGee wrote:
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.
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}"
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.
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 ------------------------------------------------------------------------
Eric Bélanger wrote:
On Tue, Nov 3, 2009 at 7:36 AM, Allan McRae <allan@archlinux.org> wrote:
Eric Bélanger wrote:
On Tue, Nov 3, 2009 at 12:59 AM, Allan McRae <allan@archlinux.org> wrote:
Dan McGee wrote:
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.
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}"
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) Allan
Allan McRae wrote:
Eric Bélanger wrote:
On Tue, Nov 3, 2009 at 7:36 AM, Allan McRae <allan@archlinux.org> wrote:
Eric Bélanger wrote:
On Tue, Nov 3, 2009 at 12:59 AM, Allan McRae <allan@archlinux.org> wrote:
Dan McGee wrote:
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.
> 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}" > > > > 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
On Tue, Nov 3, 2009 at 9:20 AM, Allan McRae <allan@archlinux.org> wrote:
Allan McRae wrote:
Eric Bélanger 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.
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
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:
On Tue, Nov 3, 2009 at 9:20 AM, Allan McRae <allan@archlinux.org> wrote:
Allan McRae wrote:
Eric Bélanger 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.
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
Yes, I'll do the change. Don't worry about it. I just wanted to make sure we were on the same page.
Do I need to do anything? :P -Dan
On Tue, Nov 3, 2009 at 8:55 AM, Eric Bélanger <snowmaniscool@gmail.com> wrote:
On Tue, Nov 3, 2009 at 3:13 AM, Ray Rashif <schivmeister@gmail.com> wrote:
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?
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.
Eric Bélanger wrote:
On Tue, Nov 3, 2009 at 8:55 AM, Eric Bélanger <snowmaniscool@gmail.com> wrote:
On Tue, Nov 3, 2009 at 3:13 AM, Ray Rashif <schivmeister@gmail.com> wrote:
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?
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. 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:
Eric Bélanger wrote:
On Tue, Nov 3, 2009 at 8:55 AM, Eric Bélanger <snowmaniscool@gmail.com> wrote:
On Tue, Nov 3, 2009 at 3:13 AM, Ray Rashif <schivmeister@gmail.com> wrote:
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?
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.
On Tue, Nov 3, 2009 at 7:34 PM, Eric Bélanger <snowmaniscool@gmail.com> wrote:
On Tue, Nov 3, 2009 at 6:46 PM, Allan McRae <allan@archlinux.org> wrote:
Eric Bélanger wrote:
On Tue, Nov 3, 2009 at 8:55 AM, Eric Bélanger <snowmaniscool@gmail.com> wrote:
On Tue, Nov 3, 2009 at 3:13 AM, Ray Rashif <schivmeister@gmail.com> wrote:
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?
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.
Eric Bélanger wrote:
On Tue, Nov 3, 2009 at 7:34 PM, Eric Bélanger <snowmaniscool@gmail.com> wrote:
On Tue, Nov 3, 2009 at 6:46 PM, Allan McRae <allan@archlinux.org> wrote:
Eric Bélanger wrote:
On Tue, Nov 3, 2009 at 8:55 AM, Eric Bélanger <snowmaniscool@gmail.com> wrote:
On Tue, Nov 3, 2009 at 3:13 AM, Ray Rashif <schivmeister@gmail.com> wrote:
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?
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
On Tue, Nov 3, 2009 at 10:29 PM, Allan McRae <allan@archlinux.org> wrote:
Eric Bélanger wrote:
On Tue, Nov 3, 2009 at 7:34 PM, Eric Bélanger <snowmaniscool@gmail.com> wrote:
On Tue, Nov 3, 2009 at 6:46 PM, Allan McRae <allan@archlinux.org> wrote:
Eric Bélanger wrote:
On Tue, Nov 3, 2009 at 8:55 AM, Eric Bélanger <snowmaniscool@gmail.com> wrote:
On Tue, Nov 3, 2009 at 3:13 AM, Ray Rashif <schivmeister@gmail.com> wrote:
> > 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? > > > >
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? 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:
On Tue, Nov 3, 2009 at 10:29 PM, Allan McRae <allan@archlinux.org> wrote:
Eric Bélanger wrote:
On Tue, Nov 3, 2009 at 7:34 PM, Eric Bélanger <snowmaniscool@gmail.com> wrote:
On Tue, Nov 3, 2009 at 6:46 PM, Allan McRae <allan@archlinux.org> wrote:
Eric Bélanger wrote:
On Tue, Nov 3, 2009 at 8:55 AM, Eric Bélanger <snowmaniscool@gmail.com> wrote:
> On Tue, Nov 3, 2009 at 3:13 AM, Ray Rashif <schivmeister@gmail.com> > wrote: > > > >> 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? >> >> >> >> >> > 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@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
On Wed, Nov 4, 2009 at 1:45 AM, Allan McRae <allan@archlinux.org> wrote:
Eric Bélanger 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?
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.
OK. I'll do that. I was inlining them manually to keep everything in the same thread.
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@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 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.
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
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:
On Wed, Nov 4, 2009 at 1:45 AM, Allan McRae <allan@archlinux.org> wrote:
Eric Bélanger 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?
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.
OK. I'll do that. I was inlining them manually to keep everything in the same thread.
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@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 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.
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
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
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
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.
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:
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.
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
Thanks. I see it now. Eric
On Thu, Nov 5, 2009 at 1:13 AM, Eric Bélanger <snowmaniscool@gmail.com> wrote:
On Wed, Nov 4, 2009 at 6:44 PM, Laszlo Papp <djszapi@archlinux.us> wrote:
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.
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
Thanks. I see it now.
Eric
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:
On Thu, Nov 5, 2009 at 1:13 AM, Eric Bélanger <snowmaniscool@gmail.com> wrote:
On Wed, Nov 4, 2009 at 6:44 PM, Laszlo Papp <djszapi@archlinux.us> wrote:
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.
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
Thanks. I see it now.
Eric
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.
I had seen it. Thanks anyway.
On Thu, Nov 5, 2009 at 1:18 PM, Xavier <shiningxc@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>
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:
On Wed, Nov 4, 2009 at 2:37 AM, Eric Bélanger <snowmaniscool@gmail.com> wrote:
Eric Bélanger 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?
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. OK. I'll do that. I was inlining them manually to keep everything in
On Wed, Nov 4, 2009 at 1:45 AM, Allan McRae <allan@archlinux.org> wrote: the same thread.
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@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 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.
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
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
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> ---
Pushed to my working branch. I added a comment above the clean up of symlinks just to document what was happening. Allan
participants (7)
-
Allan McRae
-
Cedric Staniewski
-
Dan McGee
-
Eric Bélanger
-
Laszlo Papp
-
Ray Rashif
-
Xavier