[gentoo-portage-dev] [PATCH 1/3] bin/misc-functions.sh: Introduce eqalog and eqawarnlog functions.

[gentoo-portage-dev] [PATCH 1/3] bin/misc-functions.sh: Introduce eqalog and eqawarnlog functions.

[Michael Palimaka]

These functions are to be used for creating a log of QA violations in a
machine-readable format.

Stored in ${T]/qa.log, the log consists of one entry per line in the
following format: violation-tag data

For example:
deprecated-directory /usr/man
deprecated-directory /usr/info
world-writable /var/db/foo/bar
---
 bin/misc-functions.sh | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/bin/misc-functions.sh b/bin/misc-functions.sh
index cc652a9..78da589 100755
--- a/bin/misc-functions.sh
+++ b/bin/misc-functions.sh
@@ -162,6 +162,22 @@ prepcompress() {
 	return 0
 }
 
+eqalog() {
+	local tag=$1 x
+	shift
+	for x in "$@" ; do
+		echo "${tag}" "${x}" >> "${T}"/qa.log
+	done
+}
+
+eqawarnlog() {
+	eqalog "$@"
+	shift
+	for x in "$@" ; do
+		eqawarn "  $x"
+	done
+}
+
 install_qa_check() {
 	local f i qa_var x
 	if ! ___eapi_has_prefix_variables; then
-- 
2.0.4

[gentoo-portage-dev] [PATCH 2/3] install-qa-check.d/05double-D: Write to log and improve consistency.

[Michael Palimaka]

Present the list of offending files newline-delimitered for consistency
with other checks.
---
 bin/install-qa-check.d/05double-D | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/bin/install-qa-check.d/05double-D b/bin/install-qa-check.d/05double-D
index 1634afd..84fcc89 100644
--- a/bin/install-qa-check.d/05double-D
+++ b/bin/install-qa-check.d/05double-D
@@ -2,9 +2,10 @@
 
 DD_check() {
 	if [[ -d ${D%/}${D} ]] ; then
+		eqawarn "QA Notice: files installed in \${D}/\${D}:"
 		local -i INSTALLTOD=0
 		while read -r -d $'\0' i ; do
-			eqawarn "QA Notice: /${i##${D%/}${D}} installed in \${D}/\${D}"
+			eqawarnlog double-d "/${i##${D%/}${D}}"
 			((INSTALLTOD++))
 		done < <(find "${D%/}${D}" -print0)
 		die "Aborting due to QA concerns: ${INSTALLTOD} files installed in ${D%/}${D}"
-- 
2.0.4

[gentoo-portage-dev] [PATCH 3/3] install-qa-check.d/90world-writable: Write log and general cleanup.

[Michael Palimaka]

Use eqawarn instead of __vecho for visibility.

Present the list of offending files newline-delimitered for consistency
with other checks.
---
 bin/install-qa-check.d/90world-writable | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/bin/install-qa-check.d/90world-writable b/bin/install-qa-check.d/90world-writable
index 771027e..ff186c5 100644
--- a/bin/install-qa-check.d/90world-writable
+++ b/bin/install-qa-check.d/90world-writable
@@ -2,21 +2,35 @@
 
 world_writable_check() {
 	# Now we look for all world writable files.
-	local unsafe_files=$(find "${ED}" -type f -perm -2 | sed -e "s:^${ED}:- :")
+	local unsafe_files=$(find "${ED}" -type f -perm -2 | sed -e "s:^${ED}:/:")
+	local OLDIFS x
+
+	OLDIFS=$IFS
+	IFS=$'\n'
+
 	if [[ -n ${unsafe_files} ]] ; then
-		__vecho "QA Security Notice: world writable file(s):"
-		__vecho "${unsafe_files}"
-		__vecho "- This may or may not be a security problem, most of the time it is one."
-		__vecho "- Please double check that $PF really needs a world writeable bit and file bugs accordingly."
-		sleep 1
+		eqawarn "QA Security Notice: world writable file(s):"
+
+		for x in $unsafe_files ; do
+			eqawarnlog world-writable $x
+		done
+
+		eqawarn "This may or may not be a security problem, most of the time it is one."
+		eqawarn "Please double check that $PF really needs a world writeable bit and file bugs accordingly."
+		eqawarn
 	fi
 
 	local unsafe_files=$(find "${ED}" -type f '(' -perm -2002 -o -perm -4002 ')' | sed -e "s:^${ED}:/:")
 	if [[ -n ${unsafe_files} ]] ; then
 		eqawarn "QA Notice: Unsafe files detected (set*id and world writable)"
-		eqawarn "${unsafe_files}"
+
+		for x in $unsafe_files ; do
+			eqawarnlog world-writable-setid $x
+		done
 		die "Unsafe files found in \${D}.  Portage will not install them."
 	fi
+
+	IFS=OLDIFS
 }
 
 world_writable_check
-- 
2.0.4

Re: [gentoo-portage-dev] [PATCH 1/3] bin/misc-functions.sh: Introduce eqalog and eqawarnlog functions.

[Zac Medico]

On 10/25/2014 09:15 AM, Michael Palimaka wrote:
> +eqalog() {
> +	local tag=$1 x
> +	shift
> +	for x in "$@" ; do
> +		echo "${tag}" "${x}" >> "${T}"/qa.log
> +	done
> +}

Your patches look good to me, except that I think eqalog should escape
any \n characters that might be embedded in the arguments.

We might use something like this to escape all but the last \n character:

echo "${tag}" "${x}"| (escape="" ; while read -r ; do echo -n "${escape}${REPLY}" ; escape="\\n"; done ; echo ) >> "${T}"/qa.log

Here's a test case showing that it works:

$ echo -e "foo\nbar\nbaz"| (escape="" ; while read -r ; do echo -n "${escape}${REPLY}" ; escape="\\n"; done ; echo )
foo\nbar\nbaz
-- 
Thanks,
Zac

Re: [gentoo-portage-dev] [PATCH 1/3] bin/misc-functions.sh: Introduce eqalog and eqawarnlog functions.

[Zac Medico]

On 10/25/2014 09:15 AM, Michael Palimaka wrote:
> +eqalog() {
> +	local tag=$1 x
> +	shift
> +	for x in "$@" ; do
> +		echo "${tag}" "${x}" >> "${T}"/qa.log
> +	done
> +}
> +
> +eqawarnlog() {
> +	eqalog "$@"
> +	shift
> +	for x in "$@" ; do
> +		eqawarn "  $x"
> +	done
> +}
> +

These functions are internals, so they need to be prefixed with __ like
__eqalog and __eqawarnlog.
-- 
Thanks,
Zac

Re: [gentoo-portage-dev] [PATCH 1/3] bin/misc-functions.sh: Introduce eqalog and eqawarnlog functions.

[Zac Medico]

On 10/25/2014 12:53 PM, Zac Medico wrote:
> On 10/25/2014 09:15 AM, Michael Palimaka wrote:
>> +eqalog() {
>> +	local tag=$1 x
>> +	shift
>> +	for x in "$@" ; do
>> +		echo "${tag}" "${x}" >> "${T}"/qa.log
>> +	done
>> +}
>> +
>> +eqawarnlog() {
>> +	eqalog "$@"
>> +	shift
>> +	for x in "$@" ; do
>> +		eqawarn "  $x"
>> +	done
>> +}
>> +
> 
> These functions are internals, so they need to be prefixed with __ like
> __eqalog and __eqawarnlog.
> 

Also, please unset them inside bin/save-ebuild-env.sh.
-- 
Thanks,
Zac

Re: [gentoo-portage-dev] [PATCH 1/3] bin/misc-functions.sh: Introduce eqalog and eqawarnlog functions.

[Zac Medico]

On 10/25/2014 12:54 PM, Zac Medico wrote:
> On 10/25/2014 12:53 PM, Zac Medico wrote:
>> On 10/25/2014 09:15 AM, Michael Palimaka wrote:
>>> +eqalog() {
>>> +	local tag=$1 x
>>> +	shift
>>> +	for x in "$@" ; do
>>> +		echo "${tag}" "${x}" >> "${T}"/qa.log
>>> +	done
>>> +}
>>> +
>>> +eqawarnlog() {
>>> +	eqalog "$@"
>>> +	shift
>>> +	for x in "$@" ; do
>>> +		eqawarn "  $x"
>>> +	done
>>> +}
>>> +
>>
>> These functions are internals, so they need to be prefixed with __ like
>> __eqalog and __eqawarnlog.
>>
> 
> Also, please unset them inside bin/save-ebuild-env.sh.
> 

Actually, these suggestions are optional, since the environment from
misc-functions.sh is never saved. However, if you wanted to move them to
isolated-functions.sh, then these suggestions are mandatory.
-- 
Thanks,
Zac

[gentoo-portage-dev] Re: [PATCH 1/3] bin/misc-functions.sh: Introduce eqalog and eqawarnlog functions.

[Michael Palimaka]

On 26/10/14 07:00, Zac Medico wrote:
> On 10/25/2014 12:54 PM, Zac Medico wrote:
>> On 10/25/2014 12:53 PM, Zac Medico wrote:
>>> On 10/25/2014 09:15 AM, Michael Palimaka wrote:
>>>> +eqalog() {
>>>> +	local tag=$1 x
>>>> +	shift
>>>> +	for x in "$@" ; do
>>>> +		echo "${tag}" "${x}" >> "${T}"/qa.log
>>>> +	done
>>>> +}
>>>> +
>>>> +eqawarnlog() {
>>>> +	eqalog "$@"
>>>> +	shift
>>>> +	for x in "$@" ; do
>>>> +		eqawarn "  $x"
>>>> +	done
>>>> +}
>>>> +
>>>
>>> These functions are internals, so they need to be prefixed with __ like
>>> __eqalog and __eqawarnlog.
>>>
>>
>> Also, please unset them inside bin/save-ebuild-env.sh.
>>
> 
> Actually, these suggestions are optional, since the environment from
> misc-functions.sh is never saved. However, if you wanted to move them to
> isolated-functions.sh, then these suggestions are mandatory.
> 

Is it better to move them to isolated-functions and implement the above
changes then? I only put it in misc-functions since it's near
install_qa_check and I'm not too familiar with the file layout. :-)

[gentoo-portage-dev] Re: [PATCH 1/3] bin/misc-functions.sh: Introduce eqalog and eqawarnlog functions.

[Zac Medico]

On 10/25/2014 01:13 PM, Michael Palimaka wrote:
> On 26/10/14 07:00, Zac Medico wrote:
>>>>
>>>> These functions are internals, so they need to be prefixed with __ like
>>>> __eqalog and __eqawarnlog.
>>>>
>>>
>>> Also, please unset them inside bin/save-ebuild-env.sh.
>>>
>>
>> Actually, these suggestions are optional, since the environment from
>> misc-functions.sh is never saved. However, if you wanted to move them to
>> isolated-functions.sh, then these suggestions are mandatory.
>>
> 
> Is it better to move them to isolated-functions and implement the above
> changes then? I only put it in misc-functions since it's near
> install_qa_check and I'm not too familiar with the file layout. :-)

Yes, I think so, since eventually you'll probably want to call them
elsewhere.
-- 
Thanks,
Zac

Re: [gentoo-portage-dev] [PATCH 1/3] bin/misc-functions.sh: Introduce eqalog and eqawarnlog functions.

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 627 bytes --]

Dnia 2014-10-25, o godz. 12:42:14
Zac Medico <zmedico@gentoo.org> napisał(a):

> On 10/25/2014 09:15 AM, Michael Palimaka wrote:
> > +eqalog() {
> > +	local tag=$1 x
> > +	shift
> > +	for x in "$@" ; do
> > +		echo "${tag}" "${x}" >> "${T}"/qa.log
> > +	done
> > +}
> 
> Your patches look good to me, except that I think eqalog should escape
> any \n characters that might be embedded in the arguments.

Why? That sounds like some unsafe fancy feature only one person would
use. Embedding newlines in bash is trivial, why make them implicit with
a lot of added complexity?

-- 
Best regards,
Michał Górny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 949 bytes --]

