Bug 728837 (xml2dict) - Review Request: xml2dict - Use attributes of dictionary to access xml elements.
Summary: Review Request: xml2dict - Use attributes of dictionary to access xml elements.
Keywords:
Status: CLOSED NEXTRELEASE
Alias: xml2dict
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Parag AN(पराग)
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: Nitrate
TreeView+ depends on / blocked
 
Reported: 2011-08-08 05:31 UTC by Yuguang Wang
Modified: 2012-03-20 02:37 UTC (History)
10 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-03-20 02:37:33 UTC
panemade: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Yuguang Wang 2011-08-08 05:31:04 UTC
Spec URL: http://yuwang.fedorapeople.org/xml2dict.spec

Description: Use attributes of dictionary to access xml elements.

Example as below:
from xml2dict import XML2Dict

if __name__ == '__main__':
    s = """<?xml version="1.0" encoding="utf-8" ?>
    <result>
        <count n="1">10</count>
        <data><id>491691</id><name>test</name></data>
        <data><id>491692</id><name>test2</name></data>
        <data><id>503938</id><name>hello, world</name></data>
    </result>"""

    xml = XML2Dict()
    r = xml.fromstring(s)
    from pprint import pprint
    pprint(r)
    print r.result.count.value
    print r.result.count.n

    for data in r.result.data:
        print data.id, data.name 
    pprint(xml.parse('a'))

Comment 1 Yuguang Wang 2011-08-08 05:31:46 UTC
rpmlint result:
[yuwang@yuwang xml2dict]$ rpmlint xml2dict.spec 
0 packages and 1 specfiles checked; 0 errors, 0 warnings.

Comment 2 Yuguang Wang 2011-08-08 05:36:44 UTC
Wow, I'd say the package from googlecode has no 'setup.py' thus unable to install, I wrote one for it, and the source rpm is available here:
http://yuwang.fedorapeople.org/xml2dict-1.0-1.fc13.src.rpm

Running rpmlint ok.

Comment 3 T.C. Hollingsworth 2011-08-12 02:50:05 UTC
Your SRPM fails to build because the directory name provided to the %setup macro is incorrect.  See http://koji.fedoraproject.org/koji/taskinfo?taskID=3267140 and http://koji.fedoraproject.org/koji/getfile?taskID=3267143&name=build.log for details.

This package should be called python-xml2dict in accordance with the Python module naming guidelines:  https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Addon_Packages_.28python_modules.29

Your package is claiming ownership of the entire Python site-packages directory.  Please change the "%{python_sitelib}/" in "%files" to "%{python_sitelib}/*" so you only claim ownership of files added there.  Additionally, the "%{python_sitelib}/xml2dict*.egg-info" entry is superfluous; those files will be included by "%{python_sitelib}/*"

Comment 4 T.C. Hollingsworth 2011-08-12 03:16:34 UTC
Please ignore the bit about "%{python_sitelib/xml2dict*.egg-info", as it actually is useful in that it makes sure an egg-info file is built.  Sorry about that.

Comment 5 Yuguang Wang 2011-08-23 03:18:25 UTC
Thanks for figuring out this. Package renamed and spec file and source available here:

http://yuwang.fedorapeople.org/python-xml2dict-1.0.tar.gz
http://yuwang.fedorapeople.org/python-xml2dict.spec
$ rpmlint python-xml2dict.spec 
0 packages and 1 specfiles checked; 0 errors, 0 warnings.
$ rpmlint python-xml2dict-1.0-1.fc13.noarch.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

