[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
https://bugzilla.redhat.com/show_bug.cgi?id=833226 --- Comment #41 from Jon Ciesla --- Git done (by process-git-requests). -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
https://bugzilla.redhat.com/show_bug.cgi?id=833226 Jon Ciesla changed: What|Removed |Added Flags|fedora-cvs? |fedora-cvs+ -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
https://bugzilla.redhat.com/show_bug.cgi?id=833226 Eric Smith changed: What|Removed |Added Flags|fedora-cvs+ |fedora-cvs? --- Comment #40 from Eric Smith --- Package Change Request == Package Name: python-pycparser New Branches: epel7 Owners: brouhaha -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
https://bugzilla.redhat.com/show_bug.cgi?id=833226 Fedora Update System changed: What|Removed |Added Fixed In Version|python-pycparser-2.09.1-6.f |python-pycparser-2.09.1-5.e |c18 |l6 --- Comment #39 from Fedora Update System --- python-pycparser-2.09.1-5.el6 has been pushed to the Fedora EPEL 6 stable repository. -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=AfUGIEgvD1&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
https://bugzilla.redhat.com/show_bug.cgi?id=833226 Fedora Update System changed: What|Removed |Added Fixed In Version|python-pycparser-2.09.1-6.f |python-pycparser-2.09.1-6.f |c19 |c18 --- Comment #38 from Fedora Update System --- python-pycparser-2.09.1-6.fc18 has been pushed to the Fedora 18 stable repository. If problems still persist, please make note of it in this bug report. -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=7nMU9ozfgP&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
https://bugzilla.redhat.com/show_bug.cgi?id=833226 Fedora Update System changed: What|Removed |Added Status|ON_QA |CLOSED Fixed In Version||python-pycparser-2.09.1-6.f ||c19 Resolution|--- |ERRATA Last Closed||2013-08-02 17:49:55 --- Comment #37 from Fedora Update System --- python-pycparser-2.09.1-6.fc19 has been pushed to the Fedora 19 stable repository. If problems still persist, please make note of it in this bug report. -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=lQDIs5litS&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
https://bugzilla.redhat.com/show_bug.cgi?id=833226 --- Comment #36 from Eric Smith --- I added the requested Python 3 support in the -6 release. -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=FVsqpnDUy9&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
https://bugzilla.redhat.com/show_bug.cgi?id=833226 --- Comment #35 from Fedora Update System --- python-pycparser-2.09.1-6.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/python-pycparser-2.09.1-6.fc18 -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=i9N0CKehdr&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
https://bugzilla.redhat.com/show_bug.cgi?id=833226 --- Comment #34 from Fedora Update System --- python-pycparser-2.09.1-6.fc19 has been submitted as an update for Fedora 19. https://admin.fedoraproject.org/updates/python-pycparser-2.09.1-6.fc19 -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=Dij5UDoxRQ&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
https://bugzilla.redhat.com/show_bug.cgi?id=833226 Fedora Update System changed: What|Removed |Added Status|MODIFIED|ON_QA --- Comment #33 from Fedora Update System --- python-pycparser-2.09.1-5.el6 has been pushed to the Fedora EPEL 6 testing repository. -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=LCvbqFuhzR&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
https://bugzilla.redhat.com/show_bug.cgi?id=833226 --- Comment #32 from Fedora Update System --- python-pycparser-2.09.1-5.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/python-pycparser-2.09.1-5.el6 -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=lb5g9HBBlL&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
https://bugzilla.redhat.com/show_bug.cgi?id=833226 --- Comment #31 from Fedora Update System --- python-pycparser-2.09.1-5.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/python-pycparser-2.09.1-5.fc18 -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=OpR19D5k08&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
https://bugzilla.redhat.com/show_bug.cgi?id=833226 --- Comment #30 from Fedora Update System --- python-pycparser-2.09.1-5.fc19 has been submitted as an update for Fedora 19. https://admin.fedoraproject.org/updates/python-pycparser-2.09.1-5.fc19 -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=qrJFsVwoum&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
https://bugzilla.redhat.com/show_bug.cgi?id=833226 Fedora Update System changed: What|Removed |Added Status|ASSIGNED|MODIFIED -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=vqqSkqKonK&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
https://bugzilla.redhat.com/show_bug.cgi?id=833226 Jon Ciesla changed: What|Removed |Added Flags|fedora-cvs? |fedora-cvs+ -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=d8kX2Sa3bp&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
https://bugzilla.redhat.com/show_bug.cgi?id=833226 --- Comment #29 from Jon Ciesla --- Git done (by process-git-requests). -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=Qo70pgvpV5&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
https://bugzilla.redhat.com/show_bug.cgi?id=833226 --- Comment #28 from Jos de Kloe --- Eric, thanks for your effort. I am happy with those 2 upstream reports and agree that removing ply is a fedora specific packaging thing, and upstream may have good reason to decide otherwise. -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=PmhDU5QBqn&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
https://bugzilla.redhat.com/show_bug.cgi?id=833226 --- Comment #27 from Eric Smith --- http://fedorapeople.org/~brouhaha/python-pycparser/python-pycparser.spec http://fedorapeople.org/~brouhaha/python-pycparser/python-pycparser-2.09.1-5.fc19.src.rpm As I was taking care of the "should" item regarding the patches, I discovered that the package wouldn't build if an earlier version was installed, due to the current and parent dirs being added to the end of the path rather than the beginning in _build_tables.py. The fix is to do the same thing as the unit test path patch, so I added another patch for that, added comments on the patches, and submitted two of the patches as upstream issues. -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=pCRB9mteRu&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
https://bugzilla.redhat.com/show_bug.cgi?id=833226 Eric Smith changed: What|Removed |Added Flags||fedora-cvs+ --- Comment #26 from Eric Smith --- New Package SCM Request === Package Name: python-pycparser Short Description: C parser and AST generator written in Python Owners: brouhaha Branches: f18 f19 el6 InitialCC: -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=Za5K3agZOW&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
https://bugzilla.redhat.com/show_bug.cgi?id=833226 Eric Smith changed: What|Removed |Added Flags|fedora-cvs+ |fedora-cvs? -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=C8q0k8mIOD&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
https://bugzilla.redhat.com/show_bug.cgi?id=833226
Jos de Kloe changed:
What|Removed |Added
Flags|fedora-review? |fedora-review+
--- Comment #25 from Jos de Kloe ---
Thanks Eric
here is my review:
Package Review
==
Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed
Issues:
===
- Header files in -devel subpackage, if present.
Note: python-pycparser : /usr/share/doc/python-
pycparser-2.09.1/examples/c_files/memmgr.h
See: http://fedoraproject.org/wiki/Packaging/Guidelines#DevelPackages
==>this clearly is an input file to test the c parser, so this issue
may be safely ignored.
= MUST items =
Generic:
[x]: Package is licensed with an open-source compatible license and meets
other legal requirements as defined in the legal section of Packaging
Guidelines.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[-]: Development files must be in a -devel package
[-]: Package requires other packages for directories it uses.
[x]: Package uses nothing in %doc for runtime.
[x]: Package is not known to require ExcludeArch.
[x]: Package complies to the Packaging Guidelines
[x]: License field in the package spec file matches the actual license.
Note: Checking patched sources after %prep for licenses. Licenses found:
"Unknown or generated", "*No copyright* Public domain". 98 files have
unknown license. Detailed output of licensecheck in
/home/user_to_make_rpms/reviews/833226.python-pycparser/tmp/review-
python-pycparser/licensecheck.txt
==>manually checked to be BSD
[x]: Package consistently uses macro is (instead of hard-coded directory
names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
Provides are present.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[-]: Large documentation must go in a -doc subpackage.
Note: Documentation size is 40960 bytes in 12 files.
[x]: All build dependencies are listed in BuildRequires, except for any that
are listed in the exceptions section of Packaging Guidelines.
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
beginning of %install.
[x]: Each %files section contains %defattr if rpm < 4.4
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Fully versioned dependency in subpackages, if present.
[x]: If (and only if) the source package includes the text of the license(s)
in its own file, then that file, containing the text of the license(s)
for the package is included in %doc.
[-]: Package use %makeinstall only when make install' ' DESTDIR=... doesn't
work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package do not use a name that already exist
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as provided
in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
%{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local
[x]: Package successfully compiles and builds into binary rpms on at least one
supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
Note: No rpmlint messages.
Python:
[x]: Python eggs must not download any dependencies during the build process.
[-]: A package which is used by another package via an egg interface should
provide egg info.
[x]: Package meets the Packaging Guidelines::Python
[-]: Package contains BR: python2-devel or python3-devel
[-]: Binary eggs must be removed in %prep
= SHOULD items =
Generic:
[-]: If the source package does not include license text(s) as a separate file
from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[!]: Patches link to upstream bugs/comments/lists or are otherwise justified.
==>patches should be submitted to upstream, and a comment
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
https://bugzilla.redhat.com/show_bug.cgi?id=833226 --- Comment #24 from Eric Smith --- Hi Jos! Sorry about that, it looks like I did something wrong when I scp'd it before. Perhaps put it in the wrong directory. Anyhow, I scp'd it again and verified that it's there now. Christopher, I agree with you, and had already updated the Source0 tag in my -4 spec to point to a tarball from github. I did not change it to pypi for the same reasons I've given in the package review for python-tinycss (bug #986630), though if Jos feels strongly that he would prefer the use of a pypi URL for Source0, I'll change it. I'd like to add python3 support, but pycparser requires ply, and the Fedora python-ply package doesn't include python3 support. Once the python-ply package maintainer adds that, I'll add it to python-pycparser as well. Thanks! -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=8jdSzQ0K5k&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
https://bugzilla.redhat.com/show_bug.cgi?id=833226 --- Comment #23 from Jos de Kloe --- Hi Eric, yes I still am interested in reviewing this request. Just tried again, but the rpm link http://fedorapeople.org/~brouhaha/python-pycparser/python-pycparser-2.09.1-4.fc19.src.rpm seems currently not available. Could you provide it again? -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=2Rh5fODMgv&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
https://bugzilla.redhat.com/show_bug.cgi?id=833226 --- Comment #22 from Christopher Meng --- Eric, besides the Source0 ISSUE, please also add python3 support. We can know that this module supports python3. -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=VXiLg7NupE&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
https://bugzilla.redhat.com/show_bug.cgi?id=833226 --- Comment #21 from Christopher Meng --- Checking out some git/hg repo's file is a pain. So you should use pypi as Source0. https://pypi.python.org/packages/source/p/pycparser/pycparser-2.09.1.tar.gz Easy, Simple. -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=GclQ6df5nv&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
https://bugzilla.redhat.com/show_bug.cgi?id=833226 Eric Smith changed: What|Removed |Added Blocks||986712 -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=BT5XO68tTv&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
https://bugzilla.redhat.com/show_bug.cgi?id=833226 --- Comment #20 from Eric Smith --- http://fedorapeople.org/~brouhaha/python-pycparser/python-pycparser.spec http://fedorapeople.org/~brouhaha/python-pycparser/python-pycparser-2.09.1-4.fc19.src.rpm Thanks Scott! Updated for upstream move to github, renamed patches and Source1 for easier tracking of future upstream releases, and fixed rpmlint complaint about strange permissions. Jos, if you're still willing to review this, please use the links above. -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=0nhRuNaIV1&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
https://bugzilla.redhat.com/show_bug.cgi?id=833226 --- Comment #19 from Scott Tsai --- (In reply to Eric Smith from comment #18) Eric, please feel free to take over pycparser. I originally started this with python-cffi in mind as well :) On suggestion: The upstream author, Eli Bendersky, has since switched his projects from bitbucket to github. So you would want to change the comments to refer to https://github.com/eliben/pycparser -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=6F6dfYStJg&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
https://bugzilla.redhat.com/show_bug.cgi?id=833226 --- Comment #18 from Eric Smith --- In the build process, _build_tables.py should be invoked to generate the lextab.py and yacctab.py tables, so that they can be included in the RPM. Otherwise they have to be regenerated at runtime, which is a fairly large performance penalty, and also causes pytest regression tests of python-cffi to fail. I've added that minor change to the spec: http://fedorapeople.org/~brouhaha/python-pycparser/python-pycparser.spec http://fedorapeople.org/~brouhaha/python-pycparser/python-pycparser-2.09.1-3.fc19.src.rpm Upstream provides a release tarball for 2.09.1, so I think the Source0 URL could be changed to refer directly to that, rather than a snapshot. -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=YlaoN1mFF8&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
https://bugzilla.redhat.com/show_bug.cgi?id=833226 Eric Smith changed: What|Removed |Added CC||space...@gmail.com --- Comment #17 from Eric Smith --- Scott, I need python-pycparser in order to package python-cffi. If you're no longer interested in packaging python-pycparser, I'm willing to take it. In any case, thanks for the work you've done on it so far, and thanks to Jos for working on the review. The patched spec Jos provided in comment 16 is working fine for me on f19. -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=NXg4cRsA0X&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=833226 --- Comment #16 from Jos de Kloe --- Hi Scott, you are right, the setup script does not allow switching off the embedded copy of ply. However, a simple patch should be enough to fix this issue. See these example files that I prepared to test the idea: http://www.jdekloe.nl/Fedora/python-pycparser.spec http://www.jdekloe.nl/Fedora/python-pycparser-2.09.1-2.fc18.src.rpm feel free to take them and modify them for your next version. Personally I have no direct use for pycparser, so I don't plan to takeover the packaging. Thanks for the offer though. -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=xOxJ6zF5IX&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=833226 --- Comment #15 from Scott Tsai --- (In reply to comment #14) HI Jos, I looked at the embedded PLY in the upstream repo history up to the point of determining that it wasn't patched. I tried to come up with a way to not keep a local path in Fedora for using system PLY a.l.a. how projects like Firefox has "--with-system-libXXX" options in their configure scripts but failed. Please feel free to take over packing python-pycparser. -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=yvkecoSZdd&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=833226 --- Comment #14 from Jos de Kloe --- Hi Scott, did you already find time to look into the ply issue I mentioned above? Don't hesitate to ask for help, should that be needed. Regards, Jos -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=sXzR8fCENN&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=833226
--- Comment #13 from Jos de Kloe ---
Thanks for your new version.
Some remarks on your spec file:
There is a tiny difference between the spec file you published and the spec
file in your srpm:
1c1
< # NOTE: "hgrev" and Version should match, e.g.
---
> # NOTE: "hgrev" and "version" should match, e.g.
all patches should have a comment concerning the upstream status,
but this is missing in your spec file.
See:
https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment
Also I noticed that poth upstream pycparser and Ply state that they are
compatible to python3. It would be nice if you could consider adding
support to python3 in a next version as well.
Testing was succesfull using mock, which created the srpm and the noarch rpm.
rpmlint gives the following output:
$ rpmlint python-pycparser-2.09.1-1.fc19.noarch.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
$ rpmlint python-pycparser-2.09.1-1.fc19.src.rpm
python-pycparser.src: W: strange-permission remove-relative-sys-path 0755L
python-pycparser.src:15: W: macro-in-comment %{hgrev}
python-pycparser.src: W: invalid-url Source0:
eliben-pycparser-82ace14bb612.tar.bz2
1 packages and 0 specfiles checked; 0 errors, 3 warnings.
I think the permission warning can indeed be ignored
The macro in comment could be fixed by writing %%{hgrev}
The invalid-url can maybe be fixed by referring to this url:
https://bitbucket.org/eliben/pycparser/get/release_v2.09.1.tar.bz2
This command did retrieve the package correctly on my side:
wget https://bitbucket.org/eliben/pycparser/get/release_v2.09.1.tar.bz2 \
-O pycparser-2.09.1.tar.bz2
Also koji tested the package succesfully, see:
http://koji.fedoraproject.org/koji/taskinfo?taskID=4894858
---
MUST items as mentioned in:
https://fedoraproject.org/wiki/Packaging/ReviewGuidelines
key:
[+] OK
[.] OK, not applicable
[X] needs work
[+] MUST: rpmlint must be run on the source rpm and all binary rpms the build
produces. The output should be posted in the review.[1]
[+] MUST: The package must be named according to the Package Naming Guidelines
.
[+] MUST: The spec file name must match the base package %{name}, in the format
%{name}.spec unless your package has an exemption. [2] .
PROBLEM SEE BELOW: [X] MUST: The package must meet the Packaging Guidelines.
ADDED: [+] in addition check the python specific Guidelines
[+] MUST: The package must be licensed with a Fedora approved license and meet
the Licensing Guidelines .
[+] MUST: The License field in the package spec file must match the actual
license. [3]
[+] MUST: If (and only if) the source package includes the text of the
license(s) in its own file, then that file, containing the text of the
license(s) for the package must be included in %doc.[4]
[+] MUST: The spec file must be written in American English. [5]
[+] MUST: The spec file for the package MUST be legible. [6]
[X] MUST: The sources used to build the package must match the upstream source,
as provided in the spec URL. Reviewers should use sha256sum for this task as it
is used by the sources file once imported into git. If no upstream URL can be
specified for this package, please see the Source URL Guidelines for how to
deal with this.
[+] MUST: The package MUST successfully compile and build into binary rpms on
at least one primary architecture. [7]
[.] MUST: If the package does not successfully compile, build or work on an
architecture, then those architectures should be listed in the spec in
ExcludeArch. Each architecture listed in ExcludeArch MUST have a bug filed in
bugzilla, describing the reason that the package does not compile/build/work on
that architecture. The bug number MUST be placed in a comment, next to the
corresponding ExcludeArch line. [8]
[+] MUST: All build dependencies must be listed in BuildRequires, except for
any that are listed in the exceptions section of the Packaging Guidelines ;
inclusion of those as BuildRequires is optional. Apply common sense.
[.] MUST: The spec file MUST handle locales properly. This is done by using the
%find_lang macro. Using %{_datadir}/locale/* is strictly forbidden.[9]
[.] MUST: Every binary RPM package (or subpackage) which stores shared library
files (not just symlinks) in any of the dynamic linker's default paths, must
call ldconfig in %post and %postun. [10]
[+] MUST: Packages must NOT bundle copies of system libraries.[11]
[.] MUST: If the package is designed to be relocatable, the packager must state
this fact in the request for review, along with the rationalization for
relocation of that specific package. Without this, use of Prefix: /usr is
considered a blocker. [12]
[+] MUST: A package must own all directories that it creates. If it does not
create a directory that it uses, then it should require a package which does
create that directory. [13]
[+] MUST: A Fedora package must not list
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=833226 --- Comment #12 from Scott Tsai --- (In reply to comment #11) The SRPM URL should instead be http://scottt.tw/fedora/python-pycparser/python-pycparser-2.09.1-1.fc18.src.rpm -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=l6YfmonJhO&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=833226
--- Comment #11 from Scott Tsai ---
(In reply to comment #10)
I've update
http://scottt.tw/fedora/python-pycparser/python-pycparser.spec
http://scottt.tw/fedora/python-pycparser/python-pycparser-2.08-1.fc18.src.rpm
with these changes:
1. Change the source tarball to upstream tag "release_v2.09.1"
Upstream is currently using a "tag releases in mercurial" but offer no files in
the download section" strategy:
+# NOTE: "hgrev" and "version" should match, e.g.
+# revision 82ace14bb612 is tagged as "release_v2.09.1" in
+# https://bitbucket.org/eliben/pycparser
+
+%global hgrev 82ace14bb612
+
Name: python-pycparser
-Version:2.08
+Version:2.09.1
Release:1%{?dist}
Summary:C parser and AST generator written in Python
License:BSD
-URL:http://code.google.com/p/pycparser/
-Source0:
http://pycparser.googlecode.com/files/pycparser-%{version}.tar.gz
+URL:https://bitbucket.org/eliben/pycparser
+# tarball generated by bitbucket from mercurial tag:
+# https://bitbucket.org/eliben/pycparser/commits/%{hgrev}
+Source0:eliben-pycparser-%{hgrev}.tar.bz2
2. Add BuildRequires on python2-devel:
+BuildRequires: python2-devel
Regarding the permission of "remove-relative-sys-path", I still think the
rpmlint warning should just be ignored.
--
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug
https://bugzilla.redhat.com/token.cgi?t=4bjvaTdlLx&a=cc_unsubscribe
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=833226 --- Comment #10 from Jos de Kloe --- Hi Scott, thanks for the updated version. I looked again at the package and (since I'm still learning about the review process) found a few other issues. the package still builds fine with mock: mock -r fedora-rawhide-x86_64 --rebuild python-pycparser-2.08-1.fc17.src.rpm generates these rpms: python-pycparser-2.08-1.fc19.noarch.rpm python-pycparser-2.08-1.fc19.src.rpm However rpmlint has some new warnings now: $ rpmlint python-pycparser-2.08-1.fc19.noarch.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings. $ rpmlint python-pycparser-2.08-1.fc19.src.rpm python-pycparser.src: W: strange-permission remove-relative-sys-path 0755L python-pycparser.src: W: invalid-url Source0: http://pycparser.googlecode.com/files/pycparser-2.08.tar.gz HTTP Error 404: Not Found 1 packages and 0 specfiles checked; 0 errors, 2 warnings. This introduces a problem with this requirement from the review guidelines: MUST: The sources used to build the package must match the upstream source, this is a problem now since upstream moved to a different location. The new source URL of the project is: https://bitbucket.org/eliben/pycparser However, I don't see version 2.08 listed here anymore. Latest versions are 2.09 and 2.09.1, but the one before is 2.07 Could you please either update to the latest released version, or change the source url to point to the right mercurial revision that matches your v2.08? (if you choose this you probably should contact upstream to ask which version this was, since it is not noted in mercurial comments or tags) The other rpmlint warning refers to this requirement: MUST: Permissions on files must be set properly. rpmlint complains about the permissions for the remove-relative-sys-path script. There is no need to have a write permission for this script, but then it doesn't hurt either I think since the script is not installed, but only used during the build of the rpm. Then there are additional requirements from: https://fedoraproject.org/wiki/Packaging:Python To build a package containing python2 files, you need to have BuildRequires: python2-devel this is currently missing in your spec file. Best regards, Jos -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=2AVCNw64Fm&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=833226
--- Comment #9 from Scott Tsai ---
(In reply to comment #8)
Hi Jos, sorry for the late reply.
I've added the LICENSE file to docs in:
http://scottt.tw/fedora/python-pycparser/python-pycparser.spec
http://scottt.tw/fedora/python-pycparser/python-pycparser-2.08-1.fc18.src.rpm
diff --git a/python-pycparser.spec b/python-pycparser.spec
index 63a257d..d749e6d 100644
--- a/python-pycparser.spec
+++ b/python-pycparser.spec
@@ -12,6 +12,7 @@ BuildArch: noarch
# for unit tests
BuildRequires: python-ply
+BuildRequires: dos2unix
Requires: python-ply
@@ -27,6 +28,7 @@ need to parse C source code.
# examples
cp %SOURCE1 .
./remove-relative-sys-path examples
+dos2unix LICENSE
%build
%{__python} setup.py build
@@ -38,11 +40,10 @@ cp %SOURCE1 .
%{__python} tests/all_tests.py
%files
-%doc examples
+%doc examples LICENSE
%{python_sitelib}/pycparser/
%{python_sitelib}/pycparser-*.egg-info
-
%changelog
-* Tue Jun 18 2012 Scott Tsai 2.08-1
+* Tue Jun 18 2012 Scott Tsai 2.08-1
- upstream 2.08
--
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug
https://bugzilla.redhat.com/token.cgi?t=vwaN60ibfy&a=cc_unsubscribe
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=833226 --- Comment #8 from Jos de Kloe --- Hi Scott Tsai, could you please fix the last tiny issue in the spec file (adding the LICENSE file to the %doc section, see Comment 3). The other issues I mentioned are optional, and it's up to you to decide what to do with them. After that, I have no more comments and will approve this package. Thanks. -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=4ZDrZOkzZc&a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
https://bugzilla.redhat.com/show_bug.cgi?id=833226 Jos de Kloe changed: What|Removed |Added Status|NEW |ASSIGNED Assignee|nob...@fedoraproject.org|josdek...@gmail.com Flags||fedora-review? --- Comment #7 from Jos de Kloe --- Mario, thanks for your advice. I have assigned it, and will try to complete the review after the last small issue in the spec file has been resolved. -- You are receiving this mail because: You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
https://bugzilla.redhat.com/show_bug.cgi?id=833226 --- Comment #6 from Mario Blättermann --- (In reply to comment #5) > Thanks for reminding me Mario. > I tested again using mock, using release fedora-rawhide-x86_64. This runs > fine and creates both a srpm and a rpm. > > In addition I tested using koji. Also here both rpms where created > succesfully. See: http://koji.fedoraproject.org/koji/taskinfo?taskID=4608971 Then you could go ahead with assigning this bug to you and completing the review. -- You are receiving this mail because: You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
https://bugzilla.redhat.com/show_bug.cgi?id=833226 --- Comment #5 from Jos de Kloe --- Thanks for reminding me Mario. I tested again using mock, using release fedora-rawhide-x86_64. This runs fine and creates both a srpm and a rpm. In addition I tested using koji. Also here both rpms where created succesfully. See: http://koji.fedoraproject.org/koji/taskinfo?taskID=4608971 -- You are receiving this mail because: You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
https://bugzilla.redhat.com/show_bug.cgi?id=833226 Mario Blättermann changed: What|Removed |Added CC||mario.blaetterm...@gmail.co ||m --- Comment #4 from Mario Blättermann --- (In reply to comment #3) > rpmbuild runs fine with the new spec file, and rpmlint has the same result > as above. @Jos, for checking packages please use either a local Mock installation or a Koji scratch build. For the latter, the following command works, assuming you are in the folder where the *src.rpm resides: koji build --scratch rawhide *src.rpm To get a list of possible build targets, use: koji list-targets If you build the packages on your workstation only, you possibly miss build dependencies, if some needed packages are present on your system, but not included in the spec file. -- You are receiving this mail because: You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
https://bugzilla.redhat.com/show_bug.cgi?id=833226 --- Comment #3 from Jos de Kloe --- thank you for this updated version. rpmbuild runs fine with the new spec file, and rpmlint has the same result as above. A small comment on the fact that you rename the upstream name pycparser to python-pycparser. According to the guidelines this is not needed for python packages that start their name with 'py'. See: https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Addon_Packages_.28python_modules.29 This therefore is upto you, both ways are allowed, but removing the rename makes the spec file simpler, which always is a good thing. Also please I would prefer if you would not delete changelog entries, even if the spec file is not yet approved. Finally the LICENSE file should be added to the %doc tag in the %files section. -- You are receiving this mail because: You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
https://bugzilla.redhat.com/show_bug.cgi?id=833226
--- Comment #2 from Scott Tsai ---
(In reply to comment #1)
Spec URL: http://scottt.tw/fedora/python-pycparser/python-pycparser.spec
SRPM URL:
http://scottt.tw/fedora/python-pycparser/python-pycparser-2.08-1.fc17.src.rpm
Changes:
1. Use %{buildroot} instead of $RPM_BUILD_ROOT
-%{__python} setup.py install --skip-build --root $RPM_BUILD_ROOT
+%{__python} setup.py install --skip-build --root %{buildroot}
2. List files with more specificity:
%files
%doc examples
-# For noarch packages: sitelib
-%{python_sitelib}/*
+%{python_sitelib}/pycparser/
+%{python_sitelib}/pycparser-*.egg-info
3. Remove dos2unix since upstream accepted my line ending patches
-BuildRequires: dos2unix
Requires: python-ply
@@ -26,7 +25,6 @@ need to parse C source code.
%setup -q -n pycparser-%{version}
# examples
-find examples -type f -exec dos2unix '{}' ';'
--
You are receiving this mail because:
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 833226] Review Request: python-pycparser - C parser and AST generator written in Python
https://bugzilla.redhat.com/show_bug.cgi?id=833226
Jos de Kloe changed:
What|Removed |Added
CC||josdek...@gmail.com
--- Comment #1 from Jos de Kloe ---
rpmbuild -ba runs fine
rpmlint python-pycparser-2.07-1.fc17.src.rpm
python-pycparser.src: W: strange-permission remove-relative-sys-path 0755L
1 packages and 0 specfiles checked; 0 errors, 1 warnings.
reproduces your result.
rpmlint python-pycparser-2.07-1.fc17.noarch.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
seems fine.
Two different styles of macros are mixed in this spec file, i.e.: %{__python}
versus $RPM_BUILD_ROOT
please choose a consistent style, i.e. use: %{buildroot}
(see http://fedoraproject.org/wiki/Packaging:Guidelines,
1.35.1 Using %{buildroot} and %{optflags} vs $RPM_BUILD_ROOT and
$RPM_OPT_FLAGS)
I noticed the spec file runs dos2unix to convert line-endings in examples
files, and it also runs a custom python script to delete unwanted boilerplate
sys.path lines in the examples dir.
Looking at
https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#Scripting_inside_of_spec_files
this seems fine to me.
Since all python files in this package are in the module directory pycparser I
think it would be better to be more explicit and to have
%{python_sitelib}/pycparser/ in stead of %{python_sitelib}/* in the %files
section.
--
You are receiving this mail because:
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