Re: [gentoo-portage-dev] [PATCH 1/3] bin/misc-functions.sh: Introduce eqalog and eqawarnlog functions.

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 730 bytes --]

Dnia 2014-10-25, o godz. 12:53:15
Zac Medico <zmedico@gentoo.org> napisał(a):

> On 10/25/2014 09:15 AM, Michael Palimaka wrote:
> > +eqalog() {
> > +	local tag=$1 x
> > +	shift
> > +	for x in "$@" ; do
> > +		echo "${tag}" "${x}" >> "${T}"/qa.log
> > +	done
> > +}
> > +
> > +eqawarnlog() {
> > +	eqalog "$@"
> > +	shift
> > +	for x in "$@" ; do
> > +		eqawarn "  $x"
> > +	done
> > +}
> > +
> 
> These functions are internals, so they need to be prefixed with __ like
> __eqalog and __eqawarnlog.

eqawarnlog shouldn't be internal since we support adding QA checks
in repositories. In fact, I am planning to move some Gentoo-specific QA
checks out of portage code.

-- 
Best regards,
Michał Górny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 949 bytes --]

Re: [gentoo-portage-dev] [PATCH 1/3] bin/misc-functions.sh: Introduce eqalog and eqawarnlog functions.

[Zac Medico]

On 10/25/2014 01:25 PM, Michał Górny wrote:
> Dnia 2014-10-25, o godz. 12:42:14
> Zac Medico <zmedico@gentoo.org> napisał(a):
> 
>> On 10/25/2014 09:15 AM, Michael Palimaka wrote:
>>> +eqalog() {
>>> +	local tag=$1 x
>>> +	shift
>>> +	for x in "$@" ; do
>>> +		echo "${tag}" "${x}" >> "${T}"/qa.log
>>> +	done
>>> +}
>>
>> Your patches look good to me, except that I think eqalog should escape
>> any \n characters that might be embedded in the arguments.
> 
> Why? That sounds like some unsafe fancy feature only one person would
> use. Embedding newlines in bash is trivial, why make them implicit with
> a lot of added complexity?

