Spec URL: http://dprince.fedorapeople.org/python-jsonpatch.spec SRPM URL: http://dprince.fedorapeople.org/python-jsonpatch-0.12-1.fc18.src.rpm Description: Applying JSON Patches in Python Fedora Account System Username: dprince NOTE: This will require python-jsonpointer here: https://bugzilla.redhat.com/show_bug.cgi?id=982351
I see v1.1 is available. 0.12 is from 12 months ago Also needs to s/python-devel/python2-devel/
1. I can found that in your 2 requests these pkgs are not the latest one. Please package the latest version. 2. Remove %defattr(-,root,root,-). 3. changelog is invalid IMO, please include your email address. 4. As Pádraig said, please modify the BuildRequires: python-devel 5. IMO, please package the latest version of python-jsonpointer, and then you don't have to specify ">= 0.6".
Dan is ok that I take this package, so here are new spec and SRPM, using github tarball: Spec URL: http://apevec.fedorapeople.org/python-jsonpatch.spec SRPM URL: http://apevec.fedorapeople.org/python-jsonpatch-1.1-1.fc20.src.rpm Description: Applying JSON Patches in Python Fedora Account System Username: apevec
Reviewing now.
Executing(%check): /bin/sh -e /var/tmp/rpm-tmp.MbdcHe + umask 022 + cd /builddir/build/BUILD + cd python-json-patch-2572aa4d2f5f19b7943a6ea81828559afafc9eed + /usr/bin/python tests.py Traceback (most recent call last): File "tests.py", line 8, in <module> import jsonpatch File "/builddir/build/BUILD/python-json-patch-2572aa4d2f5f19b7943a6ea81828559afafc9eed/jsonpatch.py", line 48, in <module> import jsonpointer ImportError: No module named jsonpointer RPM build errors: error: Bad exit status from /var/tmp/rpm-tmp.MbdcHe (%check) Bad exit status from /var/tmp/rpm-tmp.MbdcHe (%check) Child return code was: 1 EXCEPTION: Command failed. See logs for output. # ['bash', '--login', '-c', 'rpmbuild -bb --target i686 --nodeps builddir/build/SPECS/python-jsonpatch.spec'] Traceback (most recent call last): File "/usr/lib/python2.7/site-packages/mockbuild/trace_decorator.py", line 70, in trace result = func(*args, **kw) File "/usr/lib/python2.7/site-packages/mockbuild/util.py", line 361, in do raise mockbuild.exception.Error, ("Command failed. See logs for output.\n # %s" % (command,), child.returncode) Error: Command failed. See logs for output. # ['bash', '--login', '-c', 'rpmbuild -bb --target i686 --nodeps builddir/build/SPECS/python-jsonpatch.spec'] LEAVE do --> EXCEPTION RAISED Seems it has problem with tests. =========== BuildRequires: python2-devel missing BuildRequires: python-setuptools =========== Also just got news from FPC, we should use python2 entirely now. So all %{__python} should be %{__python2} and %{python_sitelib} should be %{python2_sitelib}
BTW you should put %check after %install.
> BTW you should put %check after %install. Yeah, had that discussion in bug 982351 comment 7 IMHO rpmbuild is wrong and should execute %check afer %build and before %install since tests are executed in builddir.
(In reply to Alan Pevec from comment #7) > > BTW you should put %check after %install. > > Yeah, had that discussion in bug 982351 comment 7 > IMHO rpmbuild is wrong and should execute %check afer %build and before > %install since tests are executed in builddir. %check section mostly represent "check your pkg to see if it can work well after installation" IMO. If you want to put some tests before install to check(I guess in most cases are used for environ sanity check?), then just write them down in %install. 0.02 cents~ --- Anyway, fix the package and upload a new one, I will run review again.
Re: comment 7 (Alan Pevec) I still disagree (bug 982351 comment 8), but my post to packaging list hasn't received helpful responses yet: https://lists.fedoraproject.org/pipermail/packaging/2013-September/009491.html
Re: comment 9 ok, your howto edits make sense, but I'd really like to see an example where %check is testing files in buildrootdir. Re: comment 5 > Seems it has problem with tests. Yes, BR on jsonpointer is missing, also that dependency was built only today, I'll update this review tomorrow when jsonpointer hits the Rawhide. > So all %{__python} should be %{__python2} and %{python_sitelib} should be %{python2_sitelib} I don't see python2_* on el6 so this needs to be conditional, right?
(In reply to Alan Pevec from comment #10) > I don't see python2_* on el6 so this needs to be conditional, right? Yes of course...
> but I'd really like to see an example where %check is testing files > in buildrootdir. Test-suites that expect libraries in run-time linker's search path are common. They would not link and run without setting LD_LIBRARY_PATH. And pointing to various local $(pwd)/src/libfoo/.libs builddirs is not really an option. Or here's a custom test from "compface": %check export LD_LIBRARY_PATH=$RPM_BUILD_ROOT%{_libdir}:$LD_LIBRARY_PATH ./compface %{SOURCE1} | ./uncompface -X > __test.xbm cmp %{SOURCE1} __test.xbm There are also large packages that include many files via '*' wildcards, and in %check you can add file-exists tests for specific files. For example, to verify that important plugins/odules have been built and installed actually if the configure script would disable them without error. [...] If there's a good reason not to use %check, perform tests in %build instead. Please don't compile any additional files during %install.
http://fedoraproject.org/wiki/Packaging:Python is not updated yet to include python2_* macros so I haven't included them, rest of the feedback included: Spec URL: http://apevec.fedorapeople.org/python-jsonpatch.spec SRPM URL: http://apevec.fedorapeople.org/python-jsonpatch-1.1-2.fc21.src.rpm Description: Applying JSON Patches in Python Fedora Account System Username: apevec
Scratch Rawhide build: http://koji.fedoraproject.org/koji/taskinfo?taskID=5931599
Re: comment 12 Thanks for the nice writeup Michael, would be good to put it on fedora wiki!
1. Homepage should be "https://python-json-patch.readthedocs.org/" 2. Use pypi can reduce size of the spec, you don't need to add commit rev number. https://pypi.python.org/pypi/jsonpatch However it's NOT a must, just my suggestion. 3. If you want to push it to el6, you can preserve %{__python}. ------------ PACKAGE APPROVED.
New Package SCM Request ======================= Package Name: python-jsonpatch Short Description: Applying JSON Patches in Python Owners: apevec dprince Branches: f19 f20 el6 InitialCC:
(In reply to Christopher Meng from comment #16) > 1. Homepage should be "https://python-json-patch.readthedocs.org/" URL: https://github.com/stefankoegl/python-json-patch was taken from egg metadata, so I assume that's what upstream prefers as a homepage? > 2. Use pypi can reduce size of the spec, you don't need to add commit rev > number. I'd like to but see bug 982351 comment 3, license file is missing in the pypi tarball. I've opened upstream issue about it https://github.com/stefankoegl/python-json-patch/issues/16 > 3. If you want to push it to el6, you can preserve %{__python}. yes, I want el6 > PACKAGE APPROVED. Thanks!
(In reply to Alan Pevec from comment #18) > (In reply to Christopher Meng from comment #16) > > 1. Homepage should be "https://python-json-patch.readthedocs.org/" > > URL: https://github.com/stefankoegl/python-json-patch > was taken from egg metadata, so I assume that's what upstream prefers as a > homepage? To me there is no determine line of which one is the closest... For packagers, using github url will be helpful when they want to check out the new version info. For devs, using readthedocs will help them find the doc easily. But I tink you are right, not too many people will have a look on spec and find docs from URL tag. And if upstream create a separate project homepage it would be better to use, whereas currently no such websites available. So using gihub url as primary URL is right.
Git done (by process-git-requests).
python-jsonpatch-1.2-1.fc19,python-warlock-1.0.1-1.fc19 has been submitted as an update for Fedora 19. https://admin.fedoraproject.org/updates/python-jsonpatch-1.2-1.fc19,python-warlock-1.0.1-1.fc19
python-jsonpatch-1.2-1.fc20,python-warlock-1.0.1-1.fc20 has been submitted as an update for Fedora 20. https://admin.fedoraproject.org/updates/python-jsonpatch-1.2-1.fc20,python-warlock-1.0.1-1.fc20
Package python-jsonpatch-1.2-1.fc20, python-warlock-1.0.1-1.fc20: * should fix your issue, * was pushed to the Fedora 20 testing repository, * should be available at your local mirror within two days. Update it with: # su -c 'yum update --enablerepo=updates-testing python-jsonpatch-1.2-1.fc20 python-warlock-1.0.1-1.fc20' as soon as you are able to. Please go to the following url: https://admin.fedoraproject.org/updates/FEDORA-2013-19078/python-jsonpatch-1.2-1.fc20,python-warlock-1.0.1-1.fc20 then log in and leave karma (feedback).
python-warlock-1.0.1-1.fc19, python-jsonpatch-1.2-2.fc19 has been pushed to the Fedora 19 stable repository. If problems still persist, please make note of it in this bug report.
python-warlock-1.0.1-1.fc20, python-jsonpatch-1.2-2.fc20 has been pushed to the Fedora 20 stable repository. If problems still persist, please make note of it in this bug report.
python-jsonpatch-1.2-2.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/python-jsonpatch-1.2-2.el6
python-jsonpatch-1.2-2.el6 has been pushed to the Fedora EPEL 6 stable repository. If problems still persist, please make note of it in this bug report.