FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Gentoo > Gentoo Development

 
 
LinkBack Thread Tools
 
Old 04-26-2012, 03:21 PM
Zac Medico
 
Default Making user patches globally available

On 04/26/2012 02:55 AM, Duncan wrote:

Zac Medico posted on Wed, 25 Apr 2012 23:26:24 -0700 as excerpted:


On 04/25/2012 11:18 PM, Duncan wrote:

IOW, let's quit letting the perfect be the enemy of the good, and just
get on with it, already.


If that means settling on something that's fragile and prone to lots of
bug reports, then it's not really practical, because it wastes peoples
time (and time is our most valuable resource).


IMO it's trying to do too much with it that's the fragile bit. If all it
does is the patching, but it /always/ does the patching (unlike the hit-
and-miss we get now), and people know they need to use the overlay-ebuild
method to do anything beyond patching, including if they need to re-
invoke eautoreconf, then it should "just work". Right now we're talking
about all this fancy stuff, detecting when we need to automatically run
eautoreconf, etc, and /that/ seems to me to be the fragile bit.


Ignoring the problem doesn't make it go away. If we ignore the problem,
then we end up dealing with bug reports of the form "FEATURES=userpatch
doesn't work with this particular patch set" until the end of time.


Also, don't forget to consider the possibility of interference between
FEATURES=userpatch and epatch_user (applying same patches twice).


Overall, the "apply_user_patches_here" approach [1] seems pretty
reasonable to me.


[1]
http://archives.gentoo.org/gentoo-dev/msg_c228be85e0c4e577ad194e6004d59062.xml

--
Thanks,
Zac
 
Old 04-26-2012, 06:27 PM
Michał Górny
 
Default Making user patches globally available

On Thu, 26 Apr 2012 06:18:32 +0000 (UTC)
Duncan <1i5t5.duncan@cox.net> wrote:

> My suggestion is therefore to do the simple thing, just apply any
> patches found in the patches dir, and punt on the complicated
> do-we-eautoreconf- or-not thing.

Agreed. Just make sure the feature will be only used for ebuilds not
calling epatch_user manually or through the eclass.

--
Best regards,
Michał Górny
 
Old 04-26-2012, 06:43 PM
Zac Medico
 
Default Making user patches globally available

On 04/26/2012 11:27 AM, Michał Górny wrote:
> On Thu, 26 Apr 2012 06:18:32 +0000 (UTC)
> Duncan <1i5t5.duncan@cox.net> wrote:
>
>> My suggestion is therefore to do the simple thing, just apply any
>> patches found in the patches dir, and punt on the complicated
>> do-we-eautoreconf- or-not thing.
>
> Agreed. Just make sure the feature will be only used for ebuilds not
> calling epatch_user manually or through the eclass.

So the package manager is supposed to interact with an eclass function?
That's pretty ugly. Why not just roll out a quick EAPI 5 that adds
support for the "apply_user_patches_here" approach [1]?

http://archives.gentoo.org/gentoo-dev/msg_c228be85e0c4e577ad194e6004d59062.xml
--
Thanks,
Zac
 
Old 04-26-2012, 06:50 PM
Michał Górny
 
Default Making user patches globally available

On Thu, 26 Apr 2012 11:43:37 -0700
Zac Medico <zmedico@gentoo.org> wrote:

> On 04/26/2012 11:27 AM, Michał Górny wrote:
> > On Thu, 26 Apr 2012 06:18:32 +0000 (UTC)
> > Duncan <1i5t5.duncan@cox.net> wrote:
> >
> >> My suggestion is therefore to do the simple thing, just apply any
> >> patches found in the patches dir, and punt on the complicated
> >> do-we-eautoreconf- or-not thing.
> >
> > Agreed. Just make sure the feature will be only used for ebuilds not
> > calling epatch_user manually or through the eclass.
>
> So the package manager is supposed to interact with an eclass
> function? That's pretty ugly. Why not just roll out a quick EAPI 5
> that adds support for the "apply_user_patches_here" approach [1]?
>
> http://archives.gentoo.org/gentoo-dev/msg_c228be85e0c4e577ad194e6004d59062.xml

1) Because it's ugly hack,
2) it will have to interact with epatch_user anyway (at least to avoid
applying patches twice),
3) it will work only for new/modified ebuilds so won't differ at all
from using epatch_user().

--
Best regards,
Michał Górny
 
Old 04-26-2012, 06:55 PM
Ciaran McCreesh
 
Default Making user patches globally available

