Re: [gentoo-portage-dev] [PATCH 2/2] xml-test/missing-attributes: missing mandatory attributes for various tags

[Göktürk Yüksek]

Brian Dolbec:
> On Sun, 1 May 2016 18:38:50 -0700
> Brian Dolbec <dolsen@g.o> wrote:
>
>> On Sun,  1 May 2016 21:08:47 -0400
>> Göktürk Yüksek <gokturk@××××××××××.edu> wrote:
>>
>>> Check for:
>>> - type attribute in <maintainer/>
>>> - name attribute in <slots/>
>>> - name attribute in <flag/>
>>> - type attribute for <remote-id/>
>>>
>>> Signed-off-by: Göktürk Yüksek <gokturk@××××××××××.edu>
>>> ---
>>>  xml-test/missing-attributes/metadata.xml           | 23
>>> ++++++++++++++++++++++ .../missing-attributes-0.1.ebuild
>>> | 13 ++++++++++++ 2 files changed, 36 insertions(+)
>>>  create mode 100644 xml-test/missing-attributes/metadata.xml
>>>  create mode 100644
>>> xml-test/missing-attributes/missing-attributes-0.1.ebuild
>>>
>>> diff --git a/xml-test/missing-attributes/metadata.xml
>>> b/xml-test/missing-attributes/metadata.xml new file mode 100644
>>> index 0000000..cf51afd
>>> --- /dev/null
>>> +++ b/xml-test/missing-attributes/metadata.xml
>>> @@ -0,0 +1,23 @@
>>> +<?xml version="1.0" encoding="UTF-8"?>
>>> +<!DOCTYPE pkgmetadata SYSTEM
>>> "http://www.gentoo.org/dtd/metadata.dtd"> +<pkgmetadata>
>>> +  <!-- Missing attribute "type" -->
>>> +  <maintainer>
>>> +    <email>foodev@g.o</email>
>>> +  </maintainer>
>>> +  <slots>
>>> +    <!-- Missing attribute "name" -->
>>> +    <slot>Bar slot</slot>
>>> +  </slots>
>>> +  <use>
>>> +    <!-- Missing attribute "name" -->
>>> +    <flag>Baz flag</flag>
>>> +  </use>
>>> +  <upstream>
>>> +    <!-- Missing attribute "type" -->
>>> +    <remote-id>gentoo/gen-b0rk</remote-id>
>>> +  </upstream>
>>> +  <longdescription>
>>> +    Tests for mandatory attributes for various tags
>>> +  </longdescription>
>>> +</pkgmetadata>
>>> diff --git
>>> a/xml-test/missing-attributes/missing-attributes-0.1.ebuild
>>> b/xml-test/missing-attributes/missing-attributes-0.1.ebuild new file
>>> mode 100644 index 0000000..5733009 --- /dev/null
>>> +++ b/xml-test/missing-attributes/missing-attributes-0.1.ebuild
>>> @@ -0,0 +1,13 @@
>>> +# Copyright 1999-2016 Gentoo Foundation
>>> +# Distributed under the terms of the GNU General Public License v2
>>> +# $Id$
>>> +
>>> +EAPI=6
>>> +
>>> +DESCRIPTION="Missing various mandatory attributes in the metadata"
>>> +HOMEPAGE="https://wiki.gentoo.org/wiki/GLEP:68"
>>> +SRC_URI=""
>>> +
>>> +LICENSE="HPND"
>>> +SLOT="0"
>>> +KEYWORDS="~amd64"
>>
>> Thank you, push it please :)
>>
>
> Am I getting this correct?  There are 4 errors in this metadata.xml?
>
> If that is the case, I am getting only 1 error reported for both 2.2.28
> and the master branch code.
>
> master branch code:
>
> brian@professor-x ~/Dev/git/gen-b0rk/xml-test $ repom full
>
> RepoMan scours the neighborhood...
>   metadata.bad [fatal]          1
>    xml-test/missing-attributes/metadata.xml: missing 'name' attribute
> for 'flag' tag metadata.missing [fatal]      1
>    xml-test/missing/metadata.xml
>
> Note: use --include-dev (-d) to check dependencies for 'dev' profiles
>
> Please fix these important QA issues first.
> RepoMan sez: "Make your QA payment on time and you'll never see the
> likes of me."
>
>
>
> Portage-2.2.28 release:
>
> brian@professor-x ~/Dev/git/gen-b0rk/xml-test $ repoman full
>
> RepoMan scours the neighborhood...
>   metadata.bad [fatal]          1
>    xml-test/missing-attributes/metadata.xml: missing 'name' attribute
> for 'flag' tag metadata.missing [fatal]      1
>    xml-test/missing/metadata.xml
>
> Note: use --include-dev (-d) to check dependencies for 'dev' profiles
>
> Please fix these important QA issues first.
> RepoMan sez: "Make your QA payment on time and you'll never see the
> likes of me."
>
> brian@professor-x ~/Dev/git/gen-b0rk/xml-test $
>
>

