kde-sunset: Calling base_src_prepare from kde.eclass
 

Hello again,

I have decided to continue discussion in new thread. This discussion
was started in thread "kde-sunset: kdepim-kresources broken".

I have replaced my code with what I originally intended (the code in
git is wrong, since I made a mistake editing it - here is what I
intended):

# Check for PATCHES in EAPI=0|1
case ${EAPI:-0} in
0|1)
if [[ -n ${PATCHES} ]] && [[ -d "${KDE_S}" ]]; then
base_src_prepare
fi
;;
esac

Look, for example, at kde-base/ark ebuild. There is a patch, but it's
not applied correctly without my code.

What I'm worried about is the check [[ -d "${KDE_S}" ]] makes sure,
that this code is called if

[[ -d "${KDE_S}" ]] || base_src_unpack unpack

did not called base-src_unpack and therefore did not applied patches.

There are two problems with EAPI=0|1

1) It depends on file naming and will not work if base_src_unpack
unpacks into $KDE_S
2) Even if base_src_unpack calls base_src_prepare, we're adding
patches automatically after this call, but they will not be applied.

Since I can't think of correct check, please let me know if you have
an idea. Also, let me know if I'm wrong about something and it's not
needed at all. (but I doubt that, because ark, as mentioned above, is
counterexample)

Regards Ladislav Laska
S pozdravem Ladislav Laska
---
xmpp/jabber: ladislav.laska@jabber.cz

kde-sunset: Calling base_src_prepare from kde.eclass
 

Hi there!

On 19.02.2010 22:23, Ladislav Laska wrote:
> Hello again,
>
> I have decided to continue discussion in new thread. This discussion
> was started in thread "kde-sunset: kdepim-kresources broken".
>
> I have replaced my code with what I originally intended (the code in
> git is wrong, since I made a mistake editing it - here is what I
> intended):
>
> # Check for PATCHES in EAPI=0|1
> case ${EAPI:-0} in
> 0|1)
> if [[ -n ${PATCHES} ]] && [[ -d "${KDE_S}" ]]; then
> base_src_prepare
> fi
> ;;
> esac
>
> Look, for example, at kde-base/ark ebuild. There is a patch, but it's
> not applied correctly without my code.

media-gfx/gwenview-1.4.2-r3, on the other hand, would try applying
patches twice /with/ your code.

> What I'm worried about is the check [[ -d "${KDE_S}" ]] makes sure,
> that this code is called if
>
> [[ -d "${KDE_S}" ]] || base_src_unpack unpack
>
> did not called base-src_unpack and therefore did not applied patches.
>
> There are two problems with EAPI=0|1
>
> 1) It depends on file naming and will not work if base_src_unpack
> unpacks into $KDE_S

You mean kde-meta_src_unpack does unpack into $KDE_S, right?

I just had a look. For kde-base/ark, kde-meta_src_unpack does unpack
part of the tarball, therefore kde_src_unpack won't call
base_src_unpack, therefore base_src_prepare won't get called either.

> 2) Even if base_src_unpack calls base_src_prepare, we're adding
> patches automatically after this call, but they will not be applied.

You're right, there is code collecting additional patches from $PATCHDIR
into the $PATCHES array. So we need to call base_src_prepare /after/
that, while the current base_src_unpack call comes before that, and
unpacking has to come before that as $PATCHDIR is unpacked as well.

As base_src_unpack calls base_src_prepare unconditionally for recent
incarnations of base.eclass, I feel we cannot call base_src_unpack at
all. Instead we should simply call "unpack ${A}" directly, and
base_src_prepare later on.

I feel that this code collecting and applying $PATCHES would be better
suited to a kde_src_prepare than kde_src_unpack, but I'm not sure it's
worth refactoring kde.eclass.

> Since I can't think of correct check, please let me know if you have
> an idea. Also, let me know if I'm wrong about something and it's not
> needed at all. (but I doubt that, because ark, as mentioned above, is
> counterexample)

No, you are correct in diagnosing the problem. I just had another stab
at a solution. Have a look at 5979aefafa6e200a09d5dd7b3a6e99b3e0a9d74e
and see if you find any package where that approach doesn't work.

Greetings,
Martin

kde-sunset: Calling base_src_prepare from kde.eclass
 

