Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#13293 closed Bug Report - General (fixed)

Parse config.xml as an XML document

Reported by: Nick Morrott Owned by: Bill Meek <billmeek@…>
Priority: minor Milestone: unknown
Component: Bindings - Perl Version: Master Head
Severity: low Keywords: perl xml config
Cc: Ticket locked: no

Description

The Perl bindings read the lines of config.xml as plain text strings, extracting the values of desired elements using matching (m##) and regex captures.

Ideally we would parse the document as XML and extract values directly from the data structure created by the XML parser.

Depending on the XML library(ies) used, this would also permit the configuration file to be validated against a suitable DTD/Schema.

Attachments (1)

perl-config-parser-v0.patch (5.2 KB) - added by Bill Meek 6 years ago.

Download all attachments as: .zip

Change History (8)

Changed 6 years ago by Bill Meek

Attachment: perl-config-parser-v0.patch added

comment:1 Changed 6 years ago by Bill Meek

Status: newinfoneeded_new

The attached is a non-Perl user's attempt at parsing config.xml properly.

It would catch the error from the mailing list that prompted this ticket.

If some folks want to kick the tires and report back here, I'll clean up the unused lines and commit it.

  • Catches duplicate tags
  • Catches missing tag ends
  • Missing tags
  • Proper comments are ignored
  • References to the mysql.txt names are gone

I picked XML::Simple, even though there are some recommendations against using it. But, there's commented out code here that uses XML::LibXML too. The big question is if all distributions have either of these. I've got both on a recently loaded Ubuntu 18.04 box. Don't know that I did anything to get them.

cd to wherever the existing MythTV.pm lives. Make a safe copy of it and apply the patch with:

sudo patch < /some/path/to/perl-config-parser-v0.patch

comment:2 Changed 6 years ago by ijc

FWIW (and my Perl _is_ a bit rusty) your code looks find to me. It should be possible to do the $mysql_conf{'db_host'} = $config->{'Database'}->{'Host'}; from within the preceeding loop which does the checks if you make each element of the list a tuple of XML name and local name, don't know if that will be any clearer though.

WRT which library to use looking in apt rdepends for both libxml-libxml-perl and libxml-simple-perl on my Debian system I see that libxml-libxml-perl is in the dependency chain for XMLTV (and a tonne of Perl libraries) while libxml-simple-perl has fewer rdepends and doesn't include anything which I immediately associated with being related to MythTV in some way. That's rather unscientific but given I would expect some sizeable fraction of myth systems to have xmltv installed perhaps that's enough to lean that way?

comment:3 Changed 6 years ago by ijc

Just looked on my actual mythbox and libxml-simple-perl seems to be a dependency of the mythweather package (for some of the grabbers) which might change the arithmetic a little? Apparently mythgame uses it too (in a contrib script)

comment:4 Changed 6 years ago by Bill Meek <billmeek@…>

Owner: set to Bill Meek <billmeek@…>
Resolution: fixed
Status: infoneeded_newclosed

In 9c5ad458b/mythtv:

Perl bindings: Actually parse config.xml

From the users list, where a line in config.xml was
'commented out' using a # rather than <!-- xxx -->.
Neither of which worked using the old method.

Potential problem if XML::Simple isn't available
in all distributions. Only Debian/Ubuntu/Mythbuntu?
were checked and libxml-simple-perl was there.

Also, removes all references to the old mysql.txt
DB..... names.

For reference: https://stackoverflow.com/questions/33267765/why-is-xmlsimple-discouraged
probably doesn't apply here as config.xml is truly
simple.

Closes #13293

comment:5 Changed 6 years ago by Bill Meek <billmeek@…>

In 9c5ad458b/mythtv:

Perl bindings: Actually parse config.xml

From the users list, where a line in config.xml was
'commented out' using a # rather than <!-- xxx -->.
Neither of which worked using the old method.

Potential problem if XML::Simple isn't available
in all distributions. Only Debian/Ubuntu/Mythbuntu?
were checked and libxml-simple-perl was there.

Also, removes all references to the old mysql.txt
DB..... names.

For reference: https://stackoverflow.com/questions/33267765/why-is-xmlsimple-discouraged
probably doesn't apply here as config.xml is truly
simple.

Closes #13293

comment:6 Changed 6 years ago by Bill Meek <billmeek@…>

In 9c5ad458b/mythtv:

Perl bindings: Actually parse config.xml

From the users list, where a line in config.xml was
'commented out' using a # rather than <!-- xxx -->.
Neither of which worked using the old method.

Potential problem if XML::Simple isn't available
in all distributions. Only Debian/Ubuntu/Mythbuntu?
were checked and libxml-simple-perl was there.

Also, removes all references to the old mysql.txt
DB..... names.

For reference: https://stackoverflow.com/questions/33267765/why-is-xmlsimple-discouraged
probably doesn't apply here as config.xml is truly
simple.

Closes #13293

comment:7 Changed 6 years ago by Bill Meek <billmeek@…>

In 9c5ad458b/mythtv:

Perl bindings: Actually parse config.xml

From the users list, where a line in config.xml was
'commented out' using a # rather than <!-- xxx -->.
Neither of which worked using the old method.

Potential problem if XML::Simple isn't available
in all distributions. Only Debian/Ubuntu/Mythbuntu?
were checked and libxml-simple-perl was there.

Also, removes all references to the old mysql.txt
DB..... names.

For reference: https://stackoverflow.com/questions/33267765/why-is-xmlsimple-discouraged
probably doesn't apply here as config.xml is truly
simple.

Closes #13293

Note: See TracTickets for help on using tickets.