Patchwork shared-mime-info: fix ordering of PACKAGES

login
register
mail settings
Submitter Koen Kooi
Date Nov. 18, 2011, 2:13 p.m.
Message ID <1321625613-30146-1-git-send-email-koen@dominion.thruhere.net>
Download mbox | patch
Permalink /patch/15137/
State Accepted
Commit 79ee0f8c849cd5de16b6ae1b3dd6c43bbad472b2
Headers show

Comments

Koen Kooi - Nov. 18, 2011, 2:13 p.m.
Signed-off-by: Koen Kooi <koen@dominion.thruhere.net>
---
 .../shared-mime-info/shared-mime-info.inc          |    2 +-
 .../shared-mime-info/shared-mime-info_0.91.bb      |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
Richard Purdie - Nov. 18, 2011, 2:27 p.m.
On Fri, 2011-11-18 at 15:13 +0100, Koen Kooi wrote:
> Signed-off-by: Koen Kooi <koen@dominion.thruhere.net>
> ---
>  .../shared-mime-info/shared-mime-info.inc          |    2 +-
>  .../shared-mime-info/shared-mime-info_0.91.bb      |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

Merged to master, thanks.

Richard
Frans Meulenbroeks - Nov. 18, 2011, 2:28 p.m.
2011/11/18 Koen Kooi <koen@dominion.thruhere.net>

> Signed-off-by: Koen Kooi <koen@dominion.thruhere.net>
> ---
>  .../shared-mime-info/shared-mime-info.inc          |    2 +-
>  .../shared-mime-info/shared-mime-info_0.91.bb      |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/meta/recipes-support/shared-mime-info/shared-mime-info.inc
> b/meta/recipes-support/shared-mime-info/shared-mime-info.inc
> index 57e829c..d7fa67c 100644
> --- a/meta/recipes-support/shared-mime-info/shared-mime-info.inc
> +++ b/meta/recipes-support/shared-mime-info/shared-mime-info.inc
> @@ -19,7 +19,7 @@ FILES_${PN}-dev +=
> "${datadir}/pkgconfig/shared-mime-info.pc"
>
>  # freedesktop.org.xml is only required when updating the mime database,
>  # package it separately
> -PACKAGES += "shared-mime-info-data"
> +PACKAGES =+ "shared-mime-info-data"
>  FILES_shared-mime-info-data =
> "${datadir}/mime/packages/freedesktop.org.xml"
>  RDEPENDS_shared-mime-info-data = "shared-mime-info"
>
> diff --git a/meta/recipes-support/shared-mime-info/
> shared-mime-info_0.91.bb b/meta/recipes-support/shared-mime-info/
> shared-mime-info_0.91.bb
> index bc4a8f7..7ae6e10 100644
> --- a/meta/recipes-support/shared-mime-info/shared-mime-info_0.91.bb
> +++ b/meta/recipes-support/shared-mime-info/shared-mime-info_0.91.bb
> @@ -1,5 +1,5 @@
>  require shared-mime-info.inc
> -PR = "r2"
> +PR = "r3"
>
>  SRC_URI[md5sum] = "982a211560ba4c47dc791ccff34e8fbc"
>  SRC_URI[sha256sum] =
> "98cfebe1d809afb24934e634373821e2a1dfa86fc6462cab230589a1c80988bd"
> --
> 1.7.2.5
>

For my education:
Is ordering of PACKAGES relevant? If so, please enlighten me why. If not
why this patch?

Frans
Paul Eggleton - Nov. 18, 2011, 2:34 p.m.
On Friday 18 November 2011 15:28:28 Frans Meulenbroeks wrote:
> Is ordering of PACKAGES relevant? If so, please enlighten me why.

It is. The packaging code reads this in order and the first package to match a 
file is the package that the file will go into; thus if you want to add a 
package that will include files that would normally be in one of the default 
packages you need to prepend to PACKAGES so that your new package gets the 
files first.