(In reply to comment #3)
> Your SRPM fails to build because the directory name provided to the %setup
> macro is incorrect.  See
> http://koji.fedoraproject.org/koji/taskinfo?taskID=3267140 and
> http://koji.fedoraproject.org/koji/getfile?taskID=3267143&name=build.log for
> details.
> 
> This package should be called python-xml2dict in accordance with the Python
> module naming guidelines: 
> https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Addon_Packages_.28python_modules.29
> 
> Your package is claiming ownership of the entire Python site-packages
> directory.  Please change the "%{python_sitelib}/" in "%files" to
> "%{python_sitelib}/*" so you only claim ownership of files added there. 
> Additionally, the "%{python_sitelib}/xml2dict*.egg-info" entry is superfluous;
> those files will be included by "%{python_sitelib}/*"

Comment 6 Yuguang Wang 2011-08-23 05:06:17 UTC
source rpm available here:
http://yuwang.fedorapeople.org/python-xml2dict-1.0-1.fc13.src.rpm

Comment 7 Jens Petersen 2011-09-02 09:17:39 UTC
Also here please post the package with the original upstream tarball
(ie don't modify it with the spec file).

Comment 8 Jens Petersen 2011-09-29 07:13:19 UTC
Ping?

Comment 9 Yuguang Wang 2011-09-30 03:10:25 UTC
Source rpm updated:
http://yuwang.fedorapeople.org/python-xml2dict.spec
http://yuwang.fedorapeople.org/python-xml2dict-1.0-1.fc13.src.rpm

I've renamed the package:
http://xml2dict.googlecode.com/files/xml2dict-2008.6-tar.gz
xml2dict-2008.6-tar.gz --> python-xml2dict-1.0.tar.gz

Thanks.

Comment 10 Till Maas 2011-11-19 21:13:19 UTC
- renaming the upstream tarball is not an option. Please include it as xml2dict-2008.6-tar.gz
- Also you need to use the version specified by upstream and not use 1.0 as version. You might use 0 as version and use 2009.6 in the release as documented here: http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Pre-Release_packages

Comment 11 Parag AN(पराग) 2011-11-21 06:22:03 UTC
suggestions:
1) use source URL as
http://xml2dict.googlecode.com/files/xml2dict-%{alphatag}-tar.gz

and define

%global alphatag 2008.6

at top of your spec


2) use release tag as 0.%{1}.%{alphatag}


While looking into source code I realised that I have seen similar tarball somewhere and found it at https://nodeload.github.com/mcspring/XML2Dict/tarball/master
with its project page
https://github.com/mcspring/XML2Dict

I see above project is updated and using license ASL 2.0 while your one is BSD

Comment 12 Parag AN(पराग) 2011-12-02 04:51:49 UTC
ping

Comment 13 Parag AN(पराग) 2011-12-12 09:49:24 UTC
ping

Comment 14 Parag AN(पराग) 2011-12-21 10:19:15 UTC
ping

Comment 15 Yuguang Wang 2011-12-22 02:29:00 UTC
oh, thanks for reminder.(I've missed the update)

The 2008.6 in pkg name puzzled me a lot, thank you!!!
(In reply to comment #11)
> suggestions:
> 1) use source URL as
> http://xml2dict.googlecode.com/files/xml2dict-%{alphatag}-tar.gz
> 
> and define
> 
> %global alphatag 2008.6
> 
> at top of your spec
> 
> 
> 2) use release tag as 0.%{1}.%{alphatag}
> 
> 
> While looking into source code I realised that I have seen similar tarball
> somewhere and found it at
> https://nodeload.github.com/mcspring/XML2Dict/tarball/master
> with its project page
> https://github.com/mcspring/XML2Dict
> 
> I see above project is updated and using license ASL 2.0 while your one is BSD

Comment 16 Yuguang Wang 2011-12-22 03:31:02 UTC
Spec URL: http://yuwang.fedorapeople.org/xml2dict.spec
SRPM: http://yuwang.fedorapeople.org/xml2dict-0-0.1.2008.6.src.rpm

