Bug 444732 – net-misc/ntp: please review prefix changes

• Status: RESOLVED FIXED
• Product: Gentoo Linux

[Description Christoph Junghans (RETIRED) 2012-11-25 20:23:34 UTC]

Created attachment 330570 [details, diff]
patch against ntp-4.2.6_p5-r1.ebuild

Changes to be added as ntp-4.2.6_p5-r2:
- EAPI bumped to 4 to make portage prefix-aware
- emake install + dodoc = EAPI4 default
- ${D} -> ${ED}
- ${ROOT} -> ${EROOT}
- don't run fperms and fowners in prefix
- dosed is obsolete in EAPI=4
- inherited user.eclass due to enewgroup and enewuser

[Comment 1 SpanKY 2012-11-26 06:09:44 UTC]

Comment on attachment 330570 [details, diff]
patch against ntp-4.2.6_p5-r1.ebuild

>-	use caps || dosed "s|-u ntp:ntp||" /etc/conf.d/ntpd
>-	dosed "s:/usr/bin:/usr/sbin:" /etc/init.d/ntpd
>+	use caps || sed "s|-u ntp:ntp||" /etc/conf.d/ntpd || die
>+	sed "s:/usr/bin:/usr/sbin:" /etc/init.d/ntpd || die

pretty sure this is wrong for two reasons:
 - missing ${ED} on file name after dosed->sed
 - addition of ||die in use caps statement means this always dies when USE=-caps

rest looks fine

[Comment 2 Christoph Junghans (RETIRED) 2012-11-26 15:48:33 UTC]

Created attachment 330638 [details, diff]
patch against ntp-4.2.6_p5-r1.ebuild

(In reply to comment #1)
> Comment on attachment 330570 [details, diff] [details, diff]
> patch against ntp-4.2.6_p5-r1.ebuild
> 
> >-	use caps || dosed "s|-u ntp:ntp||" /etc/conf.d/ntpd
> >-	dosed "s:/usr/bin:/usr/sbin:" /etc/init.d/ntpd
> >+	use caps || sed "s|-u ntp:ntp||" /etc/conf.d/ntpd || die
> >+	sed "s:/usr/bin:/usr/sbin:" /etc/init.d/ntpd || die
> 
> pretty sure this is wrong for two reasons:
>  - missing ${ED} on file name after dosed->sed
>  - addition of ||die in use caps statement means this always dies when
> USE=-caps
You are right, the "${ED}" was missing and also the -i switch for sed, patch updated.
I don't understand the other concern. A || B || C is the same as A || { B || C } and { B || C } was inside a function (dodsed) before.

[Comment 3 SpanKY 2012-11-26 18:19:16 UTC]

(In reply to comment #2)

yes, the use||sed||die logic works correctly.  generally i try to avoid that construct because it can be confusing at a glance :p.

this patch is OK to commit

[Comment 4 Christoph Junghans (RETIRED) 2012-11-26 18:31:16 UTC]

(In reply to comment #3)
> (In reply to comment #2)
> 
> yes, the use||sed||die logic works correctly.  generally i try to avoid that
> construct because it can be confusing at a glance :p.
If you prefer
if use caps ; then
  sed .. || die
fi

for readability, I am happy to commit that. 
> 
> this patch is OK to commit

[Comment 5 Christoph Junghans (RETIRED) 2012-11-26 18:37:04 UTC]

(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #2)
> > 
> > yes, the use||sed||die logic works correctly.  generally i try to avoid that
> > construct because it can be confusing at a glance :p.
> If you prefer
> if use caps ; then
Of course: if ! use caps ; then

[Comment 6 SpanKY 2012-11-26 18:40:24 UTC]

(In reply to comment #4)

if you wouldn't mind, that'd be preferable

[Comment 7 Christoph Junghans (RETIRED) 2012-11-27 05:07:25 UTC]

Committed in the preferred form, thanks.