[gentoo-dev] [PATCH 0/3] vim-plugin.eclass: improvements.

[gentoo-dev] [PATCH 0/3] vim-plugin.eclass: improvements.

[Patrice Clement]

Hi

I have put together a few patches to improve the state of the vim-plugin
eclass. One commit solves a long standing bug.

Please review.

Thanks!

Patrice Clement (3):
  vim-plugin.eclass: dohtml is now obsolete.
  vim-plugin.eclass: use Portage internal variables.
  vim-plugin.eclass: rework if else clause and reduce find calls down to
    one.

 eclass/vim-plugin.eclass | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

-- 
2.13.6

[gentoo-dev] [PATCH 1/3] vim-plugin.eclass: dohtml is now obsolete.

[Patrice Clement]

---
 eclass/vim-plugin.eclass | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/eclass/vim-plugin.eclass b/eclass/vim-plugin.eclass
index abe9c7b3661..a05cdb4520b 100644
--- a/eclass/vim-plugin.eclass
+++ b/eclass/vim-plugin.eclass
@@ -71,11 +71,7 @@ vim-plugin_src_install() {
 	local f
 	for f in *; do
 		[[ -f "${f}" ]] || continue
-		if [[ "${f}" = *.html ]]; then
-			dohtml "${f}"
-		else
-			dodoc "${f}"
-		fi
+		dodoc "${f}"
 		rm "${f}" || die
 	done
 
-- 
2.13.6

[gentoo-dev] [PATCH 2/3] vim-plugin.eclass: use Portage internal variables.

[Patrice Clement]

Closes: https://bugs.gentoo.org/469400
---
 eclass/vim-plugin.eclass | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/eclass/vim-plugin.eclass b/eclass/vim-plugin.eclass
index a05cdb4520b..3c809768cf8 100644
--- a/eclass/vim-plugin.eclass
+++ b/eclass/vim-plugin.eclass
@@ -39,11 +39,11 @@ vim-plugin_src_install() {
 		ebegin "Fixing file permissions"
 		# Make sure perms are good
 		chmod -R a+rX "${S}" || die "chmod failed"
-		find "${S}" -user  'portage' -exec chown root '{}' \; || die "chown failed"
+		find "${S}" -user "${PORTAGE_USERNAME}" -exec chown root '{}' \; || die "chown failed"
 		if use userland_BSD || [[ ${CHOST} == *-darwin* ]] ; then
-			find "${S}" -group 'portage' -exec chgrp wheel '{}' \; || die "chgrp failed"
+			find "${S}" -group "${PORTAGE_GRPNAME}" -exec chgrp wheel '{}' \; || die "chgrp failed"
 		else
-			find "${S}" -group 'portage' -exec chgrp root '{}' \; || die "chgrp failed"
+			find "${S}" -group "${PORTAGE_GRPNAME}" -exec chgrp root '{}' \; || die "chgrp failed"
 		fi
 		eend $?
 	fi
-- 
2.13.6

[gentoo-dev] [PATCH 3/3] vim-plugin.eclass: rework if else clause and reduce find calls down to one.

[Patrice Clement]

---
 eclass/vim-plugin.eclass | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/eclass/vim-plugin.eclass b/eclass/vim-plugin.eclass
index 3c809768cf8..d635a2aa436 100644
--- a/eclass/vim-plugin.eclass
+++ b/eclass/vim-plugin.eclass
@@ -40,11 +40,11 @@ vim-plugin_src_install() {
 		# Make sure perms are good
 		chmod -R a+rX "${S}" || die "chmod failed"
 		find "${S}" -user "${PORTAGE_USERNAME}" -exec chown root '{}' \; || die "chown failed"
+		local _grp="root"
 		if use userland_BSD || [[ ${CHOST} == *-darwin* ]] ; then
-			find "${S}" -group "${PORTAGE_GRPNAME}" -exec chgrp wheel '{}' \; || die "chgrp failed"
-		else
-			find "${S}" -group "${PORTAGE_GRPNAME}" -exec chgrp root '{}' \; || die "chgrp failed"
+			_grp="wheel"
 		fi
+		find "${S}" -group "${PORTAGE_GRPNAME}" -exec chgrp "${_grp}" '{}' \; || die "chgrp failed"
 		eend $?
 	fi
 
-- 
2.13.6

Re: [gentoo-dev] [PATCH 2/3] vim-plugin.eclass: use Portage internal variables.

[Ulrich Mueller]

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

>>>>> On Sat, 4 Nov 2017, Patrice Clement wrote:

> -		find "${S}" -user  'portage' -exec chown root '{}' \; || die "chown failed"
> +		find "${S}" -user "${PORTAGE_USERNAME}" -exec chown root '{}' \; || die "chown failed"
>  		if use userland_BSD || [[ ${CHOST} == *-darwin* ]] ; then
> -			find "${S}" -group 'portage' -exec chgrp wheel '{}' \; || die "chgrp failed"
> +			find "${S}" -group "${PORTAGE_GRPNAME}" -exec chgrp wheel '{}' \; || die "chgrp failed"
>  		else
> -			find "${S}" -group 'portage' -exec chgrp root '{}' \; || die "chgrp failed"
> +			find "${S}" -group "${PORTAGE_GRPNAME}" -exec chgrp root '{}' \; || die "chgrp failed"

Why are these necessary? The package manager should change ownership
of files to the root user and its group when merging from D to ROOT:
https://projects.gentoo.org/pms/6/pms.html#x1-15600012.3.1

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

Re: [gentoo-dev] [PATCH 1/3] vim-plugin.eclass: dohtml is now obsolete.

[Ulrich Mueller]

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

>>>>> On Sat, 4 Nov 2017, Patrice Clement wrote:

> -		if [[ "${f}" = *.html ]]; then
> -			dohtml "${f}"
> -		else
> -			dodoc "${f}"
> -		fi
> +		dodoc "${f}"

That's not exactly equivalent, because *.html files won't be installed
in the html/ subdirectory any more, but in /usr/share/doc/${PF}/.

Note that the former is in the default exclusion list for compression
but the latter is not (i.e., *.html may get compressed now).

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

Re: [gentoo-dev] [PATCH 2/3] vim-plugin.eclass: use Portage internal variables.

[Michał Górny]

W dniu sob, 04.11.2017 o godzinie 22∶48 +0100, użytkownik Patrice
Clement napisał:
> Closes: https://bugs.gentoo.org/469400
> ---
>  eclass/vim-plugin.eclass | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/eclass/vim-plugin.eclass b/eclass/vim-plugin.eclass
> index a05cdb4520b..3c809768cf8 100644
> --- a/eclass/vim-plugin.eclass
> +++ b/eclass/vim-plugin.eclass
> @@ -39,11 +39,11 @@ vim-plugin_src_install() {
>  		ebegin "Fixing file permissions"
>  		# Make sure perms are good
>  		chmod -R a+rX "${S}" || die "chmod failed"
> -		find "${S}" -user  'portage' -exec chown root '{}' \; || die "chown failed"
> +		find "${S}" -user "${PORTAGE_USERNAME}" -exec chown root '{}' \; || die "chown failed"
>  		if use userland_BSD || [[ ${CHOST} == *-darwin* ]] ; then
> -			find "${S}" -group 'portage' -exec chgrp wheel '{}' \; || die "chgrp failed"
> +			find "${S}" -group "${PORTAGE_GRPNAME}" -exec chgrp wheel '{}' \; || die "chgrp failed"
>  		else
> -			find "${S}" -group 'portage' -exec chgrp root '{}' \; || die "chgrp failed"
> +			find "${S}" -group "${PORTAGE_GRPNAME}" -exec chgrp root '{}' \; || die "chgrp failed"
>  		fi
>  		eend $?
>  	fi

This is going to die on every non-Portage package manager.

-- 
Best regards,
Michał Górny

Re: [gentoo-dev] [PATCH 2/3] vim-plugin.eclass: use Portage internal variables.

[Patrice Clement]

Sunday 05 Nov 2017 00:26:33, Michał Górny wrote :
> W dniu sob, 04.11.2017 o godzinie 22∶48 +0100, użytkownik Patrice
> Clement napisał:
> > Closes: https://bugs.gentoo.org/469400
> > ---
> >  eclass/vim-plugin.eclass | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/eclass/vim-plugin.eclass b/eclass/vim-plugin.eclass
> > index a05cdb4520b..3c809768cf8 100644
> > --- a/eclass/vim-plugin.eclass
> > +++ b/eclass/vim-plugin.eclass
> > @@ -39,11 +39,11 @@ vim-plugin_src_install() {
> >  		ebegin "Fixing file permissions"
> >  		# Make sure perms are good
> >  		chmod -R a+rX "${S}" || die "chmod failed"
> > -		find "${S}" -user  'portage' -exec chown root '{}' \; || die "chown failed"
> > +		find "${S}" -user "${PORTAGE_USERNAME}" -exec chown root '{}' \; || die "chown failed"
> >  		if use userland_BSD || [[ ${CHOST} == *-darwin* ]] ; then
> > -			find "${S}" -group 'portage' -exec chgrp wheel '{}' \; || die "chgrp failed"
> > +			find "${S}" -group "${PORTAGE_GRPNAME}" -exec chgrp wheel '{}' \; || die "chgrp failed"
> >  		else
> > -			find "${S}" -group 'portage' -exec chgrp root '{}' \; || die "chgrp failed"
> > +			find "${S}" -group "${PORTAGE_GRPNAME}" -exec chgrp root '{}' \; || die "chgrp failed"
> >  		fi
> >  		eend $?
> >  	fi
> 
> This is going to die on every non-Portage package manager.
> 
> -- 
> Best regards,
> Michał Górny
> 
> 

Michal, Ulrich,

ACK. This code has been there for ages and I also don't think it makes sense to
keep it as it seems these operations are handled by Portage already. Mike
(floppym) suggested to remove the whole if clause and call fperms instead:

diff --git a/eclass/vim-plugin.eclass b/eclass/vim-plugin.eclass
index abe9c7b3661..46908bb73e6 100644
--- a/eclass/vim-plugin.eclass
+++ b/eclass/vim-plugin.eclass
@@ -35,19 +35,6 @@ vim-plugin_src_install() {
        has "${EAPI:-0}" 0 1 2 && ! use prefix && ED="${D}"
        local f

-       if use !prefix && [[ ${EUID} -eq 0 ]] ; then
-               ebegin "Fixing file permissions"
-               # Make sure perms are good
-               chmod -R a+rX "${S}" || die "chmod failed"
-               find "${S}" -user  'portage' -exec chown root '{}' \; || die "chown failed"
-               if use userland_BSD || [[ ${CHOST} == *-darwin* ]] ; then
-                       find "${S}" -group 'portage' -exec chgrp wheel '{}' \; || die "chgrp failed"
-               else
-                       find "${S}" -group 'portage' -exec chgrp root '{}' \; || die "chgrp failed"
-               fi
-               eend $?
-       fi
-
        # When globbing, if nothing exists, the shell literally returns the glob
        # pattern. So turn on nullglob and extglob options to avoid this.
        eshopts_push -s extglob
@@ -86,7 +73,7 @@ vim-plugin_src_install() {
                "couldn't move ${S} to ${ED}/usr/share/vim/vimfiles"

        # Fix remaining bad permissions
-       chmod -R -x+X "${ED}"/usr/share/vim/vimfiles/ || die "chmod failed"
+       fperms -R a+rX /usr/share/vim/vimfiles
 }

Please let me know what you guys think.

-- 
Patrice Clement
Gentoo Linux developer
http://www.gentoo.org

Re: [gentoo-dev] [PATCH 2/3] vim-plugin.eclass: use Portage internal variables.

[Ulrich Mueller]

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

>>>>> On Sun, 5 Nov 2017, Patrice Clement wrote:

> ACK. This code has been there for ages and I also don't think it makes sense to
> keep it as it seems these operations are handled by Portage already. Mike
> (floppym) suggested to remove the whole if clause and call fperms instead:

> diff --git a/eclass/vim-plugin.eclass b/eclass/vim-plugin.eclass
> index abe9c7b3661..46908bb73e6 100644
> --- a/eclass/vim-plugin.eclass
> +++ b/eclass/vim-plugin.eclass
> @@ -35,19 +35,6 @@ vim-plugin_src_install() {
>         has "${EAPI:-0}" 0 1 2 && ! use prefix && ED="${D}"
>         local f

> -       if use !prefix && [[ ${EUID} -eq 0 ]] ; then
> -               ebegin "Fixing file permissions"
> -               # Make sure perms are good
> -               chmod -R a+rX "${S}" || die "chmod failed"
> -               find "${S}" -user  'portage' -exec chown root '{}' \; || die "chown failed"
> -               if use userland_BSD || [[ ${CHOST} == *-darwin* ]] ; then
> -                       find "${S}" -group 'portage' -exec chgrp wheel '{}' \; || die "chgrp failed"
> -               else
> -                       find "${S}" -group 'portage' -exec chgrp root '{}' \; || die "chgrp failed"
> -               fi
> -               eend $?
> -       fi
> -
>         # When globbing, if nothing exists, the shell literally returns the glob
>         # pattern. So turn on nullglob and extglob options to avoid this.
>         eshopts_push -s extglob
> @@ -86,7 +73,7 @@ vim-plugin_src_install() {
>                 "couldn't move ${S} to ${ED}/usr/share/vim/vimfiles"

>         # Fix remaining bad permissions
> -       chmod -R -x+X "${ED}"/usr/share/vim/vimfiles/ || die "chmod failed"
> +       fperms -R a+rX /usr/share/vim/vimfiles
>  }

LGTM (assuming that a+rX are the intended permissions).

Ulrich

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]