(In reply to comment #11)
I've also noticed this project before, but unfortunately it's nothing to do with http://code.google.com/p/xml2dict/
and I updated the license with GPLv2 according to the project homepage.
> While looking into source code I realised that I have seen similar tarball
> somewhere and found it at
> https://nodeload.github.com/mcspring/XML2Dict/tarball/master
> with its project page
> https://github.com/mcspring/XML2Dict
> 
> I see above project is updated and using license ASL 2.0 while your one is BSD

BTW, Move package name from python-xml2dict back to xml2dict according to upstream, thanks for your suggestion.
(In reply to comment #10)
> - renaming the upstream tarball is not an option. Please include it as
> xml2dict-2008.6-tar.gz
> - Also you need to use the version specified by upstream and not use 1.0 as
> version. You might use 0 as version and use 2009.6 in the release as documented
> here:
> http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Pre-Release_packages

Any other problems please let me know.
Thanks.

Comment 17 Yuguang Wang 2012-01-16 11:25:57 UTC
ping?

Comment 18 Parag AN(पराग) 2012-01-16 13:45:42 UTC
oops! sorry last time I checked the updated srpm it failed to build on koji and I forgot to comment the same here.

I have done the scratch build again which failed to build 
http://koji.fedoraproject.org/koji/taskinfo?taskID=3705755

Comment 19 Parag AN(पराग) 2012-01-16 13:54:39 UTC
1) in %setup you need to use
%setup -q -n %{name}-read-only

2)then there is no PKG-INFO in upstream source tarball so remove following
 sed -i "s|\r||g" PKG-INFO

3) then in %build, build command failed as there is no setup.py in tarball
also CFLAGS is not needed for noarch package.

4) %clean section not needed now in Fedora spec.

Please submit spec according to what is upstream providing and how to package it.

Comment 20 Parag AN(पराग) 2012-02-02 06:57:46 UTC
Ping

Comment 21 Yuguang Wang 2012-02-24 03:14:03 UTC
wow, was busy with other stuffs. I'll try to update next week.

Comment 22 Yuguang Wang 2012-03-02 06:11:30 UTC
Issues in comment 17 all fixed:
Spec URL: http://yuwang.fedorapeople.org/xml2dict.spec
SRPM: http://yuwang.fedorapeople.org/xml2dict-0-0.3.2008.6.src.rpm

$ rpmlint xml2dict-0-0.3.2008.6.src.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

Thanks.

Comment 23 Yuguang Wang 2012-03-14 03:13:35 UTC
ping, I made a koji scratch build here:

[yuwang@yuwang ~]$ koji build --scratch dist-rawhide /home/yuwang/rpmbuild/SRPMS/xml2dict-0-0.3.2008.6.src.rpm 
Uploading srpm: /home/yuwang/rpmbuild/SRPMS/xml2dict-0-0.3.2008.6.src.rpm
[====================================] 100% 00:00:01   5.32 KiB   3.68 KiB/sec
Created task: 3892856
Task info: http://koji.fedoraproject.org/koji/taskinfo?taskID=3892856
Watching tasks (this may be safely interrupted)...
3892856 build (dist-rawhide, xml2dict-0-0.3.2008.6.src.rpm): open (ppc12.phx2.fedoraproject.org)
  3892857 buildArch (xml2dict-0-0.3.2008.6.src.rpm, noarch): open (x86-12.phx2.fedoraproject.org)
3892856 build (dist-rawhide, xml2dict-0-0.3.2008.6.src.rpm): open (ppc12.phx2.fedoraproject.org) -> closed
  0 free  1 open  1 done  0 failed
  3892857 buildArch (xml2dict-0-0.3.2008.6.src.rpm, noarch): open (x86-12.phx2.fedoraproject.org) -> closed
  0 free  0 open  2 done  0 failed

3892856 build (dist-rawhide, xml2dict-0-0.3.2008.6.src.rpm) completed successfully

http://koji.fedoraproject.org/koji/taskinfo?taskID=3892856

Comment 24 Parag AN(पराग) 2012-03-15 03:42:20 UTC
Review:
+ rpmlint on rpms gave
xml2dict.noarch: W: incoherent-version-in-changelog 0-0.3 ['0-0.3.2008.6', '0-0.3.2008.6']
2 packages and 0 specfiles checked; 0 errors, 1 warnings.

+ source verified with upstream as
e976006f120d7dd79a7c1a2a310d2e2fbe237d54  xml2dict-2008.6-tar.gz
e976006f120d7dd79a7c1a2a310d2e2fbe237d54  ../SOURCES/xml2dict-2008.6-tar.gz

