Re: [pacman-dev] [PATCH v2] makepkg: print filenames and line numbers of references to $srcdir & $pkgdir in built package

[Ivy Foster]

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.fos...@gmail.com wrote:
> >>> From: Ivy Foster 
> >  > +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

Re: [pacman-dev] [PATCH v2] makepkg: print filenames and line numbers of references to $srcdir & $pkgdir in built package

[Allan McRae]

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.fos...@gmail.com wrote:
>>> From: Ivy Foster 
>  > +  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

Re: [pacman-dev] [PATCH v2] makepkg: print filenames and line numbers of references to $srcdir & $pkgdir in built package

[Ivy Foster]

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.fos...@gmail.com wrote:
> > From: Ivy Foster 
 > +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

Re: [pacman-dev] [PATCH v2] makepkg: print filenames and line numbers of references to $srcdir & $pkgdir in built package

[Andrew Gregory]

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.fos...@gmail.com wrote:
> From: Ivy Foster 
> 
> 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 
> ---
>  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