Because the caller should not be concerned about the qa.log file format.
The fact that it's delimited by newlines is an implementation detail
that the caller need know nothing about. Escaping the new lines is
really not all that complex. All of the elog functions have similar
handling for newlines.
-- 
Thanks,
Zac

Re: [gentoo-portage-dev] [PATCH 1/3] bin/misc-functions.sh: Introduce eqalog and eqawarnlog functions.

[Zac Medico]

On 10/25/2014 01:26 PM, Michał Górny wrote:
> Dnia 2014-10-25, o godz. 12:53:15
> Zac Medico <zmedico@gentoo.org> napisał(a):
> 
>> On 10/25/2014 09:15 AM, Michael Palimaka wrote:
>>> +eqalog() {
>>> +	local tag=$1 x
>>> +	shift
>>> +	for x in "$@" ; do
>>> +		echo "${tag}" "${x}" >> "${T}"/qa.log
>>> +	done
>>> +}
>>> +
>>> +eqawarnlog() {
>>> +	eqalog "$@"
>>> +	shift
>>> +	for x in "$@" ; do
>>> +		eqawarn "  $x"
>>> +	done
>>> +}
>>> +
>>
>> These functions are internals, so they need to be prefixed with __ like
>> __eqalog and __eqawarnlog.
> 
> eqawarnlog shouldn't be internal since we support adding QA checks
> in repositories. In fact, I am planning to move some Gentoo-specific QA
> checks out of portage code.