Cheers,
Paul
Richard Purdie - Nov. 18, 2011, 2:34 p.m.
On Fri, 2011-11-18 at 15:28 +0100, Frans Meulenbroeks wrote:
> 
> 
> 2011/11/18 Koen Kooi <koen@dominion.thruhere.net>
>         Signed-off-by: Koen Kooi <koen@dominion.thruhere.net>
>         ---
>          .../shared-mime-info/shared-mime-info.inc          |    2 +-
>          .../shared-mime-info/shared-mime-info_0.91.bb      |    2 +-
>          2 files changed, 2 insertions(+), 2 deletions(-)
>         
>         diff --git
>         a/meta/recipes-support/shared-mime-info/shared-mime-info.inc
>         b/meta/recipes-support/shared-mime-info/shared-mime-info.inc
>         index 57e829c..d7fa67c 100644
>         ---
>         a/meta/recipes-support/shared-mime-info/shared-mime-info.inc
>         +++
>         b/meta/recipes-support/shared-mime-info/shared-mime-info.inc
>         @@ -19,7 +19,7 @@ FILES_${PN}-dev +=
>         "${datadir}/pkgconfig/shared-mime-info.pc"
>         
>          # freedesktop.org.xml is only required when updating the mime
>         database,
>          # package it separately
>         -PACKAGES += "shared-mime-info-data"
>         +PACKAGES =+ "shared-mime-info-data"
>          FILES_shared-mime-info-data =
>         "${datadir}/mime/packages/freedesktop.org.xml"
>          RDEPENDS_shared-mime-info-data = "shared-mime-info"
>         
>         diff --git
>         a/meta/recipes-support/shared-mime-info/shared-mime-info_0.91.bb b/meta/recipes-support/shared-mime-info/shared-mime-info_0.91.bb
>         index bc4a8f7..7ae6e10 100644
>         ---
>         a/meta/recipes-support/shared-mime-info/shared-mime-info_0.91.bb
>         +++
>         b/meta/recipes-support/shared-mime-info/shared-mime-info_0.91.bb
>         @@ -1,5 +1,5 @@
>          require shared-mime-info.inc
>         -PR = "r2"
>         +PR = "r3"
>         
>          SRC_URI[md5sum] = "982a211560ba4c47dc791ccff34e8fbc"
>          SRC_URI[sha256sum] =
>         "98cfebe1d809afb24934e634373821e2a1dfa86fc6462cab230589a1c80988bd"
>         --
>         1.7.2.5
> 
> For my education:
> Is ordering of PACKAGES relevant? If so, please enlighten me why. If
> not why this patch?

Just to cut the story short here, I agree the commit message could have
had more information. There are bug reports on the list related to this
and its clear there is a problem.

I've therefore taken the patch since I'd rather fix this than let more
people fall over the problem over the weekend. In other circumstances I
might have held out for more of a commit message.

Cheers,

Richard
Frans Meulenbroeks - Nov. 19, 2011, 3:02 p.m.
2011/11/18 Paul Eggleton <paul.eggleton@linux.intel.com>

> On Friday 18 November 2011 15:28:28 Frans Meulenbroeks wrote:
> > Is ordering of PACKAGES relevant? If so, please enlighten me why.
>
> It is. The packaging code reads this in order and the first package to
> match a
> file is the package that the file will go into; thus if you want to add a
> package that will include files that would normally be in one of the
> default
> packages you need to prepend to PACKAGES so that your new package gets the
> files first.
>
> Ah, ok. understood.
Not fully sure if the current behaviour is the most desirable one.
Is it still possible to put a file into two packages by explicitly adding
it to both?

Also it might be somewhat confusing that a line that adds say a dir to a
package does not necessarily get all fies in that dir in the package
Personally I think I prefer to have all added in that case, irrespective if
a previous package already took a file from it.
Alternately it could be seen as a QA issue if a file could have been added
to two packages (in which case a more strict pattern should have been used
somewhere)

Frans
Martin Jansa - Nov. 19, 2011, 3:13 p.m.
On Sat, Nov 19, 2011 at 04:02:49PM +0100, Frans Meulenbroeks wrote:
> 2011/11/18 Paul Eggleton <paul.eggleton@linux.intel.com>
> 
> > On Friday 18 November 2011 15:28:28 Frans Meulenbroeks wrote:
> > > Is ordering of PACKAGES relevant? If so, please enlighten me why.
> >
> > It is. The packaging code reads this in order and the first package to
> > match a
> > file is the package that the file will go into; thus if you want to add a
> > package that will include files that would normally be in one of the
> > default
> > packages you need to prepend to PACKAGES so that your new package gets the
> > files first.
> >
> > Ah, ok. understood.
> Not fully sure if the current behaviour is the most desirable one.
> Is it still possible to put a file into two packages by explicitly adding
> it to both?

And then make resulting packages RCONFLICTing each other? Imho this is
bad idea and it's good that this wasn't ever possible afaik.
 
> Also it might be somewhat confusing that a line that adds say a dir to a
> package does not necessarily get all fies in that dir in the package
> Personally I think I prefer to have all added in that case, irrespective if
> a previous package already took a file from it.

Why confusing?
should only one package include all /usr/lib/foo and then separte
subpackages package only some parts of it so we won't pull whole big
/usr/lib/foo when we need in some image only
/usr/lib/foo/feature1 and /usr/lib/foo/feature2 in other package? And
what should shlibs report as feature1 provider? PN_feature1 or PN (which
also includes all /usr/lib/foo/feature1 files)?

Sorry but this idea seems wrong to me..