Suggestions:
1) No need in Fedora now to write
%defattr(-,root,root,-)

2) you should ask upstream to add license headers in .py files or a separate license file in tarball.

3) but your installation looks strange. What is need to create empty directory
install -d $RPM_BUILD_ROOT%{python_sitelib}/xml2dict

Comment 25 Yuguang Wang 2012-03-15 05:22:25 UTC
(In reply to comment #24)
I'll send mail to the author to see if there's any response.
> 2) you should ask upstream to add license headers in .py files or a separate
> license file in tarball.

For 1) and 3), will fix asap after got response from the author.

Comment 26 Yuguang Wang 2012-03-19 02:52:39 UTC
Hello, license updated according to the author, spec file & srpm:
http://yuwang.fedorapeople.org/xml2dict.spec
http://yuwang.fedorapeople.org/xml2dict-0-0.3.2008.6.1.src.rpm

rpmlint:
[yuwang@yuwang specs(master)]$ rpmlint /home/yuwang/rpmbuild/SRPMS/xml2dict-0-0.3.2008.6.1.src.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

koji scratch build successfully:
http://koji.fedoraproject.org/koji/taskinfo?taskID=3907929

Comment 27 Yuguang Wang 2012-03-19 02:56:42 UTC
(In reply to comment #24)
I removed the %{python_sitelib}/xml2dict in %files section.
> 3) but your installation looks strange. What is need to create empty directory
> install -d $RPM_BUILD_ROOT%{python_sitelib}/xml2dict

but If the above line is removed, there'd be build error, I've no idea why.
btw, there's no empty directory {python_sitelib}/xml2dict after installation.

Comment 28 Parag AN(पराग) 2012-03-19 06:52:45 UTC
See you are installing 
install -pm 0644 xml2dict.py $RPM_BUILD_ROOT%{python_sitelib}/
install -pm 0644 object_dict.py $RPM_BUILD_ROOT%{python_sitelib}/

But to install above files you must have first build root path created which is currently done by
install -d $RPM_BUILD_ROOT%{python_sitelib}/xml2dict

so all the directory hierarchy is created using above command but you have to actually use this command
install -d $RPM_BUILD_ROOT%{python_sitelib}/

otherwise you will install unnecessary directory $RPM_BUILD_ROOT%{python_sitelib}/xml2dict

Comment 29 Parag AN(पराग) 2012-03-19 06:54:30 UTC
anyway I see all things are fixed.

APPROVED.

Comment 30 Yuguang Wang 2012-03-19 07:18:14 UTC
(In reply to comment #29)
> anyway I see all things are fixed.
> 
> APPROVED.

Thank you!

Comment 31 Parag AN(पराग) 2012-03-19 09:29:27 UTC
Can you please add the SCM request? Looks like you missed it. And don't forget to follow my suggestion in comment#28 when importing this package in Fedora.

Comment 32 Yuguang Wang 2012-03-19 09:42:07 UTC
(In reply to comment #31)
> Can you please add the SCM request? Looks like you missed it. And don't forget
> to follow my suggestion in comment#28 when importing this package in Fedora.
okay, already updated the spec according to your comment#28
http://koji.fedoraproject.org/koji/taskinfo?taskID=3908575

Comment 33 Yuguang Wang 2012-03-19 09:42:41 UTC
New Package SCM Request
=======================
Package Name: xml2dict
Short Description: Use attributes of dictionary to access xml elements.
Owners: yuwang
Branches: f15 f16 f17 el6
InitialCC: yuwang

Comment 34 Gwyn Ciesla 2012-03-19 11:52:33 UTC
Git done (by process-git-requests).

Comment 35 Yuguang Wang 2012-03-20 02:37:33 UTC
Finished build in koji:
http://koji.fedoraproject.org/koji/taskinfo?taskID=3911753
Thank you all.

Close it as NEXTRELEASE


Note You need to log in before you can comment on or make changes to this bug.