On Thu, 26 Apr 2012 20:50:02 +0200
Michał Górny <mgorny@gentoo.org> wrote:
> > So the package manager is supposed to interact with an eclass
> > function? That's pretty ugly. Why not just roll out a quick EAPI 5
> > that adds support for the "apply_user_patches_here" approach [1]?
> >
> > http://archives.gentoo.org/gentoo-dev/msg_c228be85e0c4e577ad194e6004d59062.xml
>
> 1) Because it's ugly hack,

Awww. You say that about everything with my name on it.

> 2) it will have to interact with epatch_user anyway (at least to avoid
> applying patches twice),
> 3) it will work only for new/modified ebuilds so won't differ at all
> from using epatch_user().

Not if we kill epatch_user... Not like it works properly anyway, and
it's better to have a "works properly or not at all" feature than one
that breaks randomly. The package mangler could even make it fatal (at
pretend time, no less) if someone wants to apply user patches to an
ebuild whose EAPI doesn't support it.

--
Ciaran McCreesh
 
Old 04-26-2012, 10:08 PM
Duncan
 
Default Making user patches globally available

Zac Medico posted on Thu, 26 Apr 2012 08:21:02 -0700 as excerpted:

> On 04/26/2012 02:55 AM, Duncan wrote:
>> Zac Medico posted on Wed, 25 Apr 2012 23:26:24 -0700 as excerpted:
>>
>>> On 04/25/2012 11:18 PM, Duncan wrote:
>>>> IOW, let's quit letting the perfect be the enemy of the good

>>> If that means settling on something that's fragile and prone to lots
>>> of bug reports, then it's not really practical

>> IMO[,] If all it does is the patching, but it /always/ does the
>> patching[,] and people know they need to use the overlay-ebuild
>> method to do anything beyond patching, [including eautoreconf,]
>> then it should "just work".

> Ignoring the problem doesn't make it go away. If we ignore the problem,
> then we end up dealing with bug reports of the form "FEATURES=userpatch
> doesn't work with this particular patch set" until the end of time.

Standard boilerplate resolved/DUP, pointing to the first one, resolved
like this:

NOTABUG/INVALID. The feature does what it says on the label and is
working as designed. The files are patched and the build bails out if
the patch fails. The userpatch feature is deliberately limited to
patching, thus keeping the problem manageable and the code transparent
and maintainable. For anything fancy, including patches that need
eautoreconf called afterward, the userpatch feature isn't the right
tool. Copy the ebuild to an overlay and edit it as necessary to both
apply the patch and eautoreconf or whatever else needs done afterward.

> Also, don't forget to consider the possibility of interference between
> FEATURES=userpatch and epatch_user (applying same patches twice).

The existing phaselock-file solution should continue to work there. Test
for the existence of a file and punt if it's found; touch the file on
first invocation.

The only caveat is that the existing eclass solution has changed the
filename before. Once the corresponding feature exists, both the eclass
and the feature will have to use the same filename so as not to conflict
with each other, thereby effectively locking down the name and preventing
further changes to it.

> Overall, the "apply_user_patches_here" approach [1] seems pretty
> reasonable to me.
>
> [1]
> http://archives.gentoo.org/gentoo-dev/
msg_c228be85e0c4e577ad194e6004d59062.xml

With the requirements that if the ebuild never calls it, it's still run
immediately after source_prepare, thus ensuring that it gets called, AND
that calling either autoreconf or eautoreconf without calling apply-user-
patches first is a repoman-checked error, it looks like it should work,
agreed.

But I'm a bit wary as to the robustness. And without a mechanism to
apply the patches at a particular point (arguably, post-src-prepare) even
in the absence of a specific apply-user-patches-here call, we're back
where we were without a global solution, but if the hard-invocation is
done, then we're back to either inefficiently running eautoreconf twice
or forced into doing likely failure-prone detection, thus the worry over
robustness.

But of course he who codes, decides. We have existing and already proven
code for the simple case, but if someone actually codes that complex
approach, and it actually does both get us global coverage and avoid
duplicated autoreconf runs, without hard to track down failures, I for
one will certainly bless them! =:^)

I just don't want to have to wait until kingdom come for the "perfect"
solution, when we already have a demonstrated "good enough 80-90% of the
time" solution ready to roll out globally, if we'd just do it.

It's also worth pointing out that nothing prevents rolling out the "good
enough" solution right away, then growing the complexity to cover the
harder autoreconf cases when a solution for them actually gets coded.

--
Duncan - List replies preferred. No HTML msgs.
"Every nonfree program has a lord, a master --
and if you use the program, he is your master." Richard Stallman
 
