The checking of the package for $srcdir references was overly sensitive and gave a lot of what appear to be false positives with binary files (in particular with debugging symbols kept).
Restrict the search for $srcdir to non-binary files as this should still catch the majority of configuration issues the check was initially designed to catch. Also, add a similar check for $pkgdir.
Signed-off-by: Allan McRae allan@archlinux.org --- scripts/makepkg.sh.in | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index ed1380d..01d73f8 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -972,10 +972,15 @@ check_package() { fi done
- # check for references to the build directory - if find "${pkgdir}" -type f -exec grep -q "${srcdir}" {} +; then + # check for references to the build and package directory + local filelist=$(find "${pkgdir}" -type f) + if grep -q -I "${srcdir}" $filelist; then warning "$(gettext "Package contains reference to %s")" "$srcdir" fi + if grep -q -I "${pkgdir}" $filelist; then + warning "$(gettext "Package contains reference to %s")" "$pkgdir" + fi + }
create_package() {
Am Mittwoch, den 29.09.2010, 21:59 +1000 schrieb Allan McRae:
The checking of the package for $srcdir references was overly sensitive and gave a lot of what appear to be false positives with binary files (in particular with debugging symbols kept).
Restrict the search for $srcdir to non-binary files as this should still catch the majority of configuration issues the check was initially designed to catch. Also, add a similar check for $pkgdir.
Signed-off-by: Allan McRae allan@archlinux.org
scripts/makepkg.sh.in | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index ed1380d..01d73f8 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -972,10 +972,15 @@ check_package() { fi done
- # check for references to the build directory
- if find "${pkgdir}" -type f -exec grep -q "${srcdir}" {} +; then
- # check for references to the build and package directory
- local filelist=$(find "${pkgdir}" -type f)
- if grep -q -I "${srcdir}" $filelist; then warning "$(gettext "Package contains reference to %s")" "$srcdir" fi
- if grep -q -I "${pkgdir}" $filelist; then
warning "$(gettext "Package contains reference to %s")" "\$pkgdir"- fi
}
create_package() {
This change could exeed the maximum number of allowed params for large packages. It would be better to continue to use the exec param for find IMO.
Marc
On 29/09/10 22:06, Marc - A. Dahlhaus wrote:
Am Mittwoch, den 29.09.2010, 21:59 +1000 schrieb Allan McRae:
The checking of the package for $srcdir references was overly sensitive and gave a lot of what appear to be false positives with binary files (in particular with debugging symbols kept).
Restrict the search for $srcdir to non-binary files as this should still catch the majority of configuration issues the check was initially designed to catch. Also, add a similar check for $pkgdir.
Signed-off-by: Allan McRaeallan@archlinux.org
scripts/makepkg.sh.in | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index ed1380d..01d73f8 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -972,10 +972,15 @@ check_package() { fi done
- # check for references to the build directory
- if find "${pkgdir}" -type f -exec grep -q "${srcdir}" {} +; then
# check for references to the build and package directory
local filelist=$(find "${pkgdir}" -type f)
if grep -q -I "${srcdir}" $filelist; then warning "$(gettext "Package contains reference to %s")" "$srcdir" fi
if grep -q -I "${pkgdir}" $filelist; then
warning "$(gettext "Package contains reference to %s")" "\$pkgdir"fi
}
create_package() {
This change could exeed the maximum number of allowed params for large packages. It would be better to continue to use the exec param for find IMO.
That is a good point. I did not want to run the find twice but I guess that will be mostly cached anyway so it will make little difference. I will make this change on my working branch.
Allan
On 29/09/10 13:35, Allan McRae wrote:
On 29/09/10 22:06, Marc - A. Dahlhaus wrote:
Am Mittwoch, den 29.09.2010, 21:59 +1000 schrieb Allan McRae:
The checking of the package for $srcdir references was overly sensitive and gave a lot of what appear to be false positives with binary files (in particular with debugging symbols kept).
Restrict the search for $srcdir to non-binary files as this should still catch the majority of configuration issues the check was initially designed to catch. Also, add a similar check for $pkgdir.
Signed-off-by: Allan McRaeallan@archlinux.org
scripts/makepkg.sh.in | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index ed1380d..01d73f8 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -972,10 +972,15 @@ check_package() { fi done
- # check for references to the build directory
- if find "${pkgdir}" -type f -exec grep -q "${srcdir}" {} +; then
- # check for references to the build and package directory
- local filelist=$(find "${pkgdir}" -type f)
- if grep -q -I "${srcdir}" $filelist; then
warning "$(gettext "Package contains reference to %s")" "$srcdir" fi
- if grep -q -I $filelist; then
- warning "$(gettext "Package contains reference to %s")" "$pkgdir"
- fi
}
create_package() {
This change could exeed the maximum number of allowed params for large packages. It would be better to continue to use the exec param for find IMO.
That is a good point. I did not want to run the find twice but I guess that will be mostly cached anyway so it will make little difference. I will make this change on my working branch.
Allan
just pipe it through xargs, something like `find "${pkgdir}" -type f -print0 | xargs -0 grep -m 1 -q "${pkgdir}"`
it's a lot faster than calling grep over and over. I added the `-m 1` so it stops searching after the first match.
On 29/09/10 22:41, Nathan Wayde wrote:
On 29/09/10 13:35, Allan McRae wrote:
On 29/09/10 22:06, Marc - A. Dahlhaus wrote:
Am Mittwoch, den 29.09.2010, 21:59 +1000 schrieb Allan McRae:
The checking of the package for $srcdir references was overly sensitive and gave a lot of what appear to be false positives with binary files (in particular with debugging symbols kept).
Restrict the search for $srcdir to non-binary files as this should still catch the majority of configuration issues the check was initially designed to catch. Also, add a similar check for $pkgdir.
Signed-off-by: Allan McRaeallan@archlinux.org
scripts/makepkg.sh.in | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index ed1380d..01d73f8 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -972,10 +972,15 @@ check_package() { fi done
- # check for references to the build directory
- if find "${pkgdir}" -type f -exec grep -q "${srcdir}" {} +; then
- # check for references to the build and package directory
- local filelist=$(find "${pkgdir}" -type f)
- if grep -q -I "${srcdir}" $filelist; then
warning "$(gettext "Package contains reference to %s")" "$srcdir" fi
- if grep -q -I $filelist; then
- warning "$(gettext "Package contains reference to %s")" "$pkgdir"
- fi
}
create_package() {
This change could exeed the maximum number of allowed params for large packages. It would be better to continue to use the exec param for find IMO.
That is a good point. I did not want to run the find twice but I guess that will be mostly cached anyway so it will make little difference. I will make this change on my working branch.
Allan
just pipe it through xargs, something like `find "${pkgdir}" -type f -print0 | xargs -0 grep -m 1 -q "${pkgdir}"`
Doesn't that suffer from the same issue with potentially exceeding the maximum number of parameters?
it's a lot faster than calling grep over and over. I added the `-m 1` so it stops searching after the first match.
Check out what the -q flag does... :P
Allan
On Wed, Sep 29, 2010 at 7:51 AM, Allan McRae allan@archlinux.org wrote:
On 29/09/10 22:41, Nathan Wayde wrote:
On 29/09/10 13:35, Allan McRae wrote:
On 29/09/10 22:06, Marc - A. Dahlhaus wrote:
Am Mittwoch, den 29.09.2010, 21:59 +1000 schrieb Allan McRae:
The checking of the package for $srcdir references was overly sensitive and gave a lot of what appear to be false positives with binary files (in particular with debugging symbols kept).
Restrict the search for $srcdir to non-binary files as this should still catch the majority of configuration issues the check was initially designed to catch. Also, add a similar check for $pkgdir.
Signed-off-by: Allan McRaeallan@archlinux.org
scripts/makepkg.sh.in | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index ed1380d..01d73f8 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -972,10 +972,15 @@ check_package() { fi done
- # check for references to the build directory
- if find "${pkgdir}" -type f -exec grep -q "${srcdir}" {} +; then
- # check for references to the build and package directory
- local filelist=$(find "${pkgdir}" -type f)
- if grep -q -I "${srcdir}" $filelist; then
warning "$(gettext "Package contains reference to %s")" "$srcdir" fi
- if grep -q -I $filelist; then
- warning "$(gettext "Package contains reference to %s")" "$pkgdir"
- fi
}
create_package() {
This change could exeed the maximum number of allowed params for large packages. It would be better to continue to use the exec param for find IMO.
That is a good point. I did not want to run the find twice but I guess that will be mostly cached anyway so it will make little difference. I will make this change on my working branch.
Allan
just pipe it through xargs, something like `find "${pkgdir}" -type f -print0 | xargs -0 grep -m 1 -q "${pkgdir}"`
Doesn't that suffer from the same issue with potentially exceeding the maximum number of parameters?
No, the entire purpose of xargs is to split up the input so no command line maximums are exceeded.
-Dan
On 29/09/10 13:51, Allan McRae wrote:
[...]
just pipe it through xargs, something like `find "${pkgdir}" -type f -print0 | xargs -0 grep -m 1 -q "${pkgdir}"`
Doesn't that suffer from the same issue with potentially exceeding the maximum number of parameters?
no, the list is passed in via the pipe as opposed to actual argument, xargs then spits them out in smaller numbers
it's a lot faster than calling grep over and over. I added the `-m 1` so it stops searching after the first match.
Check out what the -q flag does... :P
duh *facepalm*
Allan
On 30/09/10 01:35, Allan McRae wrote:
On 29/09/10 22:06, Marc - A. Dahlhaus wrote:
Am Mittwoch, den 29.09.2010, 21:59 +1000 schrieb Allan McRae:
The checking of the package for $srcdir references was overly sensitive and gave a lot of what appear to be false positives with binary files (in particular with debugging symbols kept).
Restrict the search for $srcdir to non-binary files as this should still catch the majority of configuration issues the check was initially designed to catch. Also, add a similar check for $pkgdir.
Signed-off-by: Allan McRaeallan@archlinux.org
scripts/makepkg.sh.in | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index ed1380d..01d73f8 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -972,10 +972,15 @@ check_package() { fi done
- # check for references to the build directory
- if find "${pkgdir}" -type f -exec grep -q "${srcdir}" {} +; then
- # check for references to the build and package directory
- local filelist=$(find "${pkgdir}" -type f)
- if grep -q -I "${srcdir}" $filelist; then warning "$(gettext "Package contains reference to %s")"
"$srcdir" fi
- if grep -q -I "${pkgdir}" $filelist; then
warning "$(gettext "Package contains reference to %s")""$pkgdir"
fi
}
create_package() {
This change could exeed the maximum number of allowed params for large packages. It would be better to continue to use the exec param for find IMO.
That is a good point. I did not want to run the find twice but I guess that will be mostly cached anyway so it will make little difference. I will make this change on my working branch.
Allan
Why not just run it once? I would do something like this:
find "${pkgdir}" -type f -exec grep -q -I -F "${srcdir} ${pkgdir}" {} +;
This also means special characters (like .) won't get misinterpreted, however unlikely that may be. Not sure if there is a better way to stick that newline in there...
On 29/09/10 23:31, Jonathan Conder wrote:
On 30/09/10 01:35, Allan McRae wrote:
On 29/09/10 22:06, Marc - A. Dahlhaus wrote:
Am Mittwoch, den 29.09.2010, 21:59 +1000 schrieb Allan McRae:
The checking of the package for $srcdir references was overly sensitive and gave a lot of what appear to be false positives with binary files (in particular with debugging symbols kept).
Restrict the search for $srcdir to non-binary files as this should still catch the majority of configuration issues the check was initially designed to catch. Also, add a similar check for $pkgdir.
Signed-off-by: Allan McRaeallan@archlinux.org
scripts/makepkg.sh.in | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index ed1380d..01d73f8 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -972,10 +972,15 @@ check_package() { fi done
- # check for references to the build directory
- if find "${pkgdir}" -type f -exec grep -q "${srcdir}" {} +; then
- # check for references to the build and package directory
- local filelist=$(find "${pkgdir}" -type f)
- if grep -q -I "${srcdir}" $filelist; then
warning "$(gettext "Package contains reference to %s")" "$srcdir" fi
- if grep -q -I "${pkgdir}" $filelist; then
- warning "$(gettext "Package contains reference to %s")" "$pkgdir"
- fi
}
create_package() {
This change could exeed the maximum number of allowed params for large packages. It would be better to continue to use the exec param for find IMO.
That is a good point. I did not want to run the find twice but I guess that will be mostly cached anyway so it will make little difference. I will make this change on my working branch.
Allan
Why not just run it once? I would do something like this:
find "${pkgdir}" -type f -exec grep -q -I -F "${srcdir} ${pkgdir}" {} +;
This also means special characters (like .) won't get misinterpreted, however unlikely that may be. Not sure if there is a better way to stick that newline in there...
I did not want to run them at the same time to keep the error message informative about what was found.
Allan
On Wed 29 Sep 2010 21:59 +1000, Allan McRae wrote:
The checking of the package for $srcdir references was overly sensitive and gave a lot of what appear to be false positives with binary files (in particular with debugging symbols kept).
Restrict the search for $srcdir to non-binary files as this should still catch the majority of configuration issues the check was initially designed to catch. Also, add a similar check for $pkgdir.
Just curious. Shouldn't these checks really be part of namcap rather than makepkg?
On 30/09/10 11:56, Loui Chang wrote:
On Wed 29 Sep 2010 21:59 +1000, Allan McRae wrote:
The checking of the package for $srcdir references was overly sensitive and gave a lot of what appear to be false positives with binary files (in particular with debugging symbols kept).
Restrict the search for $srcdir to non-binary files as this should still catch the majority of configuration issues the check was initially designed to catch. Also, add a similar check for $pkgdir.
Just curious. Shouldn't these checks really be part of namcap rather than makepkg?
How would namcap know where a package was built?
On Thu 30 Sep 2010 12:10 +1000, Allan McRae wrote:
On 30/09/10 11:56, Loui Chang wrote:
On Wed 29 Sep 2010 21:59 +1000, Allan McRae wrote:
The checking of the package for $srcdir references was overly sensitive and gave a lot of what appear to be false positives with binary files (in particular with debugging symbols kept).
Restrict the search for $srcdir to non-binary files as this should still catch the majority of configuration issues the check was initially designed to catch. Also, add a similar check for $pkgdir.
Just curious. Shouldn't these checks really be part of namcap rather than makepkg?
How would namcap know where a package was built?
I haven't thought of the implementation. How would you implement it?
I wondered since namcap is the package checking tool that it should have such functionality rather than makepkg itself.
Perhaps makepkg could hand things off to namcap if the packager wishes to check the package for any issues.
On Wed, Sep 29, 2010 at 9:26 PM, Loui Chang louipc.ist@gmail.com wrote:
On Thu 30 Sep 2010 12:10 +1000, Allan McRae wrote:
On 30/09/10 11:56, Loui Chang wrote:
On Wed 29 Sep 2010 21:59 +1000, Allan McRae wrote:
The checking of the package for $srcdir references was overly sensitive and gave a lot of what appear to be false positives with binary files (in particular with debugging symbols kept).
Restrict the search for $srcdir to non-binary files as this should still catch the majority of configuration issues the check was initially designed to catch. Also, add a similar check for $pkgdir.
Just curious. Shouldn't these checks really be part of namcap rather than makepkg?
How would namcap know where a package was built?
I haven't thought of the implementation. How would you implement it?
I think he was asking you the same. :)
I wondered since namcap is the package checking tool that it should have such functionality rather than makepkg itself.
Perhaps makepkg could hand things off to namcap if the packager wishes to check the package for any issues.
This has come up several times, if we could check it in namcap, we would. It is nothing more than a warning printed to your screen so there really isn't much to it.
Your suggestion would tie the two tools a lot closer together than they are now, and would still let these things go unnoticed if makepkg and namcap were run at differing times in the packaging process.
-Dan
On 30/09/10 12:26, Loui Chang wrote:
On Thu 30 Sep 2010 12:10 +1000, Allan McRae wrote:
On 30/09/10 11:56, Loui Chang wrote:
On Wed 29 Sep 2010 21:59 +1000, Allan McRae wrote:
The checking of the package for $srcdir references was overly sensitive and gave a lot of what appear to be false positives with binary files (in particular with debugging symbols kept).
Restrict the search for $srcdir to non-binary files as this should still catch the majority of configuration issues the check was initially designed to catch. Also, add a similar check for $pkgdir.
Just curious. Shouldn't these checks really be part of namcap rather than makepkg?
How would namcap know where a package was built?
I haven't thought of the implementation. How would you implement it?
I wondered since namcap is the package checking tool that it should have such functionality rather than makepkg itself.
Perhaps makepkg could hand things off to namcap if the packager wishes to check the package for any issues.
There are reasons not to have makepkg pass stuff directly to namcap. Primarily, I do not have (or want) python in my clean chroots so namcap would not run there. The only other way would be to put a reference to the build root somewhere in the .PKGINFO file so that namcap could read it in and check for it but I do not like the idea of putting that sort of stuff there.
Overall, I agree that makepkg should not do much package checking, but this is something best suited to being in makepkg. I would definitely not like the checks performed by makepkg to unnecessarily expand beyond anything that can be done in 1 or 2 lines of bash...
Allan
On Thu 30 Sep 2010 12:38 +1000, Allan McRae wrote:
On 30/09/10 12:26, Loui Chang wrote:
On Thu 30 Sep 2010 12:10 +1000, Allan McRae wrote:
On 30/09/10 11:56, Loui Chang wrote:
On Wed 29 Sep 2010 21:59 +1000, Allan McRae wrote:
The checking of the package for $srcdir references was overly sensitive and gave a lot of what appear to be false positives with binary files (in particular with debugging symbols kept).
Restrict the search for $srcdir to non-binary files as this should still catch the majority of configuration issues the check was initially designed to catch. Also, add a similar check for $pkgdir.
Just curious. Shouldn't these checks really be part of namcap rather than makepkg?
How would namcap know where a package was built?
I haven't thought of the implementation. How would you implement it?
I wondered since namcap is the package checking tool that it should have such functionality rather than makepkg itself.
Perhaps makepkg could hand things off to namcap if the packager wishes to check the package for any issues.
There are reasons not to have makepkg pass stuff directly to namcap. Primarily, I do not have (or want) python in my clean chroots so namcap would not run there. The only other way would be to put a reference to the build root somewhere in the .PKGINFO file so that namcap could read it in and check for it but I do not like the idea of putting that sort of stuff there.
I wasn't implying that package checking should be forced. Well, package checking via namcap should remain optional. Maybe checking for $srcdir references could also be optional. I don't know if putting a reference to the build root would be a good idea. I think it could be something passed directly to namcap immediately after building.
Overall, I agree that makepkg should not do much package checking, but this is something best suited to being in makepkg. I would definitely not like the checks performed by makepkg to unnecessarily expand beyond anything that can be done in 1 or 2 lines of bash...
Yep. I'm just brainstorming different possibilities. I'm glad to have feedback about it. Cheers.
On Fri, Oct 1, 2010 at 7:27 PM, Loui Chang louipc.ist@gmail.com wrote:
On Thu 30 Sep 2010 12:38 +1000, Allan McRae wrote:
On 30/09/10 12:26, Loui Chang wrote:
On Thu 30 Sep 2010 12:10 +1000, Allan McRae wrote:
On 30/09/10 11:56, Loui Chang wrote:
On Wed 29 Sep 2010 21:59 +1000, Allan McRae wrote:
The checking of the package for $srcdir references was overly sensitive and gave a lot of what appear to be false positives with binary files (in particular with debugging symbols kept).
Restrict the search for $srcdir to non-binary files as this should still catch the majority of configuration issues the check was initially designed to catch. Also, add a similar check for $pkgdir.
Just curious. Shouldn't these checks really be part of namcap rather than makepkg?
How would namcap know where a package was built?
I haven't thought of the implementation. How would you implement it?
I wondered since namcap is the package checking tool that it should have such functionality rather than makepkg itself.
Perhaps makepkg could hand things off to namcap if the packager wishes to check the package for any issues.
There are reasons not to have makepkg pass stuff directly to namcap. Primarily, I do not have (or want) python in my clean chroots so namcap would not run there. The only other way would be to put a reference to the build root somewhere in the .PKGINFO file so that namcap could read it in and check for it but I do not like the idea of putting that sort of stuff there.
I wasn't implying that package checking should be forced. Well, package checking via namcap should remain optional. Maybe checking for $srcdir references could also be optional. I don't know if putting a reference to the build root would be a good idea. I think it could be something passed directly to namcap immediately after building.
Overall, I agree that makepkg should not do much package checking, but this is something best suited to being in makepkg. I would definitely not like the checks performed by makepkg to unnecessarily expand beyond anything that can be done in 1 or 2 lines of bash...
Yep. I'm just brainstorming different possibilities. I'm glad to have feedback about it. Cheers.
One thought I did have, and this is just a thought, is to store $pkgdir and $srcdir in the built package in PKGINFO as a way to use these values later.
-Dan
On 02/10/10 02:08, Dan McGee wrote:
[...]
One thought I did have, and this is just a thought, is to store $pkgdir and $srcdir in the built package in PKGINFO as a way to use these values later.
-Dan
I would advise against storing it in the PKGINFO if it means that it ends up in the final built package's .PKGINFO because it has the potential to create an info leak, i.e if you built the pkg outside of a chroot the path is now exposed to everyone who gets a copy of that pkg.
pacman-dev@lists.archlinux.org