[pacman-dev] [PATCH] makepkg: check for references to build root in package
Add a check that the package does not contain references to the folder it was built in. Fixes FS#14751 Signed-off-by: Allan McRae <allan@archlinux.org> --- 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 820d81f..a5b8f87 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -971,6 +971,12 @@ check_package() { warning "$(gettext "Invalid backup entry : %s")" "$file" fi done + + # check for references to the build directory + grep -R "${srcdir}" "${pkgdir}" &>/dev/null + if [ $? -eq 0 ]; then + warning "$(gettext "Package contains reference to %s")" "\$srcdir" + fi } create_package() { -- 1.6.5.1
Allan McRae wrote:
I wonder why you do not use if grep -R "${srcdir}" "${pkgdir}" &>/dev/null; then Is there a special cause for that or is it only for cosmetic reasons? Thanks.
Cedric Staniewski wrote:
No reason.... I was going to adjust my patch to this syntax. However, I have discovered, the patch causes makepkg to halt if there are no files in ${pkgdir}. This is entirely possible with something like "meta-packages" which just contain a list of deps. So I am going to need to use some sort of "grep ... || ret=" syntax anyway. Allan
Add a check that the package does not contain references to the folder it was built in. Fixes FS#14751 Signed -off-by: Allan McRae <allan@archlinux.org> --- scripts/makepkg.sh.in | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 66667f4..dd5f951 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -972,6 +972,13 @@ check_package() { warning "$(gettext "Invalid backup entry : %s")" "$file" fi done + + # check for references to the build directory + local ret=0 + grep -R "${srcdir}" "${pkgdir}" &>/dev/null || ret=1 + if [ $ret -eq 0 ]; then + warning "$(gettext "Package contains reference to %s")" "\$srcdir" + fi } create_package() { -- 1.6.5.1
Allan McRae wrote:
I tried to reproduce this, but was not able to do so. Patch: ----------------- diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 8812407..4bcb6b2 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -947,9 +947,7 @@ check_package() { done # check for references to the build directory - local ret=0 - grep -R "${srcdir}" "${pkgdir}" &>/dev/null || ret=1 - if [ $ret -eq 0 ]; then + if grep -R "${srcdir}" "${pkgdir}" &>/dev/null; then warning "$(gettext "Package contains reference to %s")" "\$srcdir" fi } ----------------- PKGBUILD: ----------------- pkgname=dummy pkgver=1 pkgrel=1 pkgdesc="A dummy package" arch=('any') url="http://dummy.xyz" license=('GPL') build() { echo nothing to see here #echo "$srcdir" > "$pkgdir/bla" } ------------------ Anyway... never mind.
On Sun, Oct 25, 2009 at 7:26 PM, Cedric Staniewski <cedric@gmx.ca> wrote:
Ah this is what I forgot to ask Allan today. I wanted to tell him I didn't understand why grep would get stuck, and why this other syntax would change anything about it.
Xavier wrote:
I have no idea.... Yesterday when trying the "if grep" syntax, makepkg would just stop directly before outputing "Creating package". I assumed it was something due to "set -e". Strange thing is, I can not replicate it today... even after pulling the stashed changes I had saved so I could go back and figure it out. I might be going mad... I have adjusted the patch to use Cedric's syntax now that it is working for me. Allan
On Mon, Oct 26, 2009 at 09:08:21AM +1000, Allan McRae wrote:
I have adjusted the patch to use Cedric's syntax now that it is working for me.
FWIW, you can also make the line a tad shorter by using grep's -q instead of &>/dev/null. -- Jeff My other computer is an abacus.
On 10/28/2009 10:07 PM, Jeff wrote:
I noticed this patch (latest revision[1]) has not made it into master yet... [1] http://projects.archlinux.org/users/allan/pacman.git/commit/?id=ce40da51611b...
On Wed, Dec 2, 2009 at 12:17 PM, Cedric Staniewski <cedric@gmx.ca> wrote:
I'm not convinced on this one as it has to scan the entire pkg/ directory, which for some things, could be gigantic (icc, go-openoffice, etc.). Am I being over-performance crazy? I guess the zip is going to do it too... To me this seems way outside the realm of makepkg and belongs in namcap. -Dan
Dan McGee wrote:
The problem with this is that namcap does not know anything about $srcdir (and how can it) so cannot check for references to it. This is unlike the check for missing backup files that is already in check_package() which should probably be in namcap... I also tested this in large packages (e.g. openoffice) and on my system (2GHz) it takes ~10sec which is a very small fraction of the build time. As an aside, extracting and grepping through the entire core+extra+some of community takes me ~10min. Per package this is a tiny amount of time and includes extraction of tarball and not having files in the cache. Allan
participants (5)
-
Allan McRae
-
Cedric Staniewski
-
Dan McGee
-
Jeff
-
Xavier