[gentoo-portage-dev] [PATCH] QA warning for files in /var/{cache,lib,lock,run}/ or /run/ (bug 493154)

[gentoo-portage-dev] [PATCH] QA warning for files in /var/{cache,lib,lock,run}/ or /run/ (bug 493154)

[SebastianLuther]

From: Sebastian Luther <SebastianLuther@gmx.de>

No warning will be issued if the directories are created, but are
left empty.

URL: https://bugs.gentoo.org/493154
---
 bin/misc-functions.sh | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/bin/misc-functions.sh b/bin/misc-functions.sh
index bd99def..9e1a5ee 100755
--- a/bin/misc-functions.sh
+++ b/bin/misc-functions.sh
@@ -242,6 +242,11 @@ install_qa_check() {
 		[[ -d ${ED}/$x ]] && f+="  $x\n"
 	done
 
+	# It's ok create these directories, but not to install into them. (bug 493154)
+	for x in var/cache var/lib var/lock var/run run ; do
+		[[ -d ${ED}/$x ]] && [[ $(find "${ED}/${x}" -prune -empty) = "" ]] && f+="  $x\n"
+	done
+
 	if [[ -n $f ]] ; then
 		eqawarn "QA Notice: This ebuild installs into the following deprecated directories:"
 		eqawarn
-- 
1.8.3.2

Re: [gentoo-portage-dev] [PATCH] QA warning for files in /var/{cache,lib,lock,run}/ or /run/ (bug 493154)

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 1366 bytes --]

On Thursday 05 December 2013 15:57:17 SebastianLuther@gmx.de wrote:
> --- a/bin/misc-functions.sh
> +++ b/bin/misc-functions.sh
> @@ -242,6 +242,11 @@ install_qa_check() {
>  		[[ -d ${ED}/$x ]] && f+="  $x\n"
>  	done
> 
> +	# It's ok create these directories, but not to install into them. (bug
> 493154)

our style uses #12345 rather than (bug 12345)
# It's ok create these directories, but not to install into them. #493154

> +	for x in var/cache var/lib var/lock var/run run ; do
> +		[[ -d ${ED}/$x ]] && [[ $(find "${ED}/${x}" -prune -empty) = "" ]] &&
> f+="  $x\n"

the -d check doesn't handle symlinks correctly ... baselayout would install 
/var/run -> /run, and the -d would deref that to the real symlink.  however, 
`find` will not descend into the symlink, so we end up being saved by that.

we have -z for detecting empty output rather than comparing to an empty string

non-builtin vars should use braces, so that'd be ${x}.  i know some of the 
code in here doesn't follow that, but we should be marching in that direction 
with new code.

>  	if [[ -n $f ]] ; then
>  		eqawarn "QA Notice: This ebuild installs into the following 
deprecated directories:"

this warning is incorrect for these paths.  we need to create a new warning 
that clearly explains what is going on.

i'll send out a new version.
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[gentoo-portage-dev] [PATCH v2] QA warning for files in /var/{cache,lock,run}/ or /run/

[Mike Frysinger]

From: Sebastian Luther <SebastianLuther@gmx.de>

No warning will be issued if the directories are created, but are
left empty.

URL: https://bugs.gentoo.org/493154
---
 bin/misc-functions.sh | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/bin/misc-functions.sh b/bin/misc-functions.sh
index bd99def..2c4d248 100755
--- a/bin/misc-functions.sh
+++ b/bin/misc-functions.sh
@@ -241,13 +241,30 @@ install_qa_check() {
 	for x in etc/app-defaults usr/man usr/info usr/X11R6 usr/doc usr/locale ; do
 		[[ -d ${ED}/$x ]] && f+="  $x\n"
 	done
-
 	if [[ -n $f ]] ; then
 		eqawarn "QA Notice: This ebuild installs into the following deprecated directories:"
 		eqawarn
 		eqawarn "$f"
 	fi
 
+	# It's ok create these directories, but not to install into them. #493154
+	# TODO: We should add var/lib to this list.
+	f=
+	for x in var/cache var/lock var/run run ; do
+		if [[ ! -L ${ED}/${x} && -d ${ED}/${x} ]] ; then
+			if [[ -z $(find "${ED}/${x}" -prune -empty) ]] ; then
+				f+=$(cd "${ED}"; find "${x}" -printf '  %p\n')
+			fi
+		fi
+	done
+	if [[ -n ${f} ]] ; then
+		eqawarn "QA Notice: This ebuild installs into paths that should be created at runtime."
+		eqawarn " To fix, simply do not install into these directories.  Instead, your package"
+		eqawarn " should create dirs on the fly at runtime as needed via init scripts/etc..."
+		eqawarn
+		eqawarn "${f}"
+	fi
+
 	set +f
 	f=
 	for x in "${ED}etc/udev/rules.d/"* "${ED}lib"*"/udev/rules.d/"* ; do
-- 
1.8.4.3

Re: [gentoo-portage-dev] [PATCH] QA warning for files in /var/{cache,lib,lock,run}/ or /run/ (bug 493154)

[Sebastian Luther]

Am 11.12.2013 08:57, schrieb Mike Frysinger:
> On Thursday 05 December 2013 15:57:17 SebastianLuther@gmx.de
> wrote:
>> --- a/bin/misc-functions.sh +++ b/bin/misc-functions.sh @@ -242,6
>> +242,11 @@ install_qa_check() { [[ -d ${ED}/$x ]] && f+="  $x\n" 
>> done
>> 
>> +	# It's ok create these directories, but not to install into
>> them. (bug 493154)
> 
> our style uses #12345 rather than (bug 12345) # It's ok create
> these directories, but not to install into them. #493154
> 
>> +	for x in var/cache var/lib var/lock var/run run ; do +		[[ -d
>> ${ED}/$x ]] && [[ $(find "${ED}/${x}" -prune -empty) = "" ]] && 
>> f+="  $x\n"
> 
> the -d check doesn't handle symlinks correctly ... baselayout would
> install /var/run -> /run, and the -d would deref that to the real
> symlink.  however, `find` will not descend into the symlink, so we
> end up being saved by that.
> 
> we have -z for detecting empty output rather than comparing to an
> empty string
> 
> non-builtin vars should use braces, so that'd be ${x}.  i know some
> of the code in here doesn't follow that, but we should be marching
> in that direction with new code.

My bash knowledge is rather limited. So, thanks for your comments.

> 
>> if [[ -n $f ]] ; then eqawarn "QA Notice: This ebuild installs
>> into the following
> deprecated directories:"
> 
> this warning is incorrect for these paths.  we need to create a new
> warning that clearly explains what is going on.
> 

Right, thanks for fixing.

> i'll send out a new version. -mike
> 

FWIW, new patch looks fine.


Sebastian

Re: [gentoo-portage-dev] [PATCH] QA warning for files in /var/{cache,lib,lock,run}/ or /run/ (bug 493154)

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 239 bytes --]

On Wednesday 11 December 2013 03:43:12 Sebastian Luther wrote:
> Am 11.12.2013 08:57, schrieb Mike Frysinger:
> > i'll send out a new version.
> 
> FWIW, new patch looks fine.

thanks for the review ... i pushed that version
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]