| Message ID | 20180125183145.9296-1-lukeshu@lukeshu.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Jan 25, 2018 at 01:31:45PM -0500, Luke Shumaker wrote: > From: Luke Shumaker <lukeshu@parabola.nu> > > https://bugs.archlinux.org/task/57193 > > If the user mistakenly passes in some other (non-package, non-PKGBUILD) > file, instead of refusing to process it (saying > "Error: Cannot process ${filename}"), it will now try to parse it as a > PKGBUILD, probably encounter and print at least one error, but finally say > "Error: ${filename} is not a valid PKGBUILD". This is actually probably > *more* helpful to a confused user who passed in the wrong file. > --- > namcap.py | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/namcap.py b/namcap.py > index b62a2fa..fd62d3c 100755 > --- a/namcap.py > +++ b/namcap.py > @@ -248,9 +248,7 @@ for package in packages: > > if os.path.isfile(package) and tarfile.is_tarfile(package): > process_realpackage(package, active_modules) > - elif package.endswith('PKGBUILD'): > + else: Install files? Other assets? Doesn't this mean we assume they're PKGBUILDs? > process_pkgbuild(package, active_modules) > - else: > - print("Error: Cannot process %s" % package) > > # vim: set ts=4 sw=4 noet: > -- > 2.15.1
On Thu, 25 Jan 2018 13:50:19 -0500, Dave Reisner wrote: > On Thu, Jan 25, 2018 at 01:31:45PM -0500, Luke Shumaker wrote: > > From: Luke Shumaker <lukeshu@parabola.nu> > > > > https://bugs.archlinux.org/task/57193 > > > > If the user mistakenly passes in some other (non-package, non-PKGBUILD) > > file, instead of refusing to process it (saying > > "Error: Cannot process ${filename}"), it will now try to parse it as a > > PKGBUILD, probably encounter and print at least one error, but finally say > > "Error: ${filename} is not a valid PKGBUILD". This is actually probably > > *more* helpful to a confused user who passed in the wrong file. > > --- > > namcap.py | 4 +--- > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > diff --git a/namcap.py b/namcap.py > > index b62a2fa..fd62d3c 100755 > > --- a/namcap.py > > +++ b/namcap.py > > @@ -248,9 +248,7 @@ for package in packages: > > > > if os.path.isfile(package) and tarfile.is_tarfile(package): > > process_realpackage(package, active_modules) > > - elif package.endswith('PKGBUILD'): > > + else: > > Install files? Other assets? Doesn't this mean we assume they're > PKGBUILDs? Yes, it does. It's already the case that running namcap on those install files and other assets is an error. Though, this will make those errors much more... verbose. For example, running `namcap *` in pacman's package directory currently looks like: $ namcap * PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH PKGBUILD (pacman) W: Reference to x86_64 should be changed to $CARCH PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH PKGBUILD (pacman) W: Reference to x86_64 should be changed to $CARCH PKGBUILD (pacman) W: Reference to x86_64 should be changed to $CARCH PKGBUILD (pacman) W: Reference to x86_64 should be changed to $CARCH Error: Cannot process makepkg.conf Error: Cannot process pacman.conf.i686 Error: Cannot process pacman.conf.x86_64 But this patch changes that to: $ namcap * PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH PKGBUILD (pacman) W: Reference to x86_64 should be changed to $CARCH PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH PKGBUILD (pacman) W: Reference to x86_64 should be changed to $CARCH PKGBUILD (pacman) W: Reference to x86_64 should be changed to $CARCH PKGBUILD (pacman) W: Reference to x86_64 should be changed to $CARCH Error: error: invalid package file Error: makepkg.conf is not a valid PKGBUILD Error: error: invalid package file Error: pacman.conf.i686: line 9: [options]: command not found pacman.conf.i686: line 18: HoldPkg: command not found pacman.conf.i686: line 23: Architecture: command not found pacman.conf.i686: line 36: CheckSpace: command not found pacman.conf.i686: line 41: SigLevel: command not found pacman.conf.i686: line 42: LocalFileSigLevel: command not found pacman.conf.i686: line 75: [core]: command not found pacman.conf.i686: line 76: Include: command not found pacman.conf.i686: line 78: [extra]: command not found pacman.conf.i686: line 79: Include: command not found pacman.conf.i686: line 84: [community]: command not found pacman.conf.i686: line 85: Include: command not found Error: pacman.conf.i686 is not a valid PKGBUILD Error: error: invalid package file Error: pacman.conf.x86_64: line 9: [options]: command not found pacman.conf.x86_64: line 18: HoldPkg: command not found pacman.conf.x86_64: line 23: Architecture: command not found pacman.conf.x86_64: line 36: CheckSpace: command not found pacman.conf.x86_64: line 41: SigLevel: command not found pacman.conf.x86_64: line 42: LocalFileSigLevel: command not found pacman.conf.x86_64: line 75: [core]: command not found pacman.conf.x86_64: line 76: Include: command not found pacman.conf.x86_64: line 78: [extra]: command not found pacman.conf.x86_64: line 79: Include: command not found pacman.conf.x86_64: line 84: [community]: command not found pacman.conf.x86_64: line 85: Include: command not found Error: pacman.conf.x86_64 is not a valid PKGBUILD There is a possible safety concern. namcap will call `parsepkgbuild` on the file, which does evaluate the file as a bash script; it is conceivable that it this could accidentally do something bad, depending on what the file is. However, I don't believe this to be a legitimate concern because of the rbash restricted environment that it is executed in.
<div>Why not just use "-p" for explicitly specifying that we want to parse file as a PKGBUILD?</div><div><br /></div><div><br /></div><div>26.01.2018, 00:41, "Luke Shumaker" <lukeshu@lukeshu.com>:</div><blockquote type="cite"><p>On Thu, 25 Jan 2018 13:50:19 -0500,<br />Dave Reisner wrote:<br /></p><blockquote> On Thu, Jan 25, 2018 at 01:31:45PM -0500, Luke Shumaker wrote:<br /> > From: Luke Shumaker <<a href="mailto:lukeshu@parabola.nu">lukeshu@parabola.nu</a>><br /> > <br /> > <a href="https://bugs.archlinux.org/task/57193">https://bugs.archlinux.org/task/57193</a><br /> > <br /> > If the user mistakenly passes in some other (non-package, non-PKGBUILD)<br /> > file, instead of refusing to process it (saying<br /> > "Error: Cannot process ${filename}"), it will now try to parse it as a<br /> > PKGBUILD, probably encounter and print at least one error, but finally say<br /> > "Error: ${filename} is not a valid PKGBUILD". This is actually probably<br /> > *more* helpful to a confused user who passed in the wrong file.<br /> > ---<br /> > namcap.py | 4 +---<br /> > 1 file changed, 1 insertion(+), 3 deletions(-)<br /> > <br /> > diff --git a/namcap.py b/namcap.py<br /> > index b62a2fa..fd62d3c 100755<br /> > --- a/namcap.py<br /> > +++ b/namcap.py<br /> > @@ -248,9 +248,7 @@ for package in packages:<br /> > <br /> > if os.path.isfile(package) and tarfile.is_tarfile(package):<br /> > process_realpackage(package, active_modules)<br /> > - elif package.endswith('PKGBUILD'):<br /> > + else:<br /><br /> Install files? Other assets? Doesn't this mean we assume they're<br /> PKGBUILDs?<br /></blockquote><p><br />Yes, it does. It's already the case that running namcap on those<br />install files and other assets is an error.<br /><br />Though, this will make those errors much more... verbose.<br /><br />For example, running `namcap *` in pacman's package directory<br />currently looks like:<br /><br /> $ namcap *<br /> PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH<br /> PKGBUILD (pacman) W: Reference to x86_64 should be changed to $CARCH<br /> PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH<br /> PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH<br /> PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH<br /> PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH<br /> PKGBUILD (pacman) W: Reference to x86_64 should be changed to $CARCH<br /> PKGBUILD (pacman) W: Reference to x86_64 should be changed to $CARCH<br /> PKGBUILD (pacman) W: Reference to x86_64 should be changed to $CARCH<br /> Error: Cannot process makepkg.conf<br /> Error: Cannot process pacman.conf.i686<br /> Error: Cannot process pacman.conf.x86_64<br /><br />But this patch changes that to:<br /><br /> $ namcap *<br /> PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH<br /> PKGBUILD (pacman) W: Reference to x86_64 should be changed to $CARCH<br /> PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH<br /> PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH<br /> PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH<br /> PKGBUILD (pacman) W: Reference to i686 should be changed to $CARCH<br /> PKGBUILD (pacman) W: Reference to x86_64 should be changed to $CARCH<br /> PKGBUILD (pacman) W: Reference to x86_64 should be changed to $CARCH<br /> PKGBUILD (pacman) W: Reference to x86_64 should be changed to $CARCH<br /> Error: error: invalid package file<br /><br /> Error: makepkg.conf is not a valid PKGBUILD<br /> Error: error: invalid package file<br /><br /> Error: pacman.conf.i686: line 9: [options]: command not found<br /> pacman.conf.i686: line 18: HoldPkg: command not found<br /> pacman.conf.i686: line 23: Architecture: command not found<br /> pacman.conf.i686: line 36: CheckSpace: command not found<br /> pacman.conf.i686: line 41: SigLevel: command not found<br /> pacman.conf.i686: line 42: LocalFileSigLevel: command not found<br /> pacman.conf.i686: line 75: [core]: command not found<br /> pacman.conf.i686: line 76: Include: command not found<br /> pacman.conf.i686: line 78: [extra]: command not found<br /> pacman.conf.i686: line 79: Include: command not found<br /> pacman.conf.i686: line 84: [community]: command not found<br /> pacman.conf.i686: line 85: Include: command not found<br /><br /> Error: pacman.conf.i686 is not a valid PKGBUILD<br /> Error: error: invalid package file<br /><br /> Error: pacman.conf.x86_64: line 9: [options]: command not found<br /> pacman.conf.x86_64: line 18: HoldPkg: command not found<br /> pacman.conf.x86_64: line 23: Architecture: command not found<br /> pacman.conf.x86_64: line 36: CheckSpace: command not found<br /> pacman.conf.x86_64: line 41: SigLevel: command not found<br /> pacman.conf.x86_64: line 42: LocalFileSigLevel: command not found<br /> pacman.conf.x86_64: line 75: [core]: command not found<br /> pacman.conf.x86_64: line 76: Include: command not found<br /> pacman.conf.x86_64: line 78: [extra]: command not found<br /> pacman.conf.x86_64: line 79: Include: command not found<br /> pacman.conf.x86_64: line 84: [community]: command not found<br /> pacman.conf.x86_64: line 85: Include: command not found<br /><br /> Error: pacman.conf.x86_64 is not a valid PKGBUILD<br /><br />There is a possible safety concern. namcap will call `parsepkgbuild`<br />on the file, which does evaluate the file as a bash script; it is<br />conceivable that it this could accidentally do something bad,<br />depending on what the file is. However, I don't believe this to be a<br />legitimate concern because of the rbash restricted environment that it<br />is executed in.<br /><br /></p><span>-- <br />Happy hacking,<br />~ Luke Shumaker<br /></span></blockquote>
diff --git a/namcap.py b/namcap.py index b62a2fa..fd62d3c 100755 --- a/namcap.py +++ b/namcap.py @@ -248,9 +248,7 @@ for package in packages: if os.path.isfile(package) and tarfile.is_tarfile(package): process_realpackage(package, active_modules) - elif package.endswith('PKGBUILD'): + else: process_pkgbuild(package, active_modules) - else: - print("Error: Cannot process %s" % package) # vim: set ts=4 sw=4 noet:
From: Luke Shumaker <lukeshu@parabola.nu> https://bugs.archlinux.org/task/57193 If the user mistakenly passes in some other (non-package, non-PKGBUILD) file, instead of refusing to process it (saying "Error: Cannot process ${filename}"), it will now try to parse it as a PKGBUILD, probably encounter and print at least one error, but finally say "Error: ${filename} is not a valid PKGBUILD". This is actually probably *more* helpful to a confused user who passed in the wrong file. --- namcap.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)