[elbe-devel] [PATCH 2/5] Add ISO files options for source cdroms

[Kurt Kanzenbach]

On Mon, Jul 15, 2019 at 11:21:59AM +0200, dion at linutronix.de wrote:
> From: Olivier Dion <dion at linutronix.de>
>
> See https://wiki.osdev.org/ISO_9660
>
> * get_iso_option
>
>   Takes a valid XML tree and iterate over the tags under the path
>   'target/src-opts/*'.  For each good tag, the tag's text is truncated
>   with the corresponding limit for the option has described by
>   ISO-9660.  The function return a valid string for the command line.
>
> * iso_option_valid
>
>   Take an option name and its text.  If the option is uknown, return
>   False.  If a character in the option' text is invalid return the
>   character.  Otherwise return True.
>
> Signed-off-by: Olivier Dion <dion at linutronix.de>

Reviewed-by: Kurt Kanzenbach <kurt at linutronix.de>

Anyway, one comment below.

> ---
>  elbepack/cdroms.py | 46 +++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 45 insertions(+), 1 deletion(-)
>
> diff --git a/elbepack/cdroms.py b/elbepack/cdroms.py
> index 3f2ecc21..ab5b6a23 100644
> --- a/elbepack/cdroms.py
> +++ b/elbepack/cdroms.py
> @@ -21,6 +21,45 @@ from elbepack.shellhelper import CommandError
>  CDROM_SIZE = 640 * 1000 * 1000
>
>
> +# https://wiki.osdev.org/ISO_9660
> +iso_options = {
> +    "sysid":     ("-sysid",      32, "Specifies the system ID",              "strA"),
> +    "volid":     ("-V",          32, "Specifies the volume ID",              "strD"),
> +    "volset":    ("-volset",    128, "Specifies the volume set ID",          "strD"),
> +    "publisher": ("-publisher", 128, "Specifies the publisher ID",           "strA"),
> +    "preparer":  ("-p",         128, "Specifies the preparer ID",            "strA"),
> +    "app":       ("-A",         128, "Specifies the application ID",         "strA"),
> +    "copyright": ("-copyright",  38, "Specifies copyright filename on disc", "strD"),
> +    "abstract":  ("-abstract",   36, "Specifies the abstract filename",      "strD"),
> +    "biblio":    ("-biblio",     37, "Specifies the bibliographic filename", "strD"),
> +}
> +
> +encoding = {
> +    "strA":"""ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_!"%&'()*+,-./:;<=>? """,
> +    "strD":"""ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_"""
> +}
> +def iso_option_valid(opt_name, text):
> +    if opt_name not in iso_options:
> +        return False
> +    str_type = encoding[iso_options[opt_name][3]]

Why not use hashes consistently? I'd prefer something like

 str_type = encoding[iso_options[opt_name]["str_type"]]

> +    if len(text) > iso_options[opt_name][1]:

 if len(text) > iso_options[opt_name]["len"]:

and so on. It makes it easier to read.

> +        return len(text) - iso_options[opt_name][1]
> +    for c in text:
> +        if c not in str_type:
> +            return c
> +    return True
> +
> +def get_iso_options(log, xml):
> +    options = []
> +    for node in xml.node("target/src-opts"):
> +        if node.tag not in iso_options:
> +            continue
> +        option = iso_options[node.tag]
> +        log.printo("Adding option %s\n%s" % (node.tag, option[2]))
> +        text = node.et.text[:option[1]]
> +        options.append('%s "%s"' % (option[0], text.replace('"', '\\"')))
> +    return " ".join(options)
> +
>  def mk_source_cdrom(
>          rfs,
>          arch,
> @@ -102,7 +141,12 @@ def mk_source_cdrom(
>
>      repo.finalize()
>
> -    return repo.buildiso(os.path.join(target, "src-cdrom.iso"))
> +    if xml is not None:
> +        options = get_iso_options(log, xml)
> +    else:
> +        options = ""
> +
> +    return repo.buildiso(os.path.join(target, "src-cdrom.iso"), options=options)
>
>  def mk_binary_cdrom(
>          rfs,
> --
> 2.11.0
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.linutronix.de/pipermail/elbe-devel/attachments/20190717/03d37cc5/attachment.sig>