Patchwork [PATCHv2,01/13] systemtap: remove usage of FILESPATH

login
register
mail settings
Submitter Petter Mabäcker
Date May 10, 2014, 8:05 a.m.
Message ID <a517943d9b90ee7793ec9f6734a0bdd80c38d75f.1399671290.git.petter@technux.se>
Download mbox | patch
Permalink /patch/71829/
State New
Headers show

Comments

Petter Mabäcker - May 10, 2014, 8:05 a.m.
Fixes [YOCTO #4497]

Usage of FILESPATH is discouraged, since it can make recipes harder to
bbappend. Instead FILESEXTRAPATHS should be used to extend the path.

Signed-off-by: Petter Mabäcker <petter@technux.se>
---
 .../systemtap/systemtap-uprobes_git.bb             |    2 ++
 meta/recipes-kernel/systemtap/systemtap_git.inc    |    2 --
 2 files changed, 2 insertions(+), 2 deletions(-)
Khem Raj - May 10, 2014, 4:58 p.m.
On Sat, May 10, 2014 at 1:05 AM, Petter Mabäcker <petter@technux.se> wrote:
> +FILESEXTRAPATHS_prepend := "${THISDIR}/systemtap:"

would something like

FILESEXTRAPATHS =. "${FILE_DIRNAME}/systemtap:"

avoid the prepend and immediate evaluation
Petter Mabäcker - May 10, 2014, 10:31 p.m.
2014-05-10 18:58 skrev Khem Raj:

> On Sat, May 10, 2014 at 1:05 AM, Petter Mabäcker <petter@technux.se>
> wrote:
>
>> +FILESEXTRAPATHS_prepend := "${THISDIR}/systemtap:"
>
> would something like
>
> FILESEXTRAPATHS =. "${FILE_DIRNAME}/systemtap:"
>
> avoid the prepend and immediate evaluation

I guess someone with deep bitbake knowledge can answer this better. But 
as far as I understand, you should always use immediate evaluation when 
using THISDIR, to be extra safe. Not sure if it's more safe to use 
FILE_DIRNAME if you want to avoid immediate expanding when using 
FILESEXTRAPATHS?

The bitbake documentations says:

"The operators "_append" and "_prepend" differ from the operators ".=" 
and "=." in that they are deferred until after parsing completes rather 
than being immediately applied."

Not sure if above means that "=." will also immediately expand 
variables or not?

Personally I have always used FILESEXTRAPATHS_prepend := 
"${THISDIR}/<somename>:" like the yocto documentation recommends. But 
sure if you can avoid immediate expanding in FILESEXTRPATHS, that would 
mean a tiny optimization. In that case perhaps the documentation should 
be updated as well. Looking in meta-layer both methods seems to be used.
Khem Raj - May 10, 2014, 11:49 p.m.
On May 10, 2014, at 3:31 PM, petter@technux.se wrote:

> 2014-05-10 18:58 skrev Khem Raj:
> 
>> On Sat, May 10, 2014 at 1:05 AM, Petter Mabäcker <petter@technux.se>
>> wrote:
>> 
>>> +FILESEXTRAPATHS_prepend := "${THISDIR}/systemtap:"
>> 
>> would something like
>> 
>> FILESEXTRAPATHS =. "${FILE_DIRNAME}/systemtap:"
>> 
>> avoid the prepend and immediate evaluation
> 
> I guess someone with deep bitbake knowledge can answer this better. But as far as I understand, you should always use immediate evaluation when using THISDIR, to be extra safe. Not sure if it's more safe to use FILE_DIRNAME if you want to avoid immediate expanding when using FILESEXTRAPATHS?
> 
> The bitbake documentations says:
> 
> "The operators "_append" and "_prepend" differ from the operators ".=" and "=." in that they are deferred until after parsing completes rather than being immediately applied."
> 
> Not sure if above means that "=." will also immediately expand variables or not?
> 
> Personally I have always used FILESEXTRAPATHS_prepend := "${THISDIR}/<somename>:" like the yocto documentation recommends. But sure if you can avoid immediate expanding in FILESEXTRPATHS, that would mean a tiny optimization. In that case perhaps the documentation should be updated as well. Looking in meta-layer both methods seems to be used.


I was hinting on avoiding THISDIR but that does not solve the problem probably what you have it ok in this case.
Richard Purdie - May 11, 2014, 12:13 p.m.
On Sun, 2014-05-11 at 00:31 +0200, petter@technux.se wrote:
> 2014-05-10 18:58 skrev Khem Raj:
> 
> > On Sat, May 10, 2014 at 1:05 AM, Petter Mabäcker <petter@technux.se>
> > wrote:
> >
> >> +FILESEXTRAPATHS_prepend := "${THISDIR}/systemtap:"
> >
> > would something like
> >
> > FILESEXTRAPATHS =. "${FILE_DIRNAME}/systemtap:"
> >
> > avoid the prepend and immediate evaluation
> 
> I guess someone with deep bitbake knowledge can answer this better. But 
> as far as I understand, you should always use immediate evaluation when 
> using THISDIR, to be extra safe. Not sure if it's more safe to use 
> FILE_DIRNAME if you want to avoid immediate expanding when using 
> FILESEXTRAPATHS?
> 
> The bitbake documentations says:
> 
> "The operators "_append" and "_prepend" differ from the operators ".=" 
> and "=." in that they are deferred until after parsing completes rather 
> than being immediately applied."
> 
> Not sure if above means that "=." will also immediately expand 
> variables or not?

They're not.

> Personally I have always used FILESEXTRAPATHS_prepend := 
> "${THISDIR}/<somename>:" like the yocto documentation recommends. But 
> sure if you can avoid immediate expanding in FILESEXTRPATHS, that would 
> mean a tiny optimization. In that case perhaps the documentation should 
> be updated as well. Looking in meta-layer both methods seems to be used.

What Khem means is that in the case we know we're in the .bb file
directory, we can use FILE_DIRNAME instead of using :=.

Why? FILE_DIRNAME is defined as:

conf/bitbake.conf:FILE_DIRNAME = "${@os.path.dirname(d.getVar('FILE'))}"

FILE will point to the .bb file.

Cheers,

Richard
Petter Mabäcker - May 11, 2014, 6:13 p.m.
On 05/11/2014 02:13 PM, Richard Purdie wrote:
> On Sun, 2014-05-11 at 00:31 +0200, petter@technux.se wrote:
>> 2014-05-10 18:58 skrev Khem Raj:
>>
>>> On Sat, May 10, 2014 at 1:05 AM, Petter Mabäcker <petter@technux.se>
>>> wrote:
>>>
>>>> +FILESEXTRAPATHS_prepend := "${THISDIR}/systemtap:"
>>> would something like
>>>
>>> FILESEXTRAPATHS =. "${FILE_DIRNAME}/systemtap:"
>>>
>>> avoid the prepend and immediate evaluation
>> I guess someone with deep bitbake knowledge can answer this better. But
>> as far as I understand, you should always use immediate evaluation when
>> using THISDIR, to be extra safe. Not sure if it's more safe to use
>> FILE_DIRNAME if you want to avoid immediate expanding when using
>> FILESEXTRAPATHS?
>>
>> The bitbake documentations says:
>>
>> "The operators "_append" and "_prepend" differ from the operators ".="
>> and "=." in that they are deferred until after parsing completes rather
>> than being immediately applied."
>>
>> Not sure if above means that "=." will also immediately expand
>> variables or not?
> They're not.
>
>> Personally I have always used FILESEXTRAPATHS_prepend :=
>> "${THISDIR}/<somename>:" like the yocto documentation recommends. But
>> sure if you can avoid immediate expanding in FILESEXTRPATHS, that would
>> mean a tiny optimization. In that case perhaps the documentation should
>> be updated as well. Looking in meta-layer both methods seems to be used.
> What Khem means is that in the case we know we're in the .bb file
> directory, we can use FILE_DIRNAME instead of using :=.
>
> Why? FILE_DIRNAME is defined as:
>
> conf/bitbake.conf:FILE_DIRNAME = "${@os.path.dirname(d.getVar('FILE'))}"
>
> FILE will point to the .bb file.
>
> Cheers,
>
> Richard
>
>
>
Ok, that makes sense. I will send up a new changeset using this method 
instead.

BR,
Petter

Patch

diff --git a/meta/recipes-kernel/systemtap/systemtap-uprobes_git.bb b/meta/recipes-kernel/systemtap/systemtap-uprobes_git.bb
index ab9acba..7d76b52 100644
--- a/meta/recipes-kernel/systemtap/systemtap-uprobes_git.bb
+++ b/meta/recipes-kernel/systemtap/systemtap-uprobes_git.bb
@@ -11,6 +11,8 @@  ALLOW_EMPTY_${PN} = "1"
 
 inherit module-base gettext
 
+FILESEXTRAPATHS_prepend := "${THISDIR}/systemtap:"
+
 FILES_${PN} += "${datadir}/systemtap/runtime/uprobes"
 
 EXTRA_OEMAKE = ""
diff --git a/meta/recipes-kernel/systemtap/systemtap_git.inc b/meta/recipes-kernel/systemtap/systemtap_git.inc
index 78439c1..717d66f 100644
--- a/meta/recipes-kernel/systemtap/systemtap_git.inc
+++ b/meta/recipes-kernel/systemtap/systemtap_git.inc
@@ -8,8 +8,6 @@  SRC_URI = "git://sourceware.org/git/systemtap.git \
            file://obsolete_automake_macros.patch \
           "
 
-FILESPATH = "${FILE_DIRNAME}/systemtap"
-
 # systemtap doesn't support mips
 COMPATIBLE_HOST = '(x86_64|i.86|powerpc|arm|aarch64).*-linux'