xmllint seems to catch 3 out of 4 for DTD and all 4 for xsd:

$ xmllint --nonet --noout --dtdvalid /usr/portage/distfiles/metadata.dtd
metadata.xml
metadata.xml:10: element slot: validity error : Element slot does not
carry attribute name
metadata.xml:14: element flag: validity error : Element flag does not
carry attribute name
metadata.xml:18: element remote-id: validity error : Element remote-id
does not carry attribute type
Document metadata.xml does not validate against
/usr/portage/distfiles/metadata.dtd

$ xmllint --nonet --noout --schema /usr/portage/distfiles/metadata.xsd
metadata.xml
metadata.xml:5: element maintainer: Schemas validity error : Element
'maintainer': The attribute 'type' is required but missing.
metadata.xml:10: element slot: Schemas validity error : Element 'slot':
The attribute 'name' is required but missing.
metadata.xml:14: element flag: Schemas validity error : Element 'flag':
The attribute 'name' is required but missing.
metadata.xml:18: element remote-id: Schemas validity error : Element
'remote-id': The attribute 'type' is required but missing.
metadata.xml fails to validate

If I supply <flag name="baz"> and run repom again, it prints all the errors:

$ repom full

RepoMan scours the neighborhood...
!!! metadata.xml is invalid:
!!!
/home/gokturk/stuff/gen-b0rk/xml-test/missing-attributes/metadata.xml:5:
element maintainer: Schemas validity error : Element 'maintainer': The
attribute 'type' is required but missing.
!!!
/home/gokturk/stuff/gen-b0rk/xml-test/missing-attributes/metadata.xml:10: element
slot: Schemas validity error : Element 'slot': The attribute 'name' is
required but missing.
!!!
/home/gokturk/stuff/gen-b0rk/xml-test/missing-attributes/metadata.xml:18: element
remote-id: Schemas validity error : Element 'remote-id': The attribute
'type' is required but missing.
!!!
/home/gokturk/stuff/gen-b0rk/xml-test/missing-attributes/metadata.xml
fails to validate
  metadata.bad [fatal]          1
   xml-test/missing-attributes/metadata.xml
  metadata.warning              1
   xml-test/missing-attributes/metadata.xml: unused local
USE-description: 'baz'

Note: use --include-dev (-d) to check dependencies for 'dev' profiles

Please fix these important QA issues first.
RepoMan sez: "Make your QA payment on time and you'll never see the
likes of me."

I am looking at the code and looks like there's an early fail in
pkgmetadata.py:

93 try:
94 _metadata_xml = xml.etree.ElementTree.parse(
95 _unicode_encode(
96 os.path.join(checkdir, "metadata.xml"),
97 encoding=_encodings['fs'], errors='strict'),
98 parser=xml_parser)
99 except (ExpatError, SyntaxError, EnvironmentError) as e:
100 metadata_bad = True
101 self.qatracker.add_error("metadata.bad", "%s/metadata.xml: %s" %
(xpkg, e))

I do not know why there's a difference in behavior. If it early-fails
for missing name in <flag>, it should behave the same for missing name
in <slot>. I'll dig into it more when I have time.