[pacman-dev] [PATCH] makepkg: check for references to build root in package

older
[pacman-dev] [GIT] The official...

Allan McRae

24 Oct 2009 24 Oct '09

2:54 p.m.

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 --- 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

Show replies by date

Cedric Staniewski

24 Oct 24 Oct

5:09 p.m.

Allan McRae wrote:

...

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 --- 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() {

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.

Allan McRae

25 Oct 25 Oct

2:15 a.m.

Cedric Staniewski wrote:

...

Allan McRae wrote:

...
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 --- 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() {

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.

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

Allan McRae

4:04 a.m.

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 --- 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

Cedric Staniewski

6:26 p.m.

Allan McRae wrote:

...

Cedric Staniewski wrote:

...
Allan McRae wrote:

...
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 --- 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() {

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.

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

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.

Xavier

6:33 p.m.

New subject: [pacman-dev] [PATCH] makepkg: check for references to build root in package

On Sun, Oct 25, 2009 at 7:26 PM, 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

I tried to reproduce this, but was not able to do so.

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.

Allan McRae

11:08 p.m.

New subject: [pacman-dev] [PATCH] makepkg: check for references to build root in package

Xavier wrote:

...

On Sun, Oct 25, 2009 at 7:26 PM, 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

I tried to reproduce this, but was not able to do so.

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.

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

Jeff

28 Oct 28 Oct

9:07 p.m.

New subject: [pacman-dev] [PATCH] makepkg: check for references to build root in package

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.

Cedric Staniewski

2 Dec 2 Dec

6:17 p.m.

New subject: [pacman-dev] [PATCH] makepkg: check for references to build root in package

On 10/28/2009 10:07 PM, Jeff wrote:

...

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.

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...

Dan McGee

3 Dec 3 Dec

12:50 a.m.

New subject: [pacman-dev] [PATCH] makepkg: check for references to build root in package

On Wed, Dec 2, 2009 at 12:17 PM, Cedric Staniewski wrote:

...

On 10/28/2009 10:07 PM, Jeff wrote:

...
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.

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...

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

Allan McRae

1:25 a.m.

New subject: [pacman-dev] [PATCH] makepkg: check for references to build root in package

Dan McGee wrote:

...

On Wed, Dec 2, 2009 at 12:17 PM, Cedric Staniewski wrote:

...
On 10/28/2009 10:07 PM, Jeff wrote:

...
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.

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...

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.

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

5251

Age (days ago)

5291

Last active (days ago)

List overview

Download

10 comments

5 participants

participants (5)

Allan McRae
Cedric Staniewski
Dan McGee
Jeff
Xavier