It's a PMS thing. If it's not in PMS and the package manager provides
it, it's supposed to be prefixed.
-- 
Thanks,
Zac

Re: [gentoo-portage-dev] [PATCH 1/3] bin/misc-functions.sh: Introduce eqalog and eqawarnlog functions.

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 1225 bytes --]

Dnia 2014-10-25, o godz. 13:28:54
Zac Medico <zmedico@gentoo.org> napisał(a):

> On 10/25/2014 01:25 PM, Michał Górny wrote:
> > Dnia 2014-10-25, o godz. 12:42:14
> > Zac Medico <zmedico@gentoo.org> napisał(a):
> > 
> >> On 10/25/2014 09:15 AM, Michael Palimaka wrote:
> >>> +eqalog() {
> >>> +	local tag=$1 x
> >>> +	shift
> >>> +	for x in "$@" ; do
> >>> +		echo "${tag}" "${x}" >> "${T}"/qa.log
> >>> +	done
> >>> +}
> >>
> >> Your patches look good to me, except that I think eqalog should escape
> >> any \n characters that might be embedded in the arguments.
> > 
> > Why? That sounds like some unsafe fancy feature only one person would
> > use. Embedding newlines in bash is trivial, why make them implicit with
> > a lot of added complexity?
> 
> Because the caller should not be concerned about the qa.log file format.
> The fact that it's delimited by newlines is an implementation detail
> that the caller need know nothing about. Escaping the new lines is
> really not all that complex. All of the elog functions have similar
> handling for newlines.

Oh, you mean converting from newlines to \n escapes? I thought
the opposite way, sorry then.

-- 
Best regards,
Michał Górny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 949 bytes --]

