| Submitter | Bob Foerster |
|---|---|
| Date | May 4, 2011, 3:56 p.m. |
| Message ID | <1304524618-4864-1-git-send-email-robert@erafx.com> |
| Download | mbox | patch |
| Permalink | /patch/3191/ |
| State | Accepted |
| Headers | show |
Comments
On (04/05/11 11:56), Bob Foerster wrote: > The recipe intended to add "yasm-native" to the dependencies on x86, > but it set DEPENDS_x86 = "yasm-native" which obliterated the base > dependencies. On a fresh build, this package was failing since it > was trying to build before the compiler was ready. > > Depends is now set with DEPENDS_append_x86, which leaves the base > dependencies intact. > > Signed-off-by: Bob Foerster <robert@erafx.com> > --- > recipes/vlc/x264_r2245.bb | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/recipes/vlc/x264_r2245.bb b/recipes/vlc/x264_r2245.bb > index 0cffea3..8c2b5b4 100644 > --- a/recipes/vlc/x264_r2245.bb > +++ b/recipes/vlc/x264_r2245.bb > @@ -23,7 +23,7 @@ X264_ECFLAGS_mipsel = "-fPIC" > X264_DISABLE_ASM = "--disable-asm" > # use assembler written functions for those archs supporting this > X264_DISABLE_ASM_x86 = "" > -DEPENDS_x86 = "yasm-native" > +DEPENDS_append_x86 = "yasm-native" ^ you need a space Otherwise looks ok > > EXTRA_OECONF = '--enable-shared ${X264_DISABLE_ASM} --extra-cflags="${X264_ECFLAGS}"' > > -- > 1.7.1 > > > _______________________________________________ > Openembedded-devel mailing list > Openembedded-devel@lists.openembedded.org > http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-devel
Am Samstag, den 07.05.2011, 17:59 -0700 schrieb Khem Raj: > On (04/05/11 11:56), Bob Foerster wrote: You can be more specific in the commit summary. do not override `DEPENDS` for x86 > > The recipe intended to add "yasm-native" to the dependencies on x86, Use present tense. s/intended/intends/ > > but it set DEPENDS_x86 = "yasm-native" which obliterated the base s/set/sets/ > > dependencies. On a fresh build, this package was failing since it > > was trying to build before the compiler was ready. > > > > Depends is now set with DEPENDS_append_x86, which leaves the base > > dependencies intact. > > > > Signed-off-by: Bob Foerster <robert@erafx.com> > > --- > > recipes/vlc/x264_r2245.bb | 2 +- > > 1 files changed, 1 insertions(+), 1 deletions(-) > > > > diff --git a/recipes/vlc/x264_r2245.bb b/recipes/vlc/x264_r2245.bb > > index 0cffea3..8c2b5b4 100644 > > --- a/recipes/vlc/x264_r2245.bb > > +++ b/recipes/vlc/x264_r2245.bb > > @@ -23,7 +23,7 @@ X264_ECFLAGS_mipsel = "-fPIC" > > X264_DISABLE_ASM = "--disable-asm" > > # use assembler written functions for those archs supporting this > > X264_DISABLE_ASM_x86 = "" > > -DEPENDS_x86 = "yasm-native" > > +DEPENDS_append_x86 = "yasm-native" > ^ > you need a space You should also be able to use +=, should not you? > Otherwise looks ok > > > > EXTRA_OECONF = '--enable-shared ${X264_DISABLE_ASM} --extra-cflags="${X264_ECFLAGS}"' Thanks, Paul
On Sun, 2011-05-08 at 08:56 +0200, Paul Menzel wrote: > Am Samstag, den 07.05.2011, 17:59 -0700 schrieb Khem Raj: > > On (04/05/11 11:56), Bob Foerster wrote: > > You can be more specific in the commit summary. > > do not override `DEPENDS` for x86 > > > > The recipe intended to add "yasm-native" to the dependencies on x86, > > Use present tense. s/intended/intends/ If this is talking about the former, erroneous behaviour (which is corrected by this patch) then the past tense is correct. > > > -DEPENDS_x86 = "yasm-native" > > > +DEPENDS_append_x86 = "yasm-native" > > ^ > > you need a space > > You should also be able to use +=, should not you? No, += doesn't work with overrides. "DEPENDS_x86 +=" would do the same wrong thing as the original "DEPENDS_x86". p.
Am Sonntag, den 08.05.2011, 10:02 +0100 schrieb Phil Blundell: > On Sun, 2011-05-08 at 08:56 +0200, Paul Menzel wrote: > > Am Samstag, den 07.05.2011, 17:59 -0700 schrieb Khem Raj: > > > On (04/05/11 11:56), Bob Foerster wrote: > > > > You can be more specific in the commit summary. > > > > do not override `DEPENDS` for x86 > > > > > > The recipe intended to add "yasm-native" to the dependencies on x86, > > > > Use present tense. s/intended/intends/ > > If this is talking about the former, erroneous behaviour (which is > corrected by this patch) then the past tense is correct. I thought in minutes or proceeding always the present tense is used. The recipe behaves incorrectly until the patch is applied. But it is not very important. > > > > -DEPENDS_x86 = "yasm-native" > > > > +DEPENDS_append_x86 = "yasm-native" > > > ^ > > > you need a space > > > > You should also be able to use +=, should not you? > > No, += doesn't work with overrides. "DEPENDS_x86 +=" would do the same > wrong thing as the original "DEPENDS_x86". Thank you for the clarification. Is for example `RDEPENDS_${PN}` also an override? If yes, then looking at $ git grep 'DEPENDS_' returns a lot of incorrect usages. Thanks, Paul
On Sun, 2011-05-08 at 11:16 +0200, Paul Menzel wrote: > Thank you for the clarification. Is for example `RDEPENDS_${PN}` also an > override? Not in the sense I was meaning here, no. (It is true that the ${PN} bit is technically an OVERRIDE, but in most/all cases RDEPENDS_${PN} is effectively a primary variable, i.e. there is no underlying RDEPENDS which it overrides.) The issue with the "DEPENDS_x86" thing is that, as far as the initial metadata processing is concerned, DEPENDS and DEPENDS_x86 are distinct variables. You might hope that "DEPENDS_x86 += ..." would do a conditional append to DEPENDS, but it doesn't: what happens is that it creates a new variable "DEPENDS_x86" which then replaces the contents of DEPENDS during override processing. So the initial value of ${DEPENDS} is lost rather than being appended to. This whole issue is basically the whole reason that the "_append" syntax hasn't been deprecated, since we don't currently have any other way to do conditional appending (apart from resorting to python). > If yes, then looking at > > $ git grep 'DEPENDS_' > > returns a lot of incorrect usages. From a quick glance at that output nothing jumped out at me as being incorrect, but there might well be a few wrong ones in there. This one, for example: classes/vala.bbclass:DEPENDS_virtclass-native += "vala-native" looks a bit dubious to me (for the same reasons as above) but without checking the recipe context it's impossible to say whether it's actually wrong or not. p.
On Sat, May 7, 2011 at 8:59 PM, Khem Raj <raj.khem@gmail.com> wrote: > On (04/05/11 11:56), Bob Foerster wrote: > > The recipe intended to add "yasm-native" to the dependencies on x86, > > but it set DEPENDS_x86 = "yasm-native" which obliterated the base > > dependencies. On a fresh build, this package was failing since it > > was trying to build before the compiler was ready. > > > > Depends is now set with DEPENDS_append_x86, which leaves the base > > dependencies intact. > > > > Signed-off-by: Bob Foerster <robert@erafx.com> > > --- > > recipes/vlc/x264_r2245.bb | 2 +- > > 1 files changed, 1 insertions(+), 1 deletions(-) > > > > diff --git a/recipes/vlc/x264_r2245.bb b/recipes/vlc/x264_r2245.bb > > index 0cffea3..8c2b5b4 100644 > > --- a/recipes/vlc/x264_r2245.bb > > +++ b/recipes/vlc/x264_r2245.bb > > @@ -23,7 +23,7 @@ X264_ECFLAGS_mipsel = "-fPIC" > > X264_DISABLE_ASM = "--disable-asm" > > # use assembler written functions for those archs supporting this > > X264_DISABLE_ASM_x86 = "" > > -DEPENDS_x86 = "yasm-native" > > +DEPENDS_append_x86 = "yasm-native" > ^ > you need a space > > Oddly, it works without the space. From bitbake -e: DEPENDS="pkgconfig-native autoconf-native automake-native help2man-native libtool-native libtool-cross gnu-config-native virtual/i486-oe-linux-gcc virtual/libc yasm-native" Nonetheless, I'll add a space and send a v2. > Otherwise looks ok > > > > EXTRA_OECONF = '--enable-shared ${X264_DISABLE_ASM} > --extra-cflags="${X264_ECFLAGS}"' > > > > -- > > 1.7.1 > > > > > > _______________________________________________ > > Openembedded-devel mailing list > > Openembedded-devel@lists.openembedded.org > > http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-devel > > -- > -Khem > > _______________________________________________ > Openembedded-devel mailing list > Openembedded-devel@lists.openembedded.org > http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-devel >
Patch
diff --git a/recipes/vlc/x264_r2245.bb b/recipes/vlc/x264_r2245.bb index 0cffea3..8c2b5b4 100644 --- a/recipes/vlc/x264_r2245.bb +++ b/recipes/vlc/x264_r2245.bb @@ -23,7 +23,7 @@ X264_ECFLAGS_mipsel = "-fPIC" X264_DISABLE_ASM = "--disable-asm" # use assembler written functions for those archs supporting this X264_DISABLE_ASM_x86 = "" -DEPENDS_x86 = "yasm-native" +DEPENDS_append_x86 = "yasm-native" EXTRA_OECONF = '--enable-shared ${X264_DISABLE_ASM} --extra-cflags="${X264_ECFLAGS}"'
The recipe intended to add "yasm-native" to the dependencies on x86, but it set DEPENDS_x86 = "yasm-native" which obliterated the base dependencies. On a fresh build, this package was failing since it was trying to build before the compiler was ready. Depends is now set with DEPENDS_append_x86, which leaves the base dependencies intact. Signed-off-by: Bob Foerster <robert@erafx.com> --- recipes/vlc/x264_r2245.bb | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)