On 21.02.2010 15:53, Chris Bandy wrote:
> As a more comprehensive test, I rebuilt everything I had in
> kde-sunset[1]. Everything regarding PATCHES worked great!

Thanks a lot, I appreciate that testing!

> However, I had to make three changes to individual ebuilds:
>
> 1. Had to remove x11-libs/qt/qt-3.3.8-libpng14.patch because it was not
> in the Manifest

Committed by Ladislav in f7ba03a46f179acb658a9775a75972fa3d2b68c5

> 2. Had to modify app-text/poppler/poppler-0.12.3-r3.ebuild as described
> here: http://forums.gentoo.org/viewtopic-p-6170313.html#6170313

Committed by me in 5ba737169016a164646bbfe1da53cbf7f34d137f

> 3. Had to revert kde-base/kdepim-kresources-3.5.10.ebuild back to EAPI=1

Committed by Ladislav in 5be5829c56380ca6ff08775d814558d936f816cc

> I've attached a log for the failed EAPI=2 build in #3.

I've had a look. Could reproduce the issue here, and could diff both
runs. One of the first differences I noted was this:

EAPI=1: checking if libkcal should be compiled... no
EAPI=2: checking if libkcal should be compiled... yes

This output is, of course, from configure. If you look at the ebuild,
you notice that it disables libkcal via an environment variable:

src_compile() {
export DO_NOT_COMPILE="knotes libkcal"

This is only done in src_compile, i.e. after src_configure has already
completed. Only in EAPI=1, where there is no src_configure phase, but
instead a configure stage of the kde-meta_src_compile function, setting
the variable in this fashion gives the desired result.

I've written a EAPI=2 version of the ebuild that avoids these issues. It
works for me, so I decided to commit it, just in case we'll want to use
EAPI=2 in the future.

Committed by me in 1a496ce8440c3b8781be988df65ecc77ea267e6b

Greetings,
Martin von Gagern

kde-sunset: Calling base_src_prepare from kde.eclass
 

Martin von Gagern wrote:
> On 21.02.2010 15:53, Chris Bandy wrote:
>
>> As a more comprehensive test, I rebuilt everything I had in
>> kde-sunset[1]. Everything regarding PATCHES worked great!
>>
>
> Thanks a lot, I appreciate that testing!
>
>
>> However, I had to make three changes to individual ebuilds:
>>
>> 1. Had to remove x11-libs/qt/qt-3.3.8-libpng14.patch because it was not
>> in the Manifest
>>
>
> Committed by Ladislav in f7ba03a46f179acb658a9775a75972fa3d2b68c5
>
>
>> 2. Had to modify app-text/poppler/poppler-0.12.3-r3.ebuild as described
>> here: http://forums.gentoo.org/viewtopic-p-6170313.html#6170313
>>
>
> Committed by me in 5ba737169016a164646bbfe1da53cbf7f34d137f
>
>
>> 3. Had to revert kde-base/kdepim-kresources-3.5.10.ebuild back to EAPI=1
>>
>
> Committed by Ladislav in 5be5829c56380ca6ff08775d814558d936f816cc
>
>
>> I've attached a log for the failed EAPI=2 build in #3.
>>
>
> I've had a look. Could reproduce the issue here, and could diff both
> runs. One of the first differences I noted was this:
>
> EAPI=1: checking if libkcal should be compiled... no
> EAPI=2: checking if libkcal should be compiled... yes
>
> This output is, of course, from configure. If you look at the ebuild,
> you notice that it disables libkcal via an environment variable:
>
> src_compile() {
> export DO_NOT_COMPILE="knotes libkcal"
>
> This is only done in src_compile, i.e. after src_configure has already
> completed. Only in EAPI=1, where there is no src_configure phase, but
> instead a configure stage of the kde-meta_src_compile function, setting
> the variable in this fashion gives the desired result.
>
> I've written a EAPI=2 version of the ebuild that avoids these issues. It
> works for me, so I decided to commit it, just in case we'll want to use
> EAPI=2 in the future.
>
> Committed by me in 1a496ce8440c3b8781be988df65ecc77ea267e6b
>
> Greetings,
> Martin von Gagern
>

I re-emerged everything I had in kde-sunset[1] again. No build errors;
everything works! Thanks!

[1] 1a496ce8440c3b8781be988df65ecc77ea267e6b

-- Chris