Spec URL: <spec info here> SRPM URL: http://salimma.fedorapeople.org/specs/python/python-django-1.3.1-3.fc15.src.rpm Description: Django is a high-level Python Web framework that encourages rapid development and a clean, pragmatic design. It focuses on automating as much as possible and adhering to the DRY (Don't Repeat Yourself) principle. This is a rename of the existing Django package, to comply with naming guidelines better and in anticipation of multiple Python variants being supported.
Are you coordinating with the packaging committee on this? We would have to rename a ton of django modules from django-foo to python-django-foo as well if you really want to do this.
Just a few quick notes: - You have the Provides: Django = %{name}-%{version}-%{release} but according to [1], it should only be Django = %{version}-%{release}. - According to the same guideline, Obsoletes should be Django < 1.3.1-3 (so do not use the macros, but hardcode the concrete versions that you Obsolete - if you would increase the version, the Obsoletes would change with macro, which is not what you want, you want to Obsolete exactly the versions lower than 1.3.1-3). - AFAIK Django ships with lots of unit tests. You should probably run them in %check section to uncover possible problems and report them upstream. - The renaming of Django modules is currently being discussed on fedora python-devel mailing list [2], you might want to take a look there. - I'd be happy to take the review, but I would recommend against getting the renamed package into F17, because the devel freeze is comming and it would be hard to rename all the packages. Let's leave it to the next Rawhide/F18, then I'll take it. What do you think? [1] http://fedoraproject.org/wiki/Packaging:Guidelines#Renaming.2FReplacing_Existing_Packages [2] http://lists.fedoraproject.org/pipermail/python-devel/2012-January/000335.html
(In reply to comment #2) > - The renaming of Django modules is currently being discussed on fedora > python-devel mailing list [2], you might want to take a look there. Will do, thanks > - I'd be happy to take the review, but I would recommend against getting the > renamed package into F17, because the devel freeze is comming and it would be > hard to rename all the packages. Let's leave it to the next Rawhide/F18, then > I'll take it. What do you think? > Targeting F-18 is definitely the safe option at this point in time. Sure, please take the review. I'll make the changes you point out.
I took the freedom to create some new SRPM/SPEC to make some progress. SPEC: http://www.matthias-runge.de/fedora/python-django.spec SRPM: http://www.matthias-runge.de/fedora/python-django-1.3.1-5.fc16.src.rpm Dear django-maintainers: If you're short in time, I'll be glad to help out. Question is: how? Matthias
(In reply to comment #4) > I took the freedom to create some new SRPM/SPEC to make some progress. > > SPEC: http://www.matthias-runge.de/fedora/python-django.spec > SRPM: http://www.matthias-runge.de/fedora/python-django-1.3.1-5.fc16.src.rpm > > Dear django-maintainers: If you're short in time, I'll be glad to help out. > Question is: how? > > Matthias - As proposed in the fpc ticket [1], we should also have Provides: django = %{version}-%{release} (with the lowercase d) for better usability via yum. - I would still love to see some unit tests running in the check section. Is there a way to do that? - Otherwise the package looks good. I agree with Matthias, that there would be lots of people willing to help with maintenance of Django. Michel, would you possibly let Matthias finish the review and then make the SCM request yourself, accepting Matthias as comaintainer (personally, I would also love to comaintain :))? This is really blocking the whole "Django rename" effort [2], so it would be nice if we could get it moving. Thank you very much! [1] https://fedorahosted.org/fpc/ticket/146 [2] https://fedoraproject.org/wiki/User:Bkabrda/Django_rename
OK, I/one should add Provides: django = %{version}-%{release} and also additional provides/obsoletes to -doc subpackage.
(In reply to comment #5) > - I would still love to see some unit tests running in the check section. Is > there a way to do that? Me too. Sadly, a about 15 of those unit test currently fail. Updated to provide django lowercase (for django-doc, too) SPEC: http://www.matthias-runge.de/fedora/python-django.spec SRPM: http://www.matthias-runge.de/fedora/python-django-1.3.1-6.fc16.src.rpm %check-update: tests in Django-1.4c1 don't succeed, either. E.g. the bundled simplejson has version 2.1.3; the test checks this. Our simplejson (in f16) is versioned 2.1.6 Tests are not required, so I'd tend to skip those as long as they can't enabled easily.
(In reply to comment #7) > (In reply to comment #5) > > - I would still love to see some unit tests running in the check section. Is > > there a way to do that? > Me too. Sadly, a about 15 of those unit test currently fail. > > Updated to provide django lowercase (for django-doc, too) > SPEC: http://www.matthias-runge.de/fedora/python-django.spec > SRPM: http://www.matthias-runge.de/fedora/python-django-1.3.1-6.fc16.src.rpm > > %check-update: > tests in Django-1.4c1 don't succeed, either. E.g. the bundled simplejson has > version 2.1.3; the test checks this. Our simplejson (in f16) is versioned 2.1.6 > > Tests are not required, so I'd tend to skip those as long as they can't enabled > easily. I believe that tests should be run. It is one of the few ways that packagers have to find out if the software works correctly. The failures should be reported upstream and fixed in cooperation with them. Only this way can we make sure that the software will work on Fedora and that we will have good relations with upstreams. (And I believe that cooperation with upstream, issue reporting and fixing and suggesting enhancements is the packager's main job.)
In this particular case, the test should simply be modified, surely? Maybe make sure simplejson is at least at the version specified upstream. Anyone know if they break API compatibility at specified versions, in which case we can set an upper bound as well?
We need a backport from trunk to fix building docs with Sphinx >= 1.1, so I'll push out an updated review spec with that and some tests as soon as I test it. Will probably apply the patch to Django as well, but not bump and rebuild, in case someone needs to rebuild for some reason or another.
Updated spec and SRPM: http://salimma.fedorapeople.org/specs/python/python-django-1.3.1-7.fc17.src.rpm http://salimma.fedorapeople.org/specs/python/python-django.spec I've enabled tests and changed the version that's obsoleted (to the latest master release + 1, per the guidelines). Needed to apply three patches from trunk (two for Sphinx changes, the last one to fix a bug in a test that causes it to work if the year is 2011 but not 2012) Matthias and others: the build currently succeeds outside of mock, but in Mock I get 13 errors. Will post the log, as I might not have time to fix it today.
Created attachment 569069 [details] Build log of the latest SRPM available for review (-7) In mock: FAILED (failures=13, errors=54, skipped=53, expected failures=3) Outside (with rpmbuild): OK (skipped=53, expected failures=3) I suspect there are additional dependencies that we're not declaring yet; slowly going through the list of imports, but to save time, here are the Python RPMs I have on my local machine that's not pulled in by mock: python3-libs-3.2.2-12.fc17.x86_64 python-async-0.6.1-4.fc17.x86_64 python-beaker-1.5.4-3.fc17.noarch python-BeautifulSoup-3.2.0-4.fc17.noarch python-boto-2.0-2.fc17.noarch python-brlapi-0.5.6-4.fc17.x86_64 python-bugzilla-0.6.2-2.fc16.noarch python-bunch-1.0.1-1.fc17.noarch python-caribou-0.4.1-5.fc17.noarch python-chardet-2.0.1-4.fc17.noarch python-cloudfiles-1.7.9.3-1.fc17.noarch python-crypto-2.5-2.fc17.x86_64 python-cups-1.9.61-1.fc17.x86_64 python-decorator-3.3.2-1.fc17.noarch python-decoratortools-1.8-3.fc17.noarch python-deltarpm-3.6-0.7.20110223git.fc17.x86_64 python-enchant-1.6.5-4.fc17.noarch python-ethtool-0.7-3.fc17.x86_64 python-fedora-0.3.26-1.fc17.noarch python-feedparser-5.1-1.fc17.noarch python-fpconst-0.7.3-9.fc17.noarch python-genshi-0.6-4.fc17.x86_64 python-gitdb-0.5.4-3.fc17.x86_64 python-GnuPGInterface-0.3.2-9.fc17.noarch python-httplib2-0.6.0-6.fc17.noarch python-iniparse-0.4-4.fc17.noarch python-IPy-0.75-2.fc17.noarch python-iwlib-1.1-2.fc17.x86_64 python-kitchen-1.1.1-1.fc17.noarch python-krbV-1.0.90-4.fc17.x86_64 python-lcms-1.19-5.fc17.x86_64 python-magic-5.10-5.fc17.x86_64 python-mako-0.5.0-2.fc17.noarch python-meh-0.12-1.fc17.noarch python-mutagen-1.20-3.fc17.noarch python-nose-1.1.2-2.fc17.noarch python-nss-0.12-3.fc17.x86_64 python-offtrac-0.0.3-4.fc17.noarch python-paramiko-1.7.7.1-2.fc17.noarch python-paste-1.7.5.1-6.20111221hg1498.fc17.noarch python-pwquality-1.0.0-2.fc17.x86_64 python-pyblock-0.53-2.fc17.x86_64 python-pycurl-7.19.0-10.fc17.x86_64 python-reportlab-2.5-4.fc17.x86_64 python-slip-0.2.20-2.fc17.noarch python-slip-dbus-0.2.20-2.fc17.noarch python-slip-gtk-0.2.20-2.fc17.noarch python-smbc-1.0.13-1.fc17.x86_64 python-smmap-0.8.1-4.fc17.noarch python-telepathy-0.15.19-4.fc17.noarch python-tempita-0.4-8.fc17.noarch python-testtools-0.9.13-1.fc17.noarch python-twisted-core-11.1.0-2.fc17.x86_64 python-twisted-web-11.1.0-2.fc17.x86_64 python-urlgrabber-3.9.1-11.fc17.noarch python-virtkey-0.50-11.fc17.x86_64 python-xlib-0.15-0.6.rc1.fc17.noarch python-zope-interface-3.7.0-1.fc17.x86_64
OK: it's not a dependency problem (I ran the tests manually from inside a mock shell and they pass), and it's not a problem running the tests from inside RPM either (since the initial rpmbuild succeeded). Any idea?
Hmm, interesting. I couldn't get the tests pass (on my f16) manually and it didn't succeed during rpmbuild, either. I took the following lines to run the tests: %check cd %{_builddir}/%{pkgname}-%{version}/tests PYTHONPATH="..:$PYTHONPATH" ./runtests.py --settings=test_sqlite overwriting Python path to make sure, the currently built version is checked, and not occasionally installed scripts in python path. I'm getting the same number of errors, so at least that's consistent. :-/ I'll tend to get package reviewed without tests and let's try to get the test working for django-1.4 coming in about 4 weeks. That's not as beautiful as it could be, but no drama.
I just tried the latest srpm (-7) in koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=3878521 Traceback (most recent call last): File "/builddir/build/BUILD/Django-1.3.1/tests/regressiontests/bug8245/tests.py", line 18, in test_bug_8245 'autodiscover should have raised a "Bad admin module" error.') AssertionError: autodiscover should have raised a "Bad admin module" error. ---------------------------------------------------------------------- Ran 3607 tests in 336.946s FAILED (failures=165, errors=101, skipped=53, expected failures=3) oopsie
Weird, those are different from the build errors I get from a local mock setup. My suggestion is adding an optional --with build option for running the test suite, until we resolve these test problems; until then, maintainers can make sure that a local rpmbuild rebuild (which currently works for 1.3.1-7) at least passes. When the tests are fixed we can flip the switch to make building with tests the default (and maybe make it non-default for unbranched Rawhide so we can quickly test new versions). Bohuslav, any other change we need to make for the review, apart from the test suite as already discussed? Thanks.
(In reply to comment #16) > Weird, those are different from the build errors I get from a local mock setup. > > My suggestion is adding an optional --with build option for running the test > suite, until we resolve these test problems; until then, maintainers can make > sure that a local rpmbuild rebuild (which currently works for 1.3.1-7) at least > passes. When the tests are fixed we can flip the switch to make building with > tests the default (and maybe make it non-default for unbranched Rawhide so we > can quickly test new versions). > > Bohuslav, any other change we need to make for the review, apart from the test > suite as already discussed? Thanks. Hi guys, I've been ill for a few days, so if you let me catch up a bit, I'll try to have a look at this as soon as I have some time. Thanks.
Ok, looking at the spec and srpm from Comment 11: - I think the definitions of python_sitelib and pyver don't have to be there (I'm not familiar with the EPEL's, but I think that these variables are always defined in python-devel. - As for the tests, I have run into similar issues with one package and the problem was, that when the output was redirected into a file (as mock does, when run from outside), the tests failed, but when ran from inside, the tests ran ok. So I'm gonna try to dig into this and find out a solution so that we can run the tests - if I'm not able to find it in a reasonable time, then we should probably comment the tests out, until it is sorted out. Is this ok for you all?
I think I got it. The rpmbuilds (both mock and non-mock) set the environment LANG=C, which effectively forces everything to work in us-ascii. When you put a statement "export LANG=en_US.utf8" just before running tests, all tests run fine. I'm however not sure, what is the real cause of this issue. My guess is, that the environment values somehow get into the Python mechanism and cause troubles - maybe some of you can come up with an explanation. Anyway, using "export LANG=en_US.utf8" is the way to run all tests and then I see no problem with approving this package.
Great catch! export LANG=en_US.utf8 did it for me for rpmbuild; mock --rebuild is fine, too. koji still reports errors (I didn't increment the release number, took the last srpm from salimma). The error messages lead me to some networking problems. (Tests try to connect to google.com, afaik, the builders are not allowed to do that). http://koji.fedoraproject.org/koji/taskinfo?taskID=3893587 ..............................................................................................................................................................................................................................................................................................................s..........s......................................................................................................................................................EE...............................................s.........................................................................................................................................................................................................................................................................x.........................................................................................................s....................................................................s..................................................................................................................................................................................................................................................................................................................................................................................................................................................................................s..................................................................................................................................................................ss...........x....................................................................................................................................................s.......................................................................................................................................................s..................................................................................s.........................................................................................................................................................................................................................................................................................................................................................................s.......s..................................................................................s..........................................................................................................................................................................................ssssssssssssssssssssssssssssssss.......................................................................................................................x..............................................................................................................................................................................................................................................................................................................ssss....................................................................................................................................................................................................................E.EE............................................................................................................................................................................................................................................................................................................ss..s....... ====================================================================== ERROR: test_correct_url_value_passes (modeltests.validation.tests.BaseModelValidationTests) ---------------------------------------------------------------------- Traceback (most recent call last): File "/builddir/build/BUILD/Django-1.3.1/tests/modeltests/validation/tests.py", line 63, in test_correct_url_value_passes self.assertEqual(None, mtv.full_clean()) # This will fail if there's no Internet connection File "/builddir/build/BUILD/Django-1.3.1/django/db/models/base.py", line 828, in full_clean raise ValidationError(errors) ValidationError: {'url_verify': [u'This URL appears to be a broken link.']} ====================================================================== ERROR: test_correct_url_with_redirect (modeltests.validation.tests.BaseModelValidationTests) ---------------------------------------------------------------------- Traceback (most recent call last): File "/builddir/build/BUILD/Django-1.3.1/tests/modeltests/validation/tests.py", line 67, in test_correct_url_with_redirect self.assertEqual(None, mtv.full_clean()) # This will fail if there's no Internet connection File "/builddir/build/BUILD/Django-1.3.1/django/db/models/base.py", line 828, in full_clean raise ValidationError(errors) ValidationError: {'url_verify': [u'This URL appears to be a broken link.']} ====================================================================== ERROR: test_urlfield_10 (regressiontests.forms.tests.fields.FieldsTests) ---------------------------------------------------------------------- Traceback (most recent call last): File "/builddir/build/BUILD/Django-1.3.1/tests/regressiontests/forms/tests/fields.py", line 632, in test_urlfield_10 self.assertEqual(url, f.clean(url)) #This will fail without internet. File "/builddir/build/BUILD/Django-1.3.1/django/forms/fields.py", line 165, in clean self.run_validators(value) File "/builddir/build/BUILD/Django-1.3.1/django/forms/fields.py", line 154, in run_validators raise ValidationError(errors) ValidationError: [u'This URL appears to be a broken link.'] ====================================================================== ERROR: test_urlfield_3 (regressiontests.forms.tests.fields.FieldsTests) ---------------------------------------------------------------------- Traceback (most recent call last): File "/builddir/build/BUILD/Django-1.3.1/tests/regressiontests/forms/tests/fields.py", line 562, in test_urlfield_3 self.assertEqual(u'http://www.google.com/', f.clean('http://www.google.com')) # This will fail if there's no Internet connection File "/builddir/build/BUILD/Django-1.3.1/django/forms/fields.py", line 165, in clean self.run_validators(value) File "/builddir/build/BUILD/Django-1.3.1/django/forms/fields.py", line 154, in run_validators raise ValidationError(errors) ValidationError: [u'This URL appears to be a broken link.'] ====================================================================== ERROR: test_urlfield_4 (regressiontests.forms.tests.fields.FieldsTests) ---------------------------------------------------------------------- Traceback (most recent call last): File "/builddir/build/BUILD/Django-1.3.1/tests/regressiontests/forms/tests/fields.py", line 579, in test_urlfield_4 self.assertEqual(u'http://www.google.com/', f.clean('http://www.google.com')) # This will fail if there's no Internet connection File "/builddir/build/BUILD/Django-1.3.1/django/forms/fields.py", line 165, in clean self.run_validators(value) File "/builddir/build/BUILD/Django-1.3.1/django/forms/fields.py", line 154, in run_validators raise ValidationError(errors) ValidationError: [u'This URL appears to be a broken link.'] ---------------------------------------------------------------------- Creating test database for alias 'default'... Creating test database for alias 'other'... Destroying test database for alias 'default'... Destroying test database for alias 'other'... Ran 3607 tests in 466.685s FAILED (errors=5, skipped=53, expected failures=3)
Yes, koji builds are in no-network environment. So perhaps you could create a patch that would comment these out - or just run the tests in BUILD rather than BUILDROOT - in BUILD, you can modify the files all you want (so possibly delete the tests entirely) and the changes won't actually get to BUILDROOT (and therefore they won't get into the resulting srpm/rpms).
python-django-1.3.1-8: patched tests to work without internet connection koji-scratchbuild http://koji.fedoraproject.org/koji/taskinfo?taskID=3905954 -- success! SRPM: http://www.matthias-runge.de/fedora/python-django-1.3.1-8.fc17.src.rpm SPEC: http://www.matthias-runge.de/fedora/python-django.spec I'd change the file-list at least to remove the asterisk at the end of the following line (to contain the dir, not only the contents): %{python_sitelib}/django/bin/profiling/* I've found some unowned directories under %{python_sitelib}/django on my filesystem. Thoughts?
(In reply to comment #22) > python-django-1.3.1-8: > > patched tests to work without internet connection > > koji-scratchbuild > http://koji.fedoraproject.org/koji/taskinfo?taskID=3905954 > > -- success! > > SRPM: http://www.matthias-runge.de/fedora/python-django-1.3.1-8.fc17.src.rpm > SPEC: http://www.matthias-runge.de/fedora/python-django.spec > Great! Maybe next time, it would be a good idea to provide a link to each patch that was submitted upstream, so that you can track the upstream's progress on this easily. > I'd change the file-list at least to remove the asterisk at the end of the > following line (to contain the dir, not only the contents): > %{python_sitelib}/django/bin/profiling/* > > I've found some unowned directories under %{python_sitelib}/django on my > filesystem. > > Thoughts? I think that what is wrong is that by taking only a part of find-lang.sh script, the ".lang" file doesn't include directories where locales are placed (because these are precisely the leftovers).
(In reply to comment #23) > Great! Maybe next time, it would be a good idea to provide a link to each patch > that was submitted upstream, so that you can track the upstream's progress on > this easily. Hmm, this is a work-around to get the tests running in koji builds. If you're building and/or testing Django in other environments, you also want those 5 tests executed. I doubt, upstream will exclude those tests.
(In reply to comment #24) > (In reply to comment #23) > > Great! Maybe next time, it would be a good idea to provide a link to each patch > > that was submitted upstream, so that you can track the upstream's progress on > > this easily. > Hmm, this is a work-around to get the tests running in koji builds. If you're > building and/or testing Django in other environments, you also want those 5 > tests executed. I doubt, upstream will exclude those tests. Yes, I didn't mean those. I mean the ones that were submitted upstream. And I wanted to say, that for the patches that _get_ submitted upstream, you should provide the links. BTW, you can use %find_lang macro, like this: %find_lang django %find_lang djangojs # for the admin javascript locale files And everything works just fine. One more nit: In %check section, you already are in %{_builddir}, so you just need to change to %{pkgname}-%{version}, But that's really just a minor simplification :)
OK, I changed language handling according to #c25 and skipped changing dir in %check. Anything else? SPEC: http://www.matthias-runge.de/fedora/python-django.spec SRPM: http://www.matthias-runge.de/fedora/python-django-1.3.1-9.fc16.src.rpm [mrunge@mrungexp SPECS]$ diff -u python-django.spec-8 python-django.spec --- python-django.spec-8 2012-03-18 13:43:56.000000000 +0100 +++ python-django.spec 2012-03-19 13:41:36.892469820 +0100 @@ -8,7 +8,7 @@ Name: python-django Version: 1.3.1 -Release: 8%{?dist} +Release: 9%{?dist} Summary: A high-level Python Web framework Group: Development/Languages @@ -74,6 +74,8 @@ %patch0 -p1 -b .sphinx-table_row_index %patch1 -p1 -b .sphinx-param_separator %patch2 -p1 -b .test-week_view_allow_future + +# patch tests to skip tests requiring internet connection %patch3 # empty files @@ -97,15 +99,10 @@ rm -rf $RPM_BUILD_ROOT %{__python} setup.py install --skip-build --root $RPM_BUILD_ROOT -# Handling locale files -# This is adapted from the %%find_lang macro, which cannot be directly -# used since Django locale files are not located in %%{_datadir} -# -# The rest of the packaging guideline still apply -- do not list -# locale files by hand! -(cd $RPM_BUILD_ROOT && find . -name 'django*.mo') | %{__sed} -e 's|^.||' | %{__sed} -e \ - 's:\(.*/locale/\)\([^/_]\+\)\(.*\.mo$\):%lang(\2) \1\2\3:' \ - >> %{name}.lang +%find_lang django +%find_lang djangojs +# append djangojs.lang to django.lang +cat djangojs.lang >> django.lang # If it's rhel5+ or any Fedora over 12 build docs %if 0%{?rhel} > 4 || 0%{?fedora} >= 12 @@ -136,9 +133,7 @@ $RPM_BUILD_ROOT%{python_sitelib}/django/bin/profiling/gather_profile_stats.py* # test section -# uncomment, if tests succeed %check -cd %{_builddir}/%{pkgname}-%{version} export PYTHONPATH=$(pwd) export LANG=en_US.utf8 cd tests @@ -148,7 +143,7 @@ rm -rf $RPM_BUILD_ROOT -%files -f %{name}.lang +%files -f django.lang %defattr(-,root,root,-) %doc AUTHORS LICENSE README %{_bindir}/django-admin @@ -158,7 +153,7 @@ %attr(0755,root,root) %{python_sitelib}/django/bin/django-admin.py* %dir %{_sysconfdir}/bash_completion.d/ %config(noreplace) %{_sysconfdir}/bash_completion.d/django_bash_completion -%{python_sitelib}/django/bin/profiling/* +%{python_sitelib}/django/bin/profiling/ %{python_sitelib}/django/bin/__init__.py* # Include everything but the locale data ... %dir %{python_sitelib}/django/ @@ -250,6 +245,9 @@ %changelog +* Mon Mar 19 2012 Matthias Runge <mrunge> - 1.3.1-9 +- spec cleanup + * Sat Mar 17 2012 Matthias Runge <mrunge> - 1.3.1-8 - patch tests to work on koji (no internet connection)
I'm completely satisfied now :) This package is APPROVED.
Great, and thank you for your review! Assuming, in this case it is ok, if I submit the SCM requests here. I definitely don't want to offend somebody. So, if I did, please speak up! New Package SCM Request ======================= Package Name: python-django Short Description: A high-level Python Web framework Owners: mrunge salimma Branches: devel
Git done (by process-git-requests).
(In reply to comment #28) > Great, and thank you for your review! > > Assuming, in this case it is ok, if I submit the SCM requests here. I > definitely don't want to offend somebody. So, if I did, please speak up! We can all request the ACLs in pkgdb, so I think it doesn't really matter :)
imported and built for devel. Thanks!
I've retired Django's master/devel branch on both git and pkgdb, and renamed Django to python-django in comps and Upstream Release Monitoring, per http://fedoraproject.org/wiki/Package_Renaming_Process Question: anyone knows for sure which Fedora release RHEL 7 will be branched from? We would want to have python-django there, instead of Django.
.. and requested that Django be blocked from the Rawhide collection: https://fedorahosted.org/rel-eng/ticket/5139