[pacman-dev] [PATCH] makepkg: print filenames and line numbers of references to $srcdir & $pkgdir in built package
From: Ivy Foster <ivy.foster@gmail.com> grep -n output is used to match format of compiler warnings. Since rewriting build_references() anyway, tweaked quoting. Implements FS#31558. Signed-off-by: Ivy Foster <ivy.foster@gmail.com> --- scripts/libmakepkg/lint_package/build_references.sh.in | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/scripts/libmakepkg/lint_package/build_references.sh.in b/scripts/libmakepkg/lint_package/build_references.sh.in index 67c14e6..32554b2 100644 --- a/scripts/libmakepkg/lint_package/build_references.sh.in +++ b/scripts/libmakepkg/lint_package/build_references.sh.in @@ -29,10 +29,17 @@ source "$LIBRARY/util/message.sh" lint_package_functions+=('warn_build_references') warn_build_references() { - if find "${pkgdir}" -type f -print0 | xargs -0 grep -q -I "${srcdir}" ; then - warning "$(gettext "Package contains reference to %s")" "\$srcdir" + local srcdir_refs pkgdir_refs + + mapfile -t srcdir_refs < <(find "$pkgdir" -type f -print0 | xargs -0 grep -n "$srcdir") + mapfile -t pkgdir_refs < <(find "$pkgdir" -type f -print0 | xargs -0 grep -n "$pkgdirbase") + + if [[ ${#srcdir_refs} -gt 0 ]]; then + warning "$(gettext 'Package contains reference to %s')" '$srcdir' + printf '%s\n' "${srcdir_refs[@]}" >&2 fi - if find "${pkgdir}" -type f -print0 | xargs -0 grep -q -I "${pkgdirbase}" ; then - warning "$(gettext "Package contains reference to %s")" "\$pkgdir" + if [[ ${#pkgdir_refs} -gt 0 ]]; then + warning "$(gettext 'Package contains reference to %s')" '$pkgdir' + printf '%s\n' "${pkgdir_refs[@]}" >&2 fi } -- 2.10.0
From: Ivy Foster <ivy.foster@gmail.com> grep -n output is used to match format of compiler warnings. Since rewriting build_references() anyway, tweaked quoting. Implements FS#31558. Signed-off-by: Ivy Foster <ivy.foster@gmail.com> --- scripts/libmakepkg/lint_package/build_references.sh.in | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/scripts/libmakepkg/lint_package/build_references.sh.in b/scripts/libmakepkg/lint_package/build_references.sh.in index 67c14e6..62f705c 100644 --- a/scripts/libmakepkg/lint_package/build_references.sh.in +++ b/scripts/libmakepkg/lint_package/build_references.sh.in @@ -25,14 +25,16 @@ LIBRARY=${LIBRARY:-'@libmakepkgdir@'} source "$LIBRARY/util/message.sh" - lint_package_functions+=('warn_build_references') warn_build_references() { - if find "${pkgdir}" -type f -print0 | xargs -0 grep -q -I "${srcdir}" ; then - warning "$(gettext "Package contains reference to %s")" "\$srcdir" - fi - if find "${pkgdir}" -type f -print0 | xargs -0 grep -q -I "${pkgdirbase}" ; then - warning "$(gettext "Package contains reference to %s")" "\$pkgdir" - fi + local refs + + for var in srcdir pkgdir; do + mapfile -t refs < <(find "$pkgdir" -type f -exec grep -n "${!var}" {} +) + if [[ ${#refs} -gt 0 ]]; then + warning "$(gettext 'Package contains reference to %s')" "\$$var" + printf '%s\n' "${refs[@]}" >&2 + fi + done } -- 2.10.0
We're not very strict about this, but please try to keep the first line of the commit message fairly short, ideally around 50 characters. On 10/14/16 at 08:21pm, ivy.foster@gmail.com wrote:
From: Ivy Foster <ivy.foster@gmail.com>
grep -n output is used to match format of compiler warnings. Since rewriting build_references() anyway, tweaked quoting. Implements FS#31558.
Signed-off-by: Ivy Foster <ivy.foster@gmail.com> --- scripts/libmakepkg/lint_package/build_references.sh.in | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/scripts/libmakepkg/lint_package/build_references.sh.in b/scripts/libmakepkg/lint_package/build_references.sh.in index 67c14e6..62f705c 100644 --- a/scripts/libmakepkg/lint_package/build_references.sh.in +++ b/scripts/libmakepkg/lint_package/build_references.sh.in @@ -25,14 +25,16 @@ LIBRARY=${LIBRARY:-'@libmakepkgdir@'}
source "$LIBRARY/util/message.sh"
- lint_package_functions+=('warn_build_references')
warn_build_references() { - if find "${pkgdir}" -type f -print0 | xargs -0 grep -q -I "${srcdir}" ; then - warning "$(gettext "Package contains reference to %s")" "\$srcdir" - fi - if find "${pkgdir}" -type f -print0 | xargs -0 grep -q -I "${pkgdirbase}" ; then - warning "$(gettext "Package contains reference to %s")" "\$pkgdir" - fi + local refs + + for var in srcdir pkgdir; do + mapfile -t refs < <(find "$pkgdir" -type f -exec grep -n "${!var}" {} +)
For packages with just a single file this won't print the name of the matching file. GNU grep has -H for this, but it's not POSIX. What about including /dev/null as an argument to grep so that it always has more than one file? Also, unrelated to this patch, but we should be using grep -F since paths can contain regular expression metacharacters.
+ if [[ ${#refs} -gt 0 ]]; then + warning "$(gettext 'Package contains reference to %s')" "\$$var" + printf '%s\n' "${refs[@]}" >&2 + fi + done } -- 2.10.0
On 14 Oct 2016, at 10:07 pm -0400, Andrew Gregory wrote:
> We're not very strict about this, but please try to keep the first
> line of the commit message fairly short, ideally around 50 characters.
Noted, thanks.
> On 10/14/16 at 08:21pm, ivy.foster@gmail.com wrote:
> > From: Ivy Foster <ivy.foster@gmail.com>
> + mapfile -t refs < <(find "$pkgdir" -type f -exec grep -n "${!var}" {} +)
> For packages with just a single file this won't print the name of the
> matching file. GNU grep has -H for this, but it's not POSIX. What
> about including /dev/null as an argument to grep so that it always has
> more than one file?
Ah, you're quite right. I'll fiddle with that and resubmit tomorrow, then.
Ivy
On 15/10/16 12:27, Ivy Foster wrote:
> On 14 Oct 2016, at 10:07 pm -0400, Andrew Gregory wrote:
>> We're not very strict about this, but please try to keep the first
>> line of the commit message fairly short, ideally around 50 characters.
>
> Noted, thanks.
>
>> On 10/14/16 at 08:21pm, ivy.foster@gmail.com wrote:
>>> From: Ivy Foster <ivy.foster@gmail.com>
> > + mapfile -t refs < <(find "$pkgdir" -type f -exec grep -n "${!var}" {} +)
>
>> For packages with just a single file this won't print the name of the
>> matching file. GNU grep has -H for this, but it's not POSIX. What
>> about including /dev/null as an argument to grep so that it always has
>> more than one file?
>
> Ah, you're quite right. I'll fiddle with that and resubmit tomorrow, then.
>
Why no use -l?
-l, --files-with-matches print only names of FILEs containing matches
A
On 15 Oct 2016, at 6:52 pm +1000, Allan McRae wrote:
> On 15/10/16 12:27, Ivy Foster wrote:
> > On 14 Oct 2016, at 10:07 pm -0400, Andrew Gregory wrote:
> >> On 10/14/16 at 08:21pm, ivy.foster@gmail.com wrote:
> >>> From: Ivy Foster <ivy.foster@gmail.com>
> > > + mapfile -t refs < <(find "$pkgdir" -type f -exec grep -n "${!var}" {} +)
> >
> >> For packages with just a single file this won't print the name of the
> >> matching file. GNU grep has -H for this, but it's not POSIX. What
> >> about including /dev/null as an argument to grep so that it always has
> >> more than one file?
> >
> > Ah, you're quite right. I'll fiddle with that and resubmit tomorrow, then.
> >
>
> Why no use -l?
>
> -l, --files-with-matches print only names of FILEs containing matches
I can do that instead, if you prefer. Personally, I wanted to use -n
rather than -l, because it matches the format of compiler warnings
(for ease of opening directly to the right line in your editor). Using
-l would have the benefit of being less verbose, I suppose.
Ivy
From: Ivy Foster <ivy.foster@gmail.com> Since rewriting build_references() anyway, tweaked quoting. Implements FS#31558. Signed-off-by: Ivy Foster <ivy.foster@gmail.com> --- scripts/libmakepkg/lint_package/build_references.sh.in | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/scripts/libmakepkg/lint_package/build_references.sh.in b/scripts/libmakepkg/lint_package/build_references.sh.in index 67c14e6..b9958c8 100644 --- a/scripts/libmakepkg/lint_package/build_references.sh.in +++ b/scripts/libmakepkg/lint_package/build_references.sh.in @@ -25,14 +25,16 @@ LIBRARY=${LIBRARY:-'@libmakepkgdir@'} source "$LIBRARY/util/message.sh" - lint_package_functions+=('warn_build_references') warn_build_references() { - if find "${pkgdir}" -type f -print0 | xargs -0 grep -q -I "${srcdir}" ; then - warning "$(gettext "Package contains reference to %s")" "\$srcdir" - fi - if find "${pkgdir}" -type f -print0 | xargs -0 grep -q -I "${pkgdirbase}" ; then - warning "$(gettext "Package contains reference to %s")" "\$pkgdir" - fi + local refs + + for var in srcdir pkgdir; do + mapfile -t refs < <(find "$pkgdir" -type f -exec grep -l "${!var}" {} +) + if [[ ${#refs} -gt 0 ]]; then + warning "$(gettext 'Package contains reference to %s')" "\$$var" + printf '%s\n' "${refs[@]}" >&2 + fi + done } -- 2.10.0
On 16/10/16 09:34, ivy.foster@gmail.com wrote:
From: Ivy Foster <ivy.foster@gmail.com>
Since rewriting build_references() anyway, tweaked quoting. Implements FS#31558.
Signed-off-by: Ivy Foster <ivy.foster@gmail.com> --- scripts/libmakepkg/lint_package/build_references.sh.in | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/scripts/libmakepkg/lint_package/build_references.sh.in b/scripts/libmakepkg/lint_package/build_references.sh.in index 67c14e6..b9958c8 100644 --- a/scripts/libmakepkg/lint_package/build_references.sh.in +++ b/scripts/libmakepkg/lint_package/build_references.sh.in @@ -25,14 +25,16 @@ LIBRARY=${LIBRARY:-'@libmakepkgdir@'}
source "$LIBRARY/util/message.sh"
- lint_package_functions+=('warn_build_references')
warn_build_references() { - if find "${pkgdir}" -type f -print0 | xargs -0 grep -q -I "${srcdir}" ; then - warning "$(gettext "Package contains reference to %s")" "\$srcdir" - fi - if find "${pkgdir}" -type f -print0 | xargs -0 grep -q -I "${pkgdirbase}" ; then - warning "$(gettext "Package contains reference to %s")" "\$pkgdir" - fi + local refs + + for var in srcdir pkgdir; do + mapfile -t refs < <(find "$pkgdir" -type f -exec grep -l "${!var}" {} +) + if [[ ${#refs} -gt 0 ]]; then
Our style used (( )) style tests for number tests. So this will become if (( ${#refs} > 0 )); then I will make this change when I apply. Thanks, Allan
participants (4)
-
Allan McRae
-
Andrew Gregory
-
Ivy Foster
-
ivy.foster＠gmail.com