> Alternately it could be seen as a QA issue if a file could have been added
> to two packages (in which case a more strict pattern should have been used
> somewhere)
> 
> Frans

> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
Frans Meulenbroeks - Nov. 20, 2011, 1:11 p.m.
2011/11/19 Martin Jansa <martin.jansa@gmail.com>

> On Sat, Nov 19, 2011 at 04:02:49PM +0100, Frans Meulenbroeks wrote:
> > 2011/11/18 Paul Eggleton <paul.eggleton@linux.intel.com>
> >
> > > On Friday 18 November 2011 15:28:28 Frans Meulenbroeks wrote:
> > > > Is ordering of PACKAGES relevant? If so, please enlighten me why.
> > >
> > > It is. The packaging code reads this in order and the first package to
> > > match a
> > > file is the package that the file will go into; thus if you want to
> add a
> > > package that will include files that would normally be in one of the
> > > default
> > > packages you need to prepend to PACKAGES so that your new package gets
> the
> > > files first.
> > >
> > > Ah, ok. understood.
> > Not fully sure if the current behaviour is the most desirable one.
> > Is it still possible to put a file into two packages by explicitly adding
> > it to both?
>
> And then make resulting packages RCONFLICTing each other? Imho this is
> bad idea and it's good that this wasn't ever possible afaik.
>
I was thinking of the situation where a recipe could generate two (indeed
conflicting) packages. E.g. a minimal version and a more expanded version.
Or versions with different options enabled. Or a static and a
dynamic linked version (which still could share e.g. a common config file).
I can also imagine one wants (or must) add a file to all packages. This
could perhaps apply to some license files.

> Also it might be somewhat confusing that a line that adds say a dir to a
> package does not necessarily get all fies in that dir in the package
> Personally I think I prefer to have all added in that case, irrespective
if
> a previous package already took a file from it.

Why confusing?
> should only one package include all /usr/lib/foo and then separte
> subpackages package only some parts of it so we won't pull whole big
> /usr/lib/foo when we need in some image only
> /usr/lib/foo/feature1 and /usr/lib/foo/feature2 in other package? And
> what should shlibs report as feature1 provider? PN_feature1 or PN (which
> also includes all /usr/lib/foo/feature1 files)?
>
> Sorry but this idea seems wrong to me..
>
I'm not saying we should move into that direction, I'm more exporing
whether there is a use case for this and whether it is desired or not.

If it is felt that allowing adding a file to two packages is not good, then
I feel that if a file could be candidate for insertion of two packages,
this could be flagged as an error to be resolved in the recipe (by more
precisely specifying the package contents). That way ordering within
PACKAGES becomes irrelevant (and as such it removes a source of silent
errors, like the one that started this thread).

Frans.


> > Alternately it could be seen as a QA issue if a file could have been
> added
> > to two packages (in which case a more strict pattern should have been
> used
> > somewhere)
> >
> > Frans
>
> > _______________________________________________
> > Openembedded-core mailing list
> > Openembedded-core@lists.openembedded.org
> > http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
>
>
> --
> Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com
>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
>
>

Patch

diff --git a/meta/recipes-support/shared-mime-info/shared-mime-info.inc b/meta/recipes-support/shared-mime-info/shared-mime-info.inc
index 57e829c..d7fa67c 100644
--- a/meta/recipes-support/shared-mime-info/shared-mime-info.inc
+++ b/meta/recipes-support/shared-mime-info/shared-mime-info.inc
@@ -19,7 +19,7 @@  FILES_${PN}-dev += "${datadir}/pkgconfig/shared-mime-info.pc"
 
 # freedesktop.org.xml is only required when updating the mime database,
 # package it separately
-PACKAGES += "shared-mime-info-data"
+PACKAGES =+ "shared-mime-info-data"
 FILES_shared-mime-info-data = "${datadir}/mime/packages/freedesktop.org.xml"
 RDEPENDS_shared-mime-info-data = "shared-mime-info"
 
diff --git a/meta/recipes-support/shared-mime-info/shared-mime-info_0.91.bb b/meta/recipes-support/shared-mime-info/shared-mime-info_0.91.bb
index bc4a8f7..7ae6e10 100644
--- a/meta/recipes-support/shared-mime-info/shared-mime-info_0.91.bb
+++ b/meta/recipes-support/shared-mime-info/shared-mime-info_0.91.bb
@@ -1,5 +1,5 @@ 
 require shared-mime-info.inc
-PR = "r2"
+PR = "r3"
 
 SRC_URI[md5sum] = "982a211560ba4c47dc791ccff34e8fbc"
 SRC_URI[sha256sum] = "98cfebe1d809afb24934e634373821e2a1dfa86fc6462cab230589a1c80988bd"