Old 04-27-2012, 01:41 AM
Zac Medico
 
Default Making user patches globally available

On 04/26/2012 03:08 PM, Duncan wrote:

Zac Medico posted on Thu, 26 Apr 2012 08:21:02 -0700 as excerpted:

Also, don't forget to consider the possibility of interference between
FEATURES=userpatch and epatch_user (applying same patches twice).


The existing phaselock-file solution should continue to work there. Test
for the existence of a file and punt if it's found; touch the file on
first invocation.

The only caveat is that the existing eclass solution has changed the
filename before. Once the corresponding feature exists, both the eclass
and the feature will have to use the same filename so as not to conflict
with each other, thereby effectively locking down the name and preventing
further changes to it.


Having the package manager interact with an eclass function like
epatch_user is ugly, and it's unnecessary since we can pull all of the
pieces into the package manager in EAPI 5. Any eclasses that currently
call epatch_user can have a conditional like this:


if has $EAPI 0 1 2 3 4 ; then
epatch_user
else
apply_user_patches_here
fi


Overall, the "apply_user_patches_here" approach [1] seems pretty
reasonable to me.

[1]
http://archives.gentoo.org/gentoo-dev/

msg_c228be85e0c4e577ad194e6004d59062.xml

With the requirements that if the ebuild never calls it, it's still run
immediately after source_prepare, thus ensuring that it gets called, AND
that calling either autoreconf or eautoreconf without calling apply-user-
patches first is a repoman-checked error, it looks like it should work,
agreed.


I think it might be better to die if it's not called in src_prepare,
like Ciaran originally suggested. This ensures that people don't forget
to call it when they are supposed to.



But I'm a bit wary as to the robustness. And without a mechanism to
apply the patches at a particular point (arguably, post-src-prepare) even
in the absence of a specific apply-user-patches-here call, we're back
where we were without a global solution, but if the hard-invocation is
done, then we're back to either inefficiently running eautoreconf twice
or forced into doing likely failure-prone detection, thus the worry over
robustness.


It's no worse than epatch_user currently is. It's the responsibility of
the person who overrides src_prepare to call eautoreconf or whatever
else when necessary, so the package manager will not have the burden.

--
Thanks,
Zac
 
Old 04-27-2012, 02:15 PM
Duncan
 
Default Making user patches globally available

Zac Medico posted on Thu, 26 Apr 2012 18:41:21 -0700 as excerpted:

> On 04/26/2012 03:08 PM, Duncan wrote:
>> Zac Medico posted on Thu, 26 Apr 2012 08:21:02 -0700 as excerpted:
>>> Also, don't forget to consider the possibility of interference between
>>> FEATURES=userpatch and epatch_user (applying same patches twice).
>>
>> The existing phaselock-file solution should continue to work there.

> Having the package manager interact with an eclass function like
> epatch_user is ugly, and it's unnecessary since we can pull all of the
> pieces into the package manager in EAPI 5. Any eclasses that currently
> call epatch_user can have a conditional like this:
>
> if has $EAPI 0 1 2 3 4 ; then
> epatch_user
> else
> apply_user_patches_here
> fi

But that doesn't solve the problem of making it globally available, since
it only applies to packages/eclasses that already call epatch_user for
EAPIs thru current EAPI-4.

In ordered to make it globally available, it cannot simply be an EAPI-5
thing, it must apply to all current ebuilds whether they (or an inherited
eclass) call epatch_user or not. Which means that conflict with the
existing epatch_user is unavoidable, since it will either try to run
twice where it's already called, or it won't run at all where it's not.

Tho I guess one solution to that would be to change the name of the
patches dir for the new version, calling it /etc/portage/patches2/ or
some such. Another solution could be to make the existing epatch_user
call a no-op, and force post-src-prepare invocation on EAPIs 1-4.

But both of these have problems in that they nullify the work done in
existing ebuilds to locate the call correctly, before eautoreconf or
whatever.

>
>>> Overall, the "apply_user_patches_here" approach [1] seems pretty
>>> reasonable to me.
>>>
>>> [1]
>>> http://archives.gentoo.org/gentoo-dev/
>> msg_c228be85e0c4e577ad194e6004d59062.xml
>>
>> With the requirements that if the ebuild never calls it, it's still run
>> immediately after source_prepare, thus ensuring that it gets called,
>> AND that calling either autoreconf or eautoreconf without calling
>> apply-user-patches fit is a repoman-checked error, it looks like it
>> should work, agreed.
>
> I think it might be better to die if it's not called in src_prepare,
> like Ciaran originally suggested. This ensures that people don't forget
> to call it when they are supposed to.