Re: [gentoo-portage-dev] [PATCH 1/3] bin/misc-functions.sh: Introduce eqalog and eqawarnlog functions.

[Zac Medico]

On 10/25/2014 01:41 PM, Michał Górny wrote:
> Dnia 2014-10-25, o godz. 13:28:54
> Zac Medico <zmedico@gentoo.org> napisał(a):
> 
>> On 10/25/2014 01:25 PM, Michał Górny wrote:
>>> Dnia 2014-10-25, o godz. 12:42:14
>>> Zac Medico <zmedico@gentoo.org> napisał(a):
>>>
>>>> On 10/25/2014 09:15 AM, Michael Palimaka wrote:
>>>>> +eqalog() {
>>>>> +	local tag=$1 x
>>>>> +	shift
>>>>> +	for x in "$@" ; do
>>>>> +		echo "${tag}" "${x}" >> "${T}"/qa.log
>>>>> +	done
>>>>> +}
>>>>
>>>> Your patches look good to me, except that I think eqalog should escape
>>>> any \n characters that might be embedded in the arguments.
>>>
>>> Why? That sounds like some unsafe fancy feature only one person would
>>> use. Embedding newlines in bash is trivial, why make them implicit with
>>> a lot of added complexity?
>>
>> Because the caller should not be concerned about the qa.log file format.
>> The fact that it's delimited by newlines is an implementation detail
>> that the caller need know nothing about. Escaping the new lines is
>> really not all that complex. All of the elog functions have similar
>> handling for newlines.
> 
> Oh, you mean converting from newlines to \n escapes? I thought
> the opposite way, sorry then.

Right, that's what my sample code does.
-- 
Thanks,
Zac

Re: [gentoo-portage-dev] [PATCH 1/3] bin/misc-functions.sh: Introduce eqalog and eqawarnlog functions.

[Zac Medico]

On 10/25/2014 01:32 PM, Zac Medico wrote:
> On 10/25/2014 01:26 PM, Michał Górny wrote:
>> Dnia 2014-10-25, o godz. 12:53:15
>> Zac Medico <zmedico@gentoo.org> napisał(a):
>>
>>> On 10/25/2014 09:15 AM, Michael Palimaka wrote:
>>>> +eqalog() {
>>>> +	local tag=$1 x
>>>> +	shift
>>>> +	for x in "$@" ; do
>>>> +		echo "${tag}" "${x}" >> "${T}"/qa.log
>>>> +	done
>>>> +}
>>>> +
>>>> +eqawarnlog() {
>>>> +	eqalog "$@"
>>>> +	shift
>>>> +	for x in "$@" ; do
>>>> +		eqawarn "  $x"
>>>> +	done
>>>> +}
>>>> +
>>>
>>> These functions are internals, so they need to be prefixed with __ like
>>> __eqalog and __eqawarnlog.
>>
>> eqawarnlog shouldn't be internal since we support adding QA checks
>> in repositories. In fact, I am planning to move some Gentoo-specific QA
>> checks out of portage code.
> 
> It's a PMS thing. If it's not in PMS and the package manager provides
> it, it's supposed to be prefixed.

Note that we could have unprefixed aliases inside misc-functions.sh,
since misc-functions.sh env is never saved.
-- 
Thanks,
Zac

[gentoo-portage-dev] [PATCH 1/3] Introduce eqalog and eqawarnlog functions.

[Michael Palimaka]

These functions are to be used for creating a log of QA violations in a
machine-readable format.

Stored in ${T]/qa.log, the log consists of one entry per line in the
following format: violation-tag data

For example:
deprecated-directory /usr/man
deprecated-directory /usr/info
world-writable /var/db/foo/bar
---
 bin/isolated-functions.sh | 16 ++++++++++++++++
 bin/save-ebuild-env.sh    |  1 +
 2 files changed, 17 insertions(+)

diff --git a/bin/isolated-functions.sh b/bin/isolated-functions.sh
index a22af57..ad44e2a 100644
--- a/bin/isolated-functions.sh
+++ b/bin/isolated-functions.sh
@@ -256,6 +256,22 @@ __elog_base() {
 	return 0
 }
 
