[Libosinfo] [PATCH osinfo-db 2/6] Add scripts/osinfo-db-add-iso.py

[Fabiano Fidêncio]

On Fri, Mar 29, 2019 at 4:46 PM Cole Robinson <crobinso at redhat.com> wrote:
>
> On 3/29/19 5:14 AM, Fabiano Fidêncio wrote:
> > On Wed, 2019-03-27 at 21:08 -0400, Cole Robinson wrote:
> >> This script simplifies the process of adding new iso data to the
> >> test suite and optionally filling in a <media> block for the <os>.
> >> Call it like
> >>
> >>   ./scripts/osinfo-db-add-iso.py SHORTID [--arch ARCH] ISOPATH
> >>
> >> It will print a <media> block template to stdout, and generate a
> >> correctly named data file in tests/isodata/
> >>
> >> Signed-off-by: Cole Robinson <crobinso at redhat.com>
> >> ---
> >>  scripts/osinfo-db-add-iso.py | 113
> >> +++++++++++++++++++++++++++++++++++
> >>  1 file changed, 113 insertions(+)
> >>  create mode 100755 scripts/osinfo-db-add-iso.py
> >>
> >> diff --git a/scripts/osinfo-db-add-iso.py b/scripts/osinfo-db-add-
> >> iso.py
> >> new file mode 100755
> >> index 0000000..8a16859
> >> --- /dev/null
> >> +++ b/scripts/osinfo-db-add-iso.py
> >> @@ -0,0 +1,113 @@
> >> +#!/usr/bin/env python3
> >> +
> >> +import argparse
> >> +import distutils.spawn
> >> +import os
> >> +import sys
> >> +import tempfile
> >> +import time
> >> +
> >> +
> >> +topdir = os.path.realpath(os.path.join(os.path.dirname(__file__),
> >> ".."))
> >> +datadir = os.path.join(topdir, "data")
> >> +sys.path.insert(0, topdir)
> >> +os.environ["INTERNAL_OSINFO_DB_DATA_DIR"] = datadir
> >> +
> >> +import tests.isodata
> >> +import tests.util
> >> +
> >> +
> >> +def fail(msg):
> >> +    print(msg)
> >> +    sys.exit(1)
> >> +
> >> +
> >> +##############################
> >> +# main() and option handling #
> >> +##############################
> >> +
> >> +def _parse_args():
> >> +    desc = ("Helper script for adding iso test data to the test "
> >> +            "suite, and matching <os> <media> data to the DB")
> >> +    parser = argparse.ArgumentParser(description=desc)
> >> +
> >> +    parser.add_argument("shortid", help="Which <os> short-id "
> >> +            "the ISO is media for")
> >> +    parser.add_argument("iso", help="The path to the ISO media")
> >> +    parser.add_argument("--arch", default="x86_64",
> >> +            help="The OS architecture the media is for.
> >> default=x86_64")
> >> +
> >> +    options = parser.parse_args()
> >> +    return options
> >> +
> >> +
> >> +def _main():
> >> +    """
> >> +    This is a template for new command line programs. Copy and edit
> >> it!
> >> +    """
> >> +    options = _parse_args()
> >> +
> >> +    iso = os.path.realpath(os.path.abspath(options.iso))
> >> +    isoinfobin = distutils.spawn.find_executable("isoinfo")
> >> +    if not os.path.exists(iso):
> >> +        fail("iso does not exist: %s" % iso)
> >> +    if not isoinfobin:
> >> +        fail("isoinfo is not installed")
> >> +
> >> +    osxml = None
> >> +    for o in tests.util.DataFiles.oses():
> >> +        if o.shortid == options.shortid:
> >> +            osxml = o
> >> +            break
> >> +    if not osxml:
> >> +        fail("Did not find any os shortid=%s" % options.shortid)
> >> +        return
> >
> > If I understand correctly, it'd simply abort in case I try to add a new
> > version of an existing OS, right? For instance, the script would bail
> > when trying to add the first fedora30 instance.
> >
> > It's something we can definitely improve in the feature.
> >
>
> Yes if we tried to do ./scripts/osinfo-db-add-iso.py fedora30 $ISOPATH,
> it will hit this error as fedora30 doesn't exist yet. I think this is
> good to reject accidental typos.
>
> I don't quite follow why we'd want to add isodata before the OS has been
> added to the DB though, seems simple enough to just swap the patch
> ordering. If we want to allow what you suggest we would also need to add
> a --distro option since we won't be able to pull it out of an osxml instance

What I had in mind was not about adding the isodata before adding the
OS, but using the isodata to add the new OS entry.
So, even before adding the fedora30 media, I'd like to run the script
and have the fedora30 media entries already done. :-)

But, anyways, that's something that can be improved in the future,  in
case it makes sense.

>
> Thanks,
> Cole