That can work for EAPI-5, but what about existing ebuilds? Remember, the
goal is global coverage without forcing an edit to every existing ebuild
that doesn't yet call it either directly or via eclass.

>> But I'm a bit wary as to the robustness. And without a mechanism to
>> apply the patches at a particular point (arguably, post-src-prepare)
>> even in the absence of a specific apply-user-patches-here call, we're
>> back where we were without a global solution, but if the
>> hard-invocation is done, then we're back to either inefficiently
>> running eautoreconf twice or forced into doing likely failure-prone
>> detection, thus the worry over robustness.
>
> It's no worse than epatch_user currently is. It's the responsibility of
> the person who overrides src_prepare to call eautoreconf or whatever
> else when necessary, so the package manager will not have the burden.

Except that in ordered to make it global without touching existing
ebuilds, the PM MUST shoulder the burden. THAT is where the robustness
issues appear.

The require-it-in-EAPI-5-plus-only solution will help longer term, but
given that many core packages are to remain EAPI-0 for the foreseeable
future, that could be a VERY long term indeed. Additionally, it only
makes user confusion WORSE, since a user still won't know for sure
whether his patches will apply to a particular ebuild without looking.
Making it "just work" is the goal, and just doing it for EAPI-5+ is nice,
but not really sufficient.

--
Duncan - List replies preferred. No HTML msgs.
"Every nonfree program has a lord, a master --
and if you use the program, he is your master." Richard Stallman
 
Old 04-27-2012, 02:20 PM
Michał Górny
 
Default Making user patches globally available

On Fri, 27 Apr 2012 14:15:35 +0000 (UTC)
Duncan <1i5t5.duncan@cox.net> wrote:

> Zac Medico posted on Thu, 26 Apr 2012 18:41:21 -0700 as excerpted:
>
> > On 04/26/2012 03:08 PM, Duncan wrote:
> >> Zac Medico posted on Thu, 26 Apr 2012 08:21:02 -0700 as excerpted:
> >>> Also, don't forget to consider the possibility of interference
> >>> between FEATURES=userpatch and epatch_user (applying same patches
> >>> twice).
> >>
> >> The existing phaselock-file solution should continue to work
> >> there.
>
> > Having the package manager interact with an eclass function like
> > epatch_user is ugly, and it's unnecessary since we can pull all of
> > the pieces into the package manager in EAPI 5. Any eclasses that
> > currently call epatch_user can have a conditional like this:
> >
> > if has $EAPI 0 1 2 3 4 ; then
> > epatch_user
> > else
> > apply_user_patches_here
> > fi
>
> But that doesn't solve the problem of making it globally available,
> since it only applies to packages/eclasses that already call
> epatch_user for EAPIs thru current EAPI-4.
>
> In ordered to make it globally available, it cannot simply be an
> EAPI-5 thing, it must apply to all current ebuilds whether they (or
> an inherited eclass) call epatch_user or not. Which means that
> conflict with the existing epatch_user is unavoidable, since it will
> either try to run twice where it's already called, or it won't run at
> all where it's not.
>
> Tho I guess one solution to that would be to change the name of the
> patches dir for the new version, calling it /etc/portage/patches2/ or
> some such. Another solution could be to make the existing
> epatch_user call a no-op, and force post-src-prepare invocation on
> EAPIs 1-4.
>
> But both of these have problems in that they nullify the work done in
> existing ebuilds to locate the call correctly, before eautoreconf or
> whatever.

We could finally decide it'll be a Portage internal feature, and modify
epatch_user() to export some Portage-specific indication that user
patches were applied.

--
Best regards,
Michał Górny
 
Old 04-27-2012, 02:27 PM
Ciaran McCreesh
 
Default Making user patches globally available

On Fri, 27 Apr 2012 14:15:35 +0000 (UTC)
Duncan <1i5t5.duncan@cox.net> wrote:
> In ordered to make it globally available, it cannot simply be an
> EAPI-5 thing, it must apply to all current ebuilds whether they (or
> an inherited eclass) call epatch_user or not. Which means that
> conflict with the existing epatch_user is unavoidable, since it will
> either try to run twice where it's already called, or it won't run at
> all where it's not.

In order to make it globally available, you put it in EAPI 5, and make
the package mangler die at pretend time if the user has patches
specified for a package that isn't EAPI 5.

--
Ciaran McCreesh
 

Thread Tools




All times are GMT. The time now is 07:21 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org