[gentoo-dev] [PATCH eutils 1/2] Add dointo && newinto.

[gentoo-dev] [PATCH eutils 1/2] Add dointo && newinto.

[Michał Górny]

---
 eutils.eclass | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/eutils.eclass b/eutils.eclass
index eb8c8f7..119fc32 100644
--- a/eutils.eclass
+++ b/eutils.eclass
@@ -650,6 +650,43 @@ edos2unix() {
 	sed -i 's/\r$//' -- "$@" || die
 }
 
+# @FUNCTION: dointo
+# @USAGE: <directory> <file> [...]
+# @DESCRIPTION:
+# Install all specified <file>s into <directory>. This doesn't modify global
+# 'insinto' path. Alike doins, calls 'die' on failure in EAPI 4+; in earlier
+# EAPIs, returns false in that case.
+dointo() {
+	[[ ${#} -gt 2 ]] || die 'Synopsis: dointo <directory> <file> [...]'
+
+	local directory=${1}
+	shift
+
+	(
+		insinto "${directory}"
+		doins "${@}"
+	)
+}
+
+# @FUNCTION: newinto
+# @USAGE: <directory> <file> <new-name>
+# @DESCRIPTION:
+# Install the specified <file> into <directory>, renaming it to <new-name>.
+# This doesn't modify global 'insinto' path. Alike doins, calls 'die' on failure
+# in EAPI 4+; in earlier EAPIs, returns false in that case.
+newinto() {
+	[[ ${#} -eq 3 ]] || die 'Synopsis: newinto <directory> <file> <new-name>'
+
+	local directory=${1}
+	local f=${2}
+	local new_name=${3}
+
+	(
+		insinto "${directory}"
+		newins "${f}" "${new_name}"
+	)
+}
+
 # @FUNCTION: make_desktop_entry
 # @USAGE: make_desktop_entry(<command>, [name], [icon], [type], [fields])
 # @DESCRIPTION:
-- 
1.7.11.1

[gentoo-dev] [PATCH eutils 2/2] Convert simple uses to dointo/newinto.

[Michał Górny]

I will provide patches for more if the functions are accepted.
---
 eutils.eclass | 24 ++++++------------------
 1 file changed, 6 insertions(+), 18 deletions(-)

diff --git a/eutils.eclass b/eutils.eclass
index 119fc32..f59b7ae 100644
--- a/eutils.eclass
+++ b/eutils.eclass
@@ -871,12 +871,8 @@ make_desktop_entry() {
 	fi
 	[[ -n ${fields} ]] && printf '%b\n' "${fields}" >> "${desktop}"
 
-	(
-		# wrap the env here so that the 'insinto' call
-		# doesn't corrupt the env of the caller
-		insinto /usr/share/applications
-		doins "${desktop}"
-	) || die "installing desktop file failed"
+	dointo /usr/share/applications "${desktop}" \
+		|| die "installing desktop file failed"
 }
 
 # @FUNCTION: validate_desktop_entries
@@ -932,12 +928,7 @@ make_session_desktop() {
 	Type=XSession
 	EOF
 
-	(
-	# wrap the env here so that the 'insinto' call
-	# doesn't corrupt the env of the caller
-	insinto /usr/share/xsessions
-	doins "${desktop}"
-	)
+	dointo /usr/share/xsessions "${desktop}"
 }
 
 # @FUNCTION: domenu
@@ -973,12 +964,9 @@ domenu() {
 # @DESCRIPTION:
 # Like all other new* functions, install the specified menu as newname.
 newmenu() {
-	(
-	# wrap the env here so that the 'insinto' call
-	# doesn't corrupt the env of the caller
-	insinto /usr/share/applications
-	newins "$@"
-	)
+	[[ ${#} -eq 2 ]] || die 'Synopsis: newmenu <menu> <newname>'
+
+	newinto /usr/share/applications "${@}"
 }
 
 # @FUNCTION: _iconins
-- 
1.7.11.1

Re: [gentoo-dev] [PATCH eutils 1/2] Add dointo && newinto.

[Diego Elio Pettenò]

On 16/08/2012 13:19, Michał Górny wrote:
>  eutils.eclass | 37 +++++++++++++++++++++++++++++++++++++

I would say "no", and let's queue this for the next EAPI.

The reason being we have everything ${foo}into {new,do}${foo}, provided
by EAPI and utils as well, why should foo=ins be different now?

-- 
Diego Elio Pettenò — Flameeyes
flameeyes@flameeyes.eu — http://blog.flameeyes.eu/

Re: [gentoo-dev] [PATCH eutils 1/2] Add dointo && newinto.

[Ulrich Mueller]

>>>>> On Thu, 16 Aug 2012, Michał Górny wrote:
 
> +dointo() {
> +	[[ ${#} -gt 2 ]] || die 'Synopsis: dointo <directory> <file> [...]'
> +
> +	local directory=${1}
> +	shift
> +
> +	(
> +		insinto "${directory}"

Shouldn't there be checking for errors here, for the case that insinto
fails?

> +		doins "${@}"
> +	)
> +}

Ulrich

Re: [gentoo-dev] [PATCH eutils 1/2] Add dointo && newinto.

[Michał Górny]

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

On Thu, 16 Aug 2012 13:29:50 -0700
Diego Elio Pettenò <flameeyes@flameeyes.eu> wrote:

> On 16/08/2012 13:19, Michał Górny wrote:
> >  eutils.eclass | 37 +++++++++++++++++++++++++++++++++++++
> 
> I would say "no", and let's queue this for the next EAPI.
> 
> The reason being we have everything ${foo}into {new,do}${foo},
> provided by EAPI and utils as well, why should foo=ins be different
> now?

I'd be happy to see that in a new EAPI but please note that a lot
of eclasses inlines that thing right now.

We can remove it from eutils when it gets into EAPI (i.e. make
conditional to older EAPIs). This will keep things both usable
and clean.

-- 
Best regards,
Michał Górny

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

Re: [gentoo-dev] [PATCH eutils 1/2] Add dointo && newinto.

[Diego Elio Pettenò]

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

On 16/08/2012 13:45, Michał Górny wrote:
> 
> We can remove it from eutils when it gets into EAPI (i.e. make
> conditional to older EAPIs). This will keep things both usable
> and clean.

I still don't like it — I definitely don't like changing the approach
midway and halfway.

So my 2 cents here keep saying "no".

-- 
Diego Elio Pettenò — Flameeyes
flameeyes@flameeyes.eu — http://blog.flameeyes.eu/


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 554 bytes --]

Re: [gentoo-dev] [PATCH eutils 1/2] Add dointo && newinto.

[Michał Górny]

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

On Thu, 16 Aug 2012 22:36:46 +0200
Ulrich Mueller <ulm@gentoo.org> wrote:

> >>>>> On Thu, 16 Aug 2012, Michał Górny wrote:
>  
> > +dointo() {
> > +	[[ ${#} -gt 2 ]] || die 'Synopsis: dointo <directory>
> > <file> [...]' +
> > +	local directory=${1}
> > +	shift
> > +
> > +	(
> > +		insinto "${directory}"
> 
> Shouldn't there be checking for errors here, for the case that insinto
> fails?

Hmm, I don't even know if insinto should ever fail. And if it does,
then the following doins should fail as well, I guess.

But I see no problem adding '&& \' there if necessary.

> > +		doins "${@}"

-- 
Best regards,
Michał Górny

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

Re: [gentoo-dev] [PATCH eutils 1/2] Add dointo && newinto.

[Michał Górny]

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

On Thu, 16 Aug 2012 13:45:50 -0700
Diego Elio Pettenò <flameeyes@flameeyes.eu> wrote:

> On 16/08/2012 13:45, Michał Górny wrote:
> > 
> > We can remove it from eutils when it gets into EAPI (i.e. make
> > conditional to older EAPIs). This will keep things both usable
> > and clean.
> 
> I still don't like it — I definitely don't like changing the approach
> midway and halfway.
> 
> So my 2 cents here keep saying "no".

Well, in that case it is pointless to add it to EAPI since almost no
ebuild will use it.

The main benefit of these functions is to eclasses which now inline
the same code. I really don't see them replacing:

    (
        insinto /foo
        doins $@
    )

with:

    case $EAPI in
        5)
            dointo /foo $@
            ;;
        *)
            (
                insinto /foo
                doins $@
            )
            ;;
    esac

What's the point of simplifications which actually make things
harder?      

-- 
Best regards,
Michał Górny

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

Re: [gentoo-dev] [PATCH eutils 1/2] Add dointo && newinto.

[Ulrich Mueller]

>>>>> On Thu, 16 Aug 2012, Michał Górny wrote:

>> > +dointo() {
>> > +	[[ ${#} -gt 2 ]] || die 'Synopsis: dointo <directory>
>> > <file> [...]' +
>> > +	local directory=${1}
>> > +	shift
>> > +
>> > +	(
>> > +		insinto "${directory}"
>> 
>> Shouldn't there be checking for errors here, for the case that insinto
>> fails?

> Hmm, I don't even know if insinto should ever fail.

It can fail if the underlying "install -d" fails, e.g. if a regular
file is in the way of the directory to be created.

> And if it does, then the following doins should fail as well, I
> guess.

Normally I also don't add any error checks for insinto in ebuilds.
But for a general eclass function, chances are higher that something
goes wrong. For example, arguments may be specified in the wrong
order. So some extra error checking cannot harm (and is cheap).

Ulrich

[gentoo-dev] [PATCH eutils] Add dointo && newinto.

[Michał Górny]

---
 gx86/eclass/eutils.eclass | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/gx86/eclass/eutils.eclass b/gx86/eclass/eutils.eclass
index eb8c8f7..edea3ea 100644
--- a/gx86/eclass/eutils.eclass
+++ b/gx86/eclass/eutils.eclass
@@ -650,6 +650,43 @@ edos2unix() {
 	sed -i 's/\r$//' -- "$@" || die
 }
 
+# @FUNCTION: dointo
+# @USAGE: <directory> <file> [...]
+# @DESCRIPTION:
+# Install all specified <file>s into <directory>. This doesn't modify global
+# 'insinto' path. Alike doins, calls 'die' on failure in EAPI 4+; in earlier
+# EAPIs, returns false in that case.
+dointo() {
+	[[ ${#} -gt 2 ]] || die 'Synopsis: dointo <directory> <file> [...]'
+
+	local directory=${1}
+	shift
+
+	(
+		insinto "${directory}" && \
+		doins "${@}"
+	)
+}
+
+# @FUNCTION: newinto
+# @USAGE: <directory> <file> <new-name>
+# @DESCRIPTION:
+# Install the specified <file> into <directory>, renaming it to <new-name>.
+# This doesn't modify global 'insinto' path. Alike doins, calls 'die' on failure
+# in EAPI 4+; in earlier EAPIs, returns false in that case.
+dointo() {
+	[[ ${#} -eq 3 ]] || die 'Synopsis: newinto <directory> <file> <new-name>'
+
+	local directory=${1}
+	local f=${2}
+	local new_name=${3}
+
+	(
+		insinto "${directory}" && \
+		newins "${f}" "${new_name}"
+	)
+}
+
 # @FUNCTION: make_desktop_entry
 # @USAGE: make_desktop_entry(<command>, [name], [icon], [type], [fields])
 # @DESCRIPTION:
-- 
1.7.11.1

Re: [gentoo-dev] [PATCH eutils 1/2] Add dointo && newinto.

[Jeroen Roovers]

On Thu, 16 Aug 2012 22:19:44 +0200
Michał Górny <mgorny@gentoo.org> wrote:

> ---
>  eutils.eclass | 37 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)

Could you provide a couple of use cases?


     jer

Re: [gentoo-dev] [PATCH eutils 1/2] Add dointo && newinto.

[Michał Górny]

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

On Fri, 17 Aug 2012 18:53:43 +0200
Jeroen Roovers <jer@gentoo.org> wrote:

> On Thu, 16 Aug 2012 22:19:44 +0200
> Michał Górny <mgorny@gentoo.org> wrote:
> 
> > ---
> >  eutils.eclass | 37 +++++++++++++++++++++++++++++++++++++
> >  1 file changed, 37 insertions(+)
> 
> Could you provide a couple of use cases?

Patch 2 does that. It's basically intended to be used in other eclasses
to implement do*() and new*() functions.

-- 
Best regards,
Michał Górny

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

Re: [gentoo-dev] [PATCH eutils 1/2] Add dointo && newinto.

[Mike Frysinger]

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

On Thursday 16 August 2012 16:19:44 Michał Górny wrote:
> --- a/eutils.eclass
> +++ b/eutils.eclass
> 
> +# Install all specified <file>s into <directory>. This doesn't modify global
> +# 'insinto' path. Alike doins, calls 'die' on failure in EAPI 4+; in earlier
> +# EAPIs, returns false in that case.

i don't really see the point in differentiating here.  we have plenty of
helpers that have always implicitly called die regardless of the EAPI level,
and it's not like you'd be breaking any existing behavior since no one is
using this already.  and even then, you'd be "breaking" builds that were
already broken.

> +dointo() {
> +	[[ ${#} -gt 2 ]] || die 'Synopsis: dointo <directory> <file> [...]'

"Usage" is the standard prefix, not "Synopsis"

style wise, {} around #/@/1/2/3/4/5/6/7/8/9/? variables is noise imo
-mike

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

Re: [gentoo-dev] [PATCH eutils 1/2] Add dointo && newinto.

[Michał Górny]

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

On Fri, 17 Aug 2012 23:25:10 -0400
Mike Frysinger <vapier@gentoo.org> wrote:

> On Thursday 16 August 2012 16:19:44 Michał Górny wrote:
> > --- a/eutils.eclass
> > +++ b/eutils.eclass
> > 
> > +# Install all specified <file>s into <directory>. This doesn't
> > modify global +# 'insinto' path. Alike doins, calls 'die' on
> > failure in EAPI 4+; in earlier +# EAPIs, returns false in that case.
> 
> i don't really see the point in differentiating here.  we have plenty
> of helpers that have always implicitly called die regardless of the
> EAPI level, and it's not like you'd be breaking any existing behavior
> since no one is using this already.  and even then, you'd be
> "breaking" builds that were already broken.

Maybe. Alternatively, I could end up doing doins || die || die. It will
work but what's the point?

> > +dointo() {
> > +	[[ ${#} -gt 2 ]] || die 'Synopsis: dointo <directory>
> > <file> [...]'
> 
> "Usage" is the standard prefix, not "Synopsis"

Fixed.

-- 
Best regards,
Michał Górny

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

Re: [gentoo-dev] [PATCH eutils 1/2] Add dointo && newinto.

[Mike Frysinger]

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

On Saturday 18 August 2012 03:21:20 Michał Górny wrote:
> On Fri, 17 Aug 2012 23:25:10 -0400 Mike Frysinger wrote:
> > On Thursday 16 August 2012 16:19:44 Michał Górny wrote:
> > > --- a/eutils.eclass
> > > +++ b/eutils.eclass
> > > 
> > > +# Install all specified <file>s into <directory>. This doesn't
> > > modify global +# 'insinto' path. Alike doins, calls 'die' on
> > > failure in EAPI 4+; in earlier +# EAPIs, returns false in that case.
> > 
> > i don't really see the point in differentiating here.  we have plenty
> > of helpers that have always implicitly called die regardless of the
> > EAPI level, and it's not like you'd be breaking any existing behavior
> > since no one is using this already.  and even then, you'd be
> > "breaking" builds that were already broken.
> 
> Maybe. Alternatively, I could end up doing doins || die || die. It will
> work but what's the point?

the double die only kicks in with EAPI=4+, and even then is hidden to most 
people at the code level.  it also looks a lot better than:
	(
		insinto ... && doins ...
	)
	case ${EAPI:-0} in 0|1|2|3) [[ $? -ne 0 ]] && die ;; esac
vs
	(insinto ... && doins ...) || die
-mike

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

Re: [gentoo-dev] [PATCH eutils 1/2] Add dointo && newinto.

[Mike Frysinger]

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

were you going to post an updated version for merging ?
-mike

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

Re: [gentoo-dev] [PATCH eutils 1/2] Add dointo && newinto.

[Michał Górny]

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

On Wed, 19 Sep 2012 01:45:04 -0400
Mike Frysinger <vapier@gentoo.org> wrote:

> were you going to post an updated version for merging ?

The whole idea was blocked by Diego, and was submitted for the next
Council meeting.

-- 
Best regards,
Michał Górny

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