Bug 483304 – multilib-minimal.eclass: make doc installation part of default multilib_src_install_all()

• Status: RESOLVED FIXED
• Product: Gentoo Linux

[Description Michał Górny 2013-09-01 20:59:23 UTC]

Currently it is not possible to pass DOCS through to multilib_src_install() and/or multilib_src_install_all() without triggering the implicit doc installation code. Please run that code as default implementation of multilib_src_install_all() instead, to allow proper overriding.

As a note, this should be probably carried out after einstalldocs() land in eutils.eclass, so that people would be able to easily get the default in their own impl like:

  multilib_src_install_all() {
    einstalldocs
    ...
  }

[Comment 1 Michał Górny 2013-09-13 19:01:03 UTC]

FYI, einstalldocs has been approved by Council and added to eutils.eclass today.

[Comment 2 Ulrich Müller 2013-09-26 13:02:19 UTC]

Why is bug 459692 a blocker for this bug? einstalldocs in eutils should be good enough. (?)

[Comment 3 Michał Górny 2013-10-07 14:31:24 UTC]

Ping. 5 weeks without an answer.

[Comment 4 Julian Ospald 2013-10-10 18:45:45 UTC]

That would be a change in behavior, so it has to be tested/fixed on _all_ packages using this eclass.

[Comment 5 Michał Górny 2013-10-13 09:58:00 UTC]

If you're fine with it, I'll start adding 'einstalldocs' to the deps wherever necessary. Worst case, packages would have docs installed twice for short time. I just need the confirmation that we are ready to go.

[Comment 6 Julian Ospald 2013-10-13 10:16:03 UTC]

confirmation is when we have a patch and when we have tested it on all dependencies

there are not too many reverse deps, so I guess we could do it without diego

[Comment 7 Ulrich Müller 2013-10-13 11:53:31 UTC]

(In reply to Michał Górny from comment #5)
> If you're fine with it, I'll start adding 'einstalldocs' to the deps
> wherever necessary. Worst case, packages would have docs installed twice for
> short time. I just need the confirmation that we are ready to go.

Last time I checked, about half of the inheriting ebuilds had things like explicit dodoc calls in src_install, so they would need no change at all.


(In reply to Julian Ospald (hasufell) from comment #6)
> confirmation is when we have a patch and when we have tested it on all
> dependencies

All in all, I don't see much potential for breakage. Missing doc files aren't catastrophic.

[Comment 8 Ulrich Müller 2013-10-13 11:56:31 UTC]

Oops, I don't know why that blocker appeared. Sorry for the bugspam.

[Comment 9 Julian Ospald 2013-10-13 12:02:16 UTC]

(In reply to Ulrich Müller from comment #7)
> (In reply to Julian Ospald (hasufell) from comment #6)
> > confirmation is when we have a patch and when we have tested it on all
> > dependencies
> 
> All in all, I don't see much potential for breakage. Missing doc files
> aren't catastrophic.

Maybe, maybe not. From now on I will do _nothing_ for any feature request until I see a patch.

I am not interested in adding stuff, asking for review, getting an ok and then have people tell me a few weeks later it wasn't what they wanted.

[Comment 10 Ulrich Müller 2013-10-13 12:37:09 UTC]

Created attachment 360778 [details, diff]
Patch for multilib-minimal.eclass

(In reply to Julian Ospald (hasufell) from comment #9)
> I am not interested in adding stuff, asking for review, getting an ok and
> then have people tell me a few weeks later it wasn't what they wanted.

Well, we're all humans, so mistakes happen.

Patch is attached, please review.

[Comment 11 Julian Ospald 2013-10-13 12:40:43 UTC]

(In reply to Ulrich Müller from comment #10)
> Created attachment 360778 [details, diff] [details, diff]
> Patch for multilib-minimal.eclass
> 
> (In reply to Julian Ospald (hasufell) from comment #9)
> > I am not interested in adding stuff, asking for review, getting an ok and
> > then have people tell me a few weeks later it wasn't what they wanted.
> 
> Well, we're all humans, so mistakes happen.
> 
> Patch is attached, please review.

See, I am not sure that this is what the reporter wants.

[Comment 12 Michał Górny 2013-10-13 16:14:36 UTC]

(In reply to Ulrich Müller from comment #10)
> Created attachment 360778 [details, diff] [details, diff]
> Patch for multilib-minimal.eclass
> 
> Patch is attached, please review.

LGTM. Though I'd rather put einstalldocs where necessary first. It wouldn't hurt if both the ebuild and the eclass call it.

[Comment 13 Michał Górny 2013-10-14 18:25:23 UTC]

(In reply to Michał Górny from comment #12)
> LGTM. Though I'd rather put einstalldocs where necessary first. It wouldn't
> hurt if both the ebuild and the eclass call it.

All ebuilds should be ready now. If someone takes a second look, please remember that some ebuilds intentionally skipped installing docs.

[Comment 14 Julian Ospald 2013-10-15 19:19:39 UTC]

I'll try to do some testing at weekend, a bit busy right now.

[Comment 15 Julian Ospald 2013-10-20 13:37:02 UTC]

running tests now

[Comment 16 Julian Ospald 2013-10-20 16:09:24 UTC]

all consumers tested

[Comment 17 Julian Ospald 2013-10-20 16:27:39 UTC]

+  20 Oct 2013; Julian Ospald <[e-mail]> multilib-minimal.eclass:
+  make doc installation part of default multilib_src_install_all() wrt #483304

[Comment 18 Ulrich Müller 2013-10-20 18:00:25 UTC]

(In reply to Julian Ospald (hasufell) from comment #16)
> all consumers tested

(In reply to Julian Ospald (hasufell) from comment #17)
> +  20 Oct 2013; Julian Ospald <[e-mail]> multilib-minimal.eclass:
> +  make doc installation part of default multilib_src_install_all() wrt
> #483304

Thanks a lot!

[Comment 19 Alexis Ballier 2013-10-26 07:30:34 UTC]

*** Bug 487852 has been marked as a duplicate of this bug. ***