Bug 449806 – [Future EAPI] Extend default src_install phase to read DOCS variable for appending more DOCS files

• Product: Portage Development
• Status: RESOLVED WONTFIX

[Description Pacho Ramos 2013-01-02 19:07:38 UTC]

Current default src_install phase is:
http://devmanual.gentoo.org/ebuild-writing/eapi/index.html

src_install() {
	if [[ -f Makefile ]] || [[ -f GNUmakefile]] || [[ -f makefile ]] ; then
		emake DESTDIR="${D}" install
	fi

	if ! declare -p DOCS >/dev/null 2>&1 ; then
		local d
		for d in README* ChangeLog AUTHORS NEWS TODO CHANGES THANKS BUGS \
				FAQ CREDITS CHANGELOG ; do
			[[ -s "${d}" ]] && dodoc "${d}"
		done
	# TODO: wrong "declare -a" command...
	elif declare -p DOCS | grep -q `^declare -a` ; then
		dodoc "${DOCS[@]}"
	else
		dodoc ${DOCS}
	fi
}

The problem is that there are lots of cases where we could benefit from current defaults checked when DOCS is not set, but we would need to append one or two more files.

Currently we need to specify all DOCS in DOCS variable, if src_install could append DOCS files, we would only need to specify "special" docs

Reproducible: Always

[Comment 1 Michał Górny 2013-01-03 05:10:56 UTC]

Well, that is a common problem that number of eclasses suffers as well. So far, I couldn't think of a clear way of supporting that.

[Comment 2 Ulrich Müller 2013-01-03 07:30:12 UTC]

Is there a need for such an extension? One can always do:

    src_install() {
        default
        dodoc FOO BAR
    }

[Comment 3 Pacho Ramos 2013-01-03 11:29:59 UTC]

(In reply to comment #2)
> Is there a need for such an extension? One can always do:
> 
>     src_install() {
>         default
>         dodoc FOO BAR
>     }

It would allow us to not need to add explicitly a phase and simply set DOCS variable.

[Comment 4 Michał Górny 2013-02-24 18:26:27 UTC]

I think we could do something like the following.

1) make DOCS default to DOCS=( __defaults__ ),

2) ebuild now can override it or use DOCS+=( ... ),

3) default_src_install reads DOCS and installs default files if __defaults__ is there.

Of course, assuming that '__defaults__' may be any internal name and users are not supposed to rely on that.

[Comment 5 Pacho Ramos 2013-03-29 10:30:19 UTC]

(In reply to comment #4)
> I think we could do something like the following.
> 
> 1) make DOCS default to DOCS=( __defaults__ ),
> 
> 2) ebuild now can override it or use DOCS+=( ... ),
> 
> 3) default_src_install reads DOCS and installs default files if __defaults__
> is there.
> 
> Of course, assuming that '__defaults__' may be any internal name and users
> are not supposed to rely on that.

That option also looks reasonable to me

[Comment 6 Ulrich Müller 2013-03-29 13:01:13 UTC]

(In reply to comment #4)
> I think we could do something like the following.
> 
> 1) make DOCS default to DOCS=( __defaults__ ),
> 
> 2) ebuild now can override it or use DOCS+=( ... ),
>
> 3) default_src_install reads DOCS and installs default files if __defaults__
> is there.

This would add DOCS as a global variable to every ebuild's environment, even in cases where it's not needed at all. I also dislike the in-band signalling, how would you install a file named "__defaults__"? Some escape mechanism would be needed.

Wouldn't it be easier to introduce another variable like DOCS_EXTRA?

[Comment 7 Michał Górny 2013-03-29 13:25:28 UTC]

(In reply to comment #6)
> (In reply to comment #4)
> > I think we could do something like the following.
> > 
> > 1) make DOCS default to DOCS=( __defaults__ ),
> > 
> > 2) ebuild now can override it or use DOCS+=( ... ),
> >
> > 3) default_src_install reads DOCS and installs default files if __defaults__
> > is there.
> 
> This would add DOCS as a global variable to every ebuild's environment, even
> in cases where it's not needed at all.

Is that really an issue?

The only problem I can see is changing the behavior of eclasses which use $DOCS. But supposedly those eclasses would switch to the new PMS doc-install function.

> I also dislike the in-band
> signalling, how would you install a file named "__defaults__"? Some escape
> mechanism would be needed.

This is the only way to make it a bit natural with the +=() syntax. And please don't really get into the corner-name case, I'm sure PM maintainers will find a junk that will never be found in real package. vapier's good at designing things like that.

> Wouldn't it be easier to introduce another variable like DOCS_EXTRA?

Maybe. But it would feel weird to have yet another variable. And how would an ebuild with:

  DOCS=( foo )
  DOCS_EXTRA=( bar )

behave? Or even with:

  DOCS=()
  DOCS_EXTRA=( baz )

?

...which reminds me that I was supposed to request explicit support for DOCS=().

[Comment 8 Ulrich Müller 2013-03-29 14:34:06 UTC]

(In reply to comment #7)
> This is the only way to make it a bit natural with the +=() syntax.

It's not really natural, because it takes away some of the existing functionality: DOCS+=( foo bar ) would work, and so would DOCS="foo bar", but DOCS+=" foo bar" wouldn't. That doesn't follow the principle of least surprise.

> > Wouldn't it be easier to introduce another variable like DOCS_EXTRA?
> 
> Maybe. But it would feel weird to have yet another variable.

"It would feel weird" isn't a good argument. ;-)

> And how would an ebuild with:
> 
>   DOCS=( foo )
>   DOCS_EXTRA=( bar )
> 
> behave? Or even with:
> 
>   DOCS=()
>   DOCS_EXTRA=( baz )

No problem, it would install both DOCS and DOCS_EXTRA. But why should an ebuild define more than one of these variables?

[Comment 9 Michał Górny 2013-03-29 14:57:56 UTC]

(In reply to comment #8)
> > And how would an ebuild with:
> > 
> >   DOCS=( foo )
> >   DOCS_EXTRA=( bar )
> > 
> > behave? Or even with:
> > 
> >   DOCS=()
> >   DOCS_EXTRA=( baz )
> 
> No problem, it would install both DOCS and DOCS_EXTRA. But why should an
> ebuild define more than one of these variables?

Don't ask me. It's not me who's suggesting to have two variables which serve the same purpose :).

One last idea would be to use 'append' marker inside DOCS but you'd probably dislike that anyway. Something like:

  DOCS=( +HACKING )
  DOCS="+HACKING"

Possibly in pair with excludes for the defaults:

  DOCS=( -README ... )
  DOCS="-README ..."

but I guess it's a triumph of form over content.

[Comment 10 Ulrich Müller 2013-03-30 10:47:04 UTC]

We already have a perfect way for specifying a set of documentation files that's different from the default, see comment #2. Why do we need a second way to do the same thing?

Seriously, I'm worried that our EAPIs get more and more complicated and that the learning curve for new developers gets steeper. Certainly, each proposal for itself doesn't add much complexity, but it's the sum of all things.

[Comment 11 Zac Medico 2013-03-30 16:29:23 UTC]

Rather than introduce special DOCS syntax for people to learn, we could encapsulate the functionality in a helper function. That way, the syntax is hidden and people just deal with helper options and arguments. For example, consider how docompress makes it easy to enable/disable compression for specific files.

[Comment 12 Michał Górny 2013-03-30 23:05:28 UTC]

(In reply to comment #10)
> We already have a perfect way for specifying a set of documentation files
> that's different from the default, see comment #2. Why do we need a second
> way to do the same thing?
> 
> Seriously, I'm worried that our EAPIs get more and more complicated and that
> the learning curve for new developers gets steeper. Certainly, each proposal
> for itself doesn't add much complexity, but it's the sum of all things.

I'd guess. Feel free to WONTFIX then, and focus on getting 'einstalldocs'.

[Comment 13 Michał Górny 2015-11-21 13:40:38 UTC]

Let's WONTFIX this since we won't be able to get any clear solution here anyway, and the uses cases are rather limited.

[Comment 14 Pacho Ramos 2015-11-21 13:49:11 UTC]

Well, the use cases are a lot of gnome2.eclass consumers for example in what we need to set DOCS only to append HACKING docs file for example

Well, maybe this could be accomplished in einstalldocs instead :/

[Comment 15 Michał Górny 2015-11-21 15:05:34 UTC]

  [[ -f HACKING ]] && dodoc HACKING

in gnome2_src_install? ;-)