Re: [libosinfo PATCH v2 0/8] Use "all" arch as a fallback for media/tree detection

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Apr 1, 2019 at 9:13 PM Fabiano Fidêncio <fidencio@xxxxxxxxxx> wrote:
>
> On Mon, Apr 1, 2019 at 8:36 PM Cole Robinson <crobinso@xxxxxxxxxx> wrote:
> >
> > On 3/28/19 5:26 PM, Fabiano Fidêncio wrote:
> > > This series has been written considering:
> > > - https://www.redhat.com/archives/libosinfo/2019-February/msg00247.html
> > >
> > > Let's assume that trees and medias set with architecture "all" will be
> > > treated as fallback, always.
> > >
> > > https://gitlab.com/libosinfo/libosinfo/issues/20
> > >
> > > Fabiano Fidêncio (8):
> > >   db: Rename tree to treeinfo in guess_os_from_tree()
> > >   db: Rename os_* to os_treeinfo_* in guess_os_from_tree()
> > >   db: Consider the tree arch when guessing an OS from tree
> > >   db: Deal with "all" tree architectures
> > >   test-db: Add test for guessing tree from OS
> > >   db: Consider the media arch when guess an OS from media
> > >   db: Deal with "all" media architecture
> > >   test-db: Add test to cover identifying a media with "unknown" arch
> > >
> > >  osinfo/osinfo_db.c                            | 184 ++++++++++++------
> > >  .../dbdata/os/libosinfo.org/test-db-media.xml |  14 ++
> > >  .../dbdata/os/libosinfo.org/test-db-tree.xml  |  25 +++
> > >  tests/test-db.c                               |  78 ++++++++
> > >  4 files changed, 237 insertions(+), 64 deletions(-)
> > >  create mode 100644 tests/dbdata/os/libosinfo.org/test-db-tree.xml
> > >
> >
> > Reviewed-by: Cole Robinson <crobinso@xxxxxxxxxx>
> >
> > I tested with my rhel5 arch='unknown' patchset switched to arch='all'
> > and things seemed to work too.
> >
> >
> > Separate from this patchset I've been thinking about tree testing in
> > general and I think before the next release we need:
> >
> > - A functional test to run every <tree><url> with <treeinfo> through
> > libosinfo code to ensure that our logic actually detects the correct
> > short-id and tree arch.
>
> Yep, a functional test could be added to our tools, using osinfo-detect tools.
> It can be done using python (in the same way we've done for osinfo-db).
>
> I'm currently trying to work on something similar for osinfo-db-tools
> and libosinfo would be my next target;
>
> >
> > - Populate the osinfo-db test suite with treeinfo files similar to what
> > we do with isoinfo output and do unit testing that way.
>
> Agreed.
>
> >
> > It scares me a little that an overly liberal regex added to osinfo-db
> > could cause tree detection regressions by matching the newly added db
> > entry. Partly it makes me think that we should actually attempt to match
> > against every <treeinfo> or <media> in osinfo-db and return the one with
> > the most matching fields. This would let us make fedora-unknown just
> > have a <treeinfo><family>*Fedora*<... value and not need to worry about
> > it also matching other Fedora versions since those will have more than
> > one field. It will also reduce the chance of accidental collision with
> > newly added <os>. Just some ideas...
>
> Hmmm. I like the idea, but it'd make things slightly slower.
> In order to do so we could add some weight on every match we have,
> sort the matches by the weight and just return the heaviest one.
> Do-able.
>
> Daniel, I'd like to hear from you here.
>
> Best Regards,
> --
> Fabiano Fidêncio

Hmm. Cole found an issue with the series, so I'll hold till it's fixed.

Basically, when calling create_media_from_location(), when actually
creating the media, we'd hit the following code path:
```
    media = g_object_new(OSINFO_TYPE_MEDIA,
                         "id", uri,
                         NULL);

```
(same codepath can be seen at:
https://gitlab.com/libosinfo/libosinfo/blob/master/osinfo/osinfo_media.c#L814)

It means that *all* medias are created *without* having an
architecture set. Which basically changes everything for the media
related patches, because we *cannot* rely on comparing architectures.

I'll re-work the patches taking into consideration this limitation and
submit a v3.

Best Regards,
--
Fabiano Fidêncio

_______________________________________________
Libosinfo mailing list
Libosinfo@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libosinfo




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Fedora Users]     [Fedora Maintainers]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]

  Powered by Linux