+__eqalog() {
+	local tag=$1 x
+	shift
+	for x in "$@" ; do
+		echo "${tag}" "${x}"| (escape="" ; while read -r ; do echo -n "${escape}${REPLY}" ; escape="\\n"; done ; echo ) >> "${T}"/qa.log
+	done
+}
+
+__eqawarnlog() {
+	__eqalog "$@"
+	shift
+	for x in "$@" ; do
+		eqawarn "  ${x}"
+	done
+}
+
 eqawarn() {
 	__elog_base QA "$*"
 	[[ ${RC_ENDCOL} != "yes" && ${LAST_E_CMD} == "ebegin" ]] && echo
diff --git a/bin/save-ebuild-env.sh b/bin/save-ebuild-env.sh
index 775c02c..dd233a9 100644
--- a/bin/save-ebuild-env.sh
+++ b/bin/save-ebuild-env.sh
@@ -76,6 +76,7 @@ __save_ebuild_env() {
 		__ebuild_arg_to_phase __ebuild_phase_funcs default \
 		__unpack_tar __unset_colors \
 		__source_env_files __try_source \
+		__eqalog __eqawarnlog \
 		${QA_INTERCEPTORS}
 
 	___eapi_has_usex && unset -f usex
-- 
2.0.4

[gentoo-portage-dev] [PATCH 2/3] install-qa-check.d/05double-D: Write to log and improve consistency.

[Michael Palimaka]

Present the list of offending files newline-delimitered for consistency
with other checks.
---
 bin/install-qa-check.d/05double-D | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/bin/install-qa-check.d/05double-D b/bin/install-qa-check.d/05double-D
index 1634afd..7d958f1 100644
--- a/bin/install-qa-check.d/05double-D
+++ b/bin/install-qa-check.d/05double-D
@@ -2,9 +2,10 @@
 
 DD_check() {
 	if [[ -d ${D%/}${D} ]] ; then
+		eqawarn "QA Notice: files installed in \${D}/\${D}:"
 		local -i INSTALLTOD=0
 		while read -r -d $'\0' i ; do
-			eqawarn "QA Notice: /${i##${D%/}${D}} installed in \${D}/\${D}"
+			__eqawarnlog double-d "/${i##${D%/}${D}}"
 			((INSTALLTOD++))
 		done < <(find "${D%/}${D}" -print0)
 		die "Aborting due to QA concerns: ${INSTALLTOD} files installed in ${D%/}${D}"
-- 
2.0.4

[gentoo-portage-dev] [PATCH 3/3] install-qa-check.d/90world-writable: Write log and general cleanup.

[Michael Palimaka]

Use eqawarn instead of __vecho for visibility.

Present the list of offending files newline-delimitered for consistency
with other checks.
---
 bin/install-qa-check.d/90world-writable | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/bin/install-qa-check.d/90world-writable b/bin/install-qa-check.d/90world-writable
index 771027e..4d5f4ab 100644
--- a/bin/install-qa-check.d/90world-writable
+++ b/bin/install-qa-check.d/90world-writable
@@ -2,21 +2,35 @@
 
 world_writable_check() {
 	# Now we look for all world writable files.
-	local unsafe_files=$(find "${ED}" -type f -perm -2 | sed -e "s:^${ED}:- :")
+	local unsafe_files=$(find "${ED}" -type f -perm -2 | sed -e "s:^${ED}:/:")
+	local OLDIFS x
+
+	OLDIFS=$IFS
+	IFS=$'\n'
+
 	if [[ -n ${unsafe_files} ]] ; then
-		__vecho "QA Security Notice: world writable file(s):"
-		__vecho "${unsafe_files}"
-		__vecho "- This may or may not be a security problem, most of the time it is one."
-		__vecho "- Please double check that $PF really needs a world writeable bit and file bugs accordingly."
-		sleep 1
+		eqawarn "QA Security Notice: world writable file(s):"
+
+		for x in $unsafe_files ; do
+			__eqawarnlog world-writable $x
+		done
+
+		eqawarn "This may or may not be a security problem, most of the time it is one."
+		eqawarn "Please double check that $PF really needs a world writeable bit and file bugs accordingly."
+		eqawarn
 	fi
 
 	local unsafe_files=$(find "${ED}" -type f '(' -perm -2002 -o -perm -4002 ')' | sed -e "s:^${ED}:/:")
 	if [[ -n ${unsafe_files} ]] ; then
 		eqawarn "QA Notice: Unsafe files detected (set*id and world writable)"
-		eqawarn "${unsafe_files}"
+
+		for x in $unsafe_files ; do
+			__eqawarnlog world-writable-setid $x
+		done
 		die "Unsafe files found in \${D}.  Portage will not install them."
 	fi
+
+	IFS=OLDIFS
 }
 
 world_writable_check
-- 
2.0.4

[gentoo-portage-dev] Re: [PATCH 1/3] bin/misc-functions.sh: Introduce eqalog and eqawarnlog functions.

[Michael Palimaka]

On 26/10/14 07:57, Zac Medico wrote:
> On 10/25/2014 01:32 PM, Zac Medico wrote:
>> On 10/25/2014 01:26 PM, Michał Górny wrote:
>>> Dnia 2014-10-25, o godz. 12:53:15
>>> Zac Medico <zmedico@gentoo.org> napisał(a):
>>>
>>>> On 10/25/2014 09:15 AM, Michael Palimaka wrote:
>>>>> +eqalog() {
>>>>> +	local tag=$1 x
>>>>> +	shift
>>>>> +	for x in "$@" ; do
>>>>> +		echo "${tag}" "${x}" >> "${T}"/qa.log
>>>>> +	done
>>>>> +}
>>>>> +
>>>>> +eqawarnlog() {
>>>>> +	eqalog "$@"
>>>>> +	shift
>>>>> +	for x in "$@" ; do
>>>>> +		eqawarn "  $x"
>>>>> +	done
>>>>> +}
>>>>> +
>>>>
>>>> These functions are internals, so they need to be prefixed with __ like
>>>> __eqalog and __eqawarnlog.
>>>
>>> eqawarnlog shouldn't be internal since we support adding QA checks
>>> in repositories. In fact, I am planning to move some Gentoo-specific QA
>>> checks out of portage code.
>>
>> It's a PMS thing. If it's not in PMS and the package manager provides
>> it, it's supposed to be prefixed.
> 
> Note that we could have unprefixed aliases inside misc-functions.sh,
> since misc-functions.sh env is never saved.
> 

I've sent updated patches based on the last feedback. Should I send a
new one with the aliases, and if so, should the portage checks use the
alias or real function?

Just let me know what to change. I have no opinion what goes where. :-)

Re: [gentoo-portage-dev] Re: [PATCH 1/3] bin/misc-functions.sh: Introduce eqalog and eqawarnlog functions.

[Zac Medico]

On 10/26/2014 12:31 PM, Michael Palimaka wrote:
> On 26/10/14 07:57, Zac Medico wrote:
>> On 10/25/2014 01:32 PM, Zac Medico wrote:
>>> On 10/25/2014 01:26 PM, Michał Górny wrote:
>>>> Dnia 2014-10-25, o godz. 12:53:15
>>>> Zac Medico <zmedico@gentoo.org> napisał(a):
>>>>>
>>>>> These functions are internals, so they need to be prefixed with __ like
>>>>> __eqalog and __eqawarnlog.
>>>>
>>>> eqawarnlog shouldn't be internal since we support adding QA checks
>>>> in repositories. In fact, I am planning to move some Gentoo-specific QA
>>>> checks out of portage code.
>>>
>>> It's a PMS thing. If it's not in PMS and the package manager provides
>>> it, it's supposed to be prefixed.
>>
>> Note that we could have unprefixed aliases inside misc-functions.sh,
>> since misc-functions.sh env is never saved.
>>
> 
> I've sent updated patches based on the last feedback. Should I send a
> new one with the aliases, and if so, should the portage checks use the
> alias or real function?

Considering Michał's plan to expose these functions to QA checks in
repositories, it would make sense to go ahead and add expose the aliases
in misc-functions.sh now. On the other hand, it makes sense to use the
prefixed versions in all internal portage code, for consistency. So, I'd
probably just wait until later to add the unprefixed versions. I don't
have a strong opinion though. The new patch set that you posted looks
good to me.

> Just let me know what to change. I have no opinion what goes where. :-)
-- 
Thanks,
Zac

[gentoo-portage-dev] Re: [PATCH 1/3] bin/misc-functions.sh: Introduce eqalog and eqawarnlog functions.

[Michael Palimaka]

On 27/10/14 07:05, Zac Medico wrote:
> On 10/26/2014 12:31 PM, Michael Palimaka wrote:
>> On 26/10/14 07:57, Zac Medico wrote:
>>> On 10/25/2014 01:32 PM, Zac Medico wrote:
>>>> On 10/25/2014 01:26 PM, Michał Górny wrote:
>>>>> Dnia 2014-10-25, o godz. 12:53:15
>>>>> Zac Medico <zmedico@gentoo.org> napisał(a):
>>>>>>
>>>>>> These functions are internals, so they need to be prefixed with __ like
>>>>>> __eqalog and __eqawarnlog.
>>>>>
>>>>> eqawarnlog shouldn't be internal since we support adding QA checks
>>>>> in repositories. In fact, I am planning to move some Gentoo-specific QA
>>>>> checks out of portage code.
>>>>
>>>> It's a PMS thing. If it's not in PMS and the package manager provides
>>>> it, it's supposed to be prefixed.
>>>
>>> Note that we could have unprefixed aliases inside misc-functions.sh,
>>> since misc-functions.sh env is never saved.
>>>
>>
>> I've sent updated patches based on the last feedback. Should I send a
>> new one with the aliases, and if so, should the portage checks use the
>> alias or real function?
> 
> Considering Michał's plan to expose these functions to QA checks in
> repositories, it would make sense to go ahead and add expose the aliases
> in misc-functions.sh now. On the other hand, it makes sense to use the
> prefixed versions in all internal portage code, for consistency. So, I'd
> probably just wait until later to add the unprefixed versions. I don't
> have a strong opinion though. The new patch set that you posted looks
> good to me.

That's fine, we can just add the alias when Michał's GLEP is finalised then.

Re: [gentoo-portage-dev] Re: [PATCH 1/3] bin/misc-functions.sh: Introduce eqalog and eqawarnlog functions.

[Zac Medico]

On 10/27/2014 06:15 AM, Michael Palimaka wrote:
> On 27/10/14 07:05, Zac Medico wrote:
>> On 10/26/2014 12:31 PM, Michael Palimaka wrote:
>>> I've sent updated patches based on the last feedback. Should I send a
>>> new one with the aliases, and if so, should the portage checks use the
>>> alias or real function?
>>
>> Considering Michał's plan to expose these functions to QA checks in
>> repositories, it would make sense to go ahead and add expose the aliases
>> in misc-functions.sh now. On the other hand, it makes sense to use the
>> prefixed versions in all internal portage code, for consistency. So, I'd
>> probably just wait until later to add the unprefixed versions. I don't
>> have a strong opinion though. The new patch set that you posted looks
>> good to me.
> 
> That's fine, we can just add the alias when Michał's GLEP is finalised then.

I've pushed your patches:

https://github.com/gentoo/portage/commit/ab43c1944f0cb6bf43d5b40cceb2e8186645d347
https://github.com/gentoo/portage/commit/5c54f2b18112b779d5dcba30837b34aac74739e9
https://github.com/gentoo/portage/commit/01e148aac631a0d1c78968dea96ebd9ed94e5918

In the first patch, I wrapped lines inside __eqalog.

In the last patch, I did some trivial fixups for quoting and globbing:

diff --git a/bin/install-qa-check.d/90world-writable b/bin/install-qa-check.d/90world-writable
index 4d5f4ab..490aaee 100644
--- a/bin/install-qa-check.d/90world-writable
+++ b/bin/install-qa-check.d/90world-writable
@@ -3,16 +3,17 @@
 world_writable_check() {
 	# Now we look for all world writable files.
 	local unsafe_files=$(find "${ED}" -type f -perm -2 | sed -e "s:^${ED}:/:")
-	local OLDIFS x
+	local OLDIFS x prev_shopts=$-
 
 	OLDIFS=$IFS
 	IFS=$'\n'
+	set -f
 
 	if [[ -n ${unsafe_files} ]] ; then
 		eqawarn "QA Security Notice: world writable file(s):"
 
 		for x in $unsafe_files ; do
-			__eqawarnlog world-writable $x
+			__eqawarnlog world-writable "$x"
 		done
 
 		eqawarn "This may or may not be a security problem, most of the time it is one."
@@ -25,12 +26,13 @@ world_writable_check() {
 		eqawarn "QA Notice: Unsafe files detected (set*id and world writable)"
 
 		for x in $unsafe_files ; do
-			__eqawarnlog world-writable-setid $x
+			__eqawarnlog world-writable-setid "$x"
 		done
 		die "Unsafe files found in \${D}.  Portage will not install them."
 	fi
 
 	IFS=OLDIFS
+	[[ ${prev_shopts} == *f* ]] || set +f
 }
 
 world_writable_check

-- 
Thanks,
Zac