Hi Mark, Thank you very much for the detailed review. On 02.08.2015 21:56, Mark H Weaver wrote: > Hi Eric, > > First, I notice that your mail client added > "guix-devel-bounces+eric=dvorsak.fr@gnu.org" to the CC list. That's > the > "envelope sender" of the emails you receive from this email list, and > it's used to detect bounces in case the mail is not successfully > delivered to you. If you send email to it more than a few times, our > mailman software might conclude that your email address is broken and > disable deliveries to you. To make matters worse, anyone who "replies > to all" to your email will by default include that address. > > Your mail client is buggy, and should be fixed somehow. When you > "Reply > to all" (or similar), it should *not* include the envelope sender. > Maybe you could report the problem to its author, or switch clients? > It's the webclient (roundcube) from my mail provider (OVH). I am a bit surprised that it is buggy and I will switch for an Emacs client really soon, I just need to choose and configure it first. > Anyway, moving on to your patch... > > eric@dvorsak.fr writes: >> I updated the package to 2.7.10. This patch should be added to Hydra >> in a branch first to see if the 500 dependencies are still building >> properly. > > Your patch looks good except for three issues: > > * it needs a proper commit log conforming to our conventions I used your commit log below, I couldn't find anything else to add and I will try to be more cautious next time. > * it needs proper indentation I hope I nailed the big comment because I wasn't sure I got your instructions right. Emacs "M-q" command was breaking it, that's why I did not use it there and failed the alignment. > * you should add a copyright line for yourself to the top of python.scm It's already there from a previous patch. Eric > > See below for more details. > >> Altough the tests are still failing, I ran them after the build failed >> with : >> >> guix build -K >> cd tmp/nix-build-* >> env -i $(which bash) >> source environment-variables >> >> And they all passed except for some skips and a module that failed >> trying to write to a dir without permissions. > > Okay, I guess the tests fail because of something missing from the > isolated build environment. This is a longstanding problem unaffected > by this patch. > >> From 1ffc21f29ce255a1d3f613cb42532a0233f03b60 Mon Sep 17 00:00:00 2001 >> From: Eric Dvorsak >> Date: Sun, 2 Aug 2015 19:27:24 +0200 >> Subject: [PATCH] gnu: python-2: update to 2.7.10. Remove patches added >> to >> upstream > > This needs a proper commit log that lists the changes made to each > file, > conforming to our conventions. See our existing git commits for > examples. In this case, it should look something like this: > > --8<---------------cut here---------------start------------->8--- > gnu: python-2: Update to 2.7.10. > > * gnu/packages/patches/python2-sqlite-3.8.4-test-fix.patch, > gnu/packages/patches/python-libffi-mips-n32-fix.patch: Remove files. > * gnu-system.am (dist_patch_DATA): Remove them. > * gnu/packages/python.scm (python-2): Update to 2.7.10. Remove > patches. > Update comment showing test failures. > --8<---------------cut here---------------end--------------->8--- > >> diff --git a/gnu/packages/python.scm b/gnu/packages/python.scm >> index 4c13316..4e08fc9 100644 >> --- a/gnu/packages/python.scm >> +++ b/gnu/packages/python.scm > > Please add a copyright line for yourself to the top of this file. > >> @@ -74,63 +74,55 @@ >> (define-public python-2 >> (package >> (name "python") >> - (version "2.7.6") >> + (version "2.7.10") >> (source >> (origin >> (method url-fetch) >> (uri (string-append "https://www.python.org/ftp/python/" >> version "/Python-" version ".tar.xz")) >> - (patches (list (search-patch >> "python-libffi-mips-n32-fix.patch") >> - (search-patch >> "python2-sqlite-3.8.4-test-fix.patch"))) >> - (patch-flags '("-p0")) >> (sha256 >> (base32 >> - "18gnpyh071dxa0rv3silrz92jw9qpblswzwv4gzqcwxzz20qxmhz")))) >> + "1h7zbrf9pkj29hlm18b10548ch9757f75m64l47sy75rh43p7lqw")))) > > The open quote should be below the "b", as it was before. > >> (build-system gnu-build-system) >> (arguments >> `(#:tests? #f >> -;; 258 tests OK. >> -;; 103 tests failed: >> -;; test_bz2 test_distutils test_file test_file2k test_popen2 >> -;; test_shutil test_signal test_site test_slice test_smtplib >> -;; test_smtpnet test_socket test_socketserver test_softspace >> -;; test_sort test_sqlite test_ssl test_startfile test_str >> -;; test_strftime test_string test_stringprep test_strop >> test_strptime >> -;; test_strtod test_struct test_structmembers test_structseq >> -;; test_subprocess test_sunaudiodev test_sundry >> test_symtable >> -;; test_syntax test_sys test_sys_setprofile >> test_sys_settrace >> -;; test_sysconfig test_tarfile test_tcl test_telnetlib >> test_tempfile >> -;; test_textwrap test_thread test_threaded_import >> -;; test_threadedtempfile test_threading test_threading_local >> -;; test_threadsignals test_time test_timeout test_tk >> test_tokenize >> -;; test_tools test_trace test_traceback test_transformer >> -;; test_ttk_guionly test_ttk_textonly test_tuple >> test_typechecks >> -;; test_ucn test_unary test_undocumented_details >> test_unicode >> -;; test_unicode_file test_unicodedata test_univnewlines >> -;; test_univnewlines2k test_unpack test_urllib test_urllib2 >> -;; test_urllib2_localnet test_urllib2net test_urllibnet >> test_urlparse >> -;; test_userdict test_userlist test_userstring test_uu >> test_uuid >> -;; test_wait3 test_wait4 test_warnings test_wave >> test_weakref >> -;; test_weakset test_whichdb test_winreg test_winsound >> test_with >> -;; test_wsgiref test_xdrlib test_xml_etree test_xml_etree_c >> -;; test_xmllib test_xmlrpc test_xpickle test_xrange >> test_zipfile >> -;; test_zipfile64 test_zipimport test_zipimport_support >> test_zlib >> -;; 31 tests skipped: >> -;; test_aepack test_al test_applesingle test_ascii_formatd >> test_bsddb >> -;; test_bsddb185 test_bsddb3 test_cd test_cl >> test_codecmaps_cn >> -;; test_codecmaps_hk test_codecmaps_jp test_codecmaps_kr >> -;; test_codecmaps_tw test_ctypes test_curses test_dl >> test_gdb test_gl >> -;; test_imageop test_imgfile test_ioctl test_kqueue >> -;; test_linuxaudiodev test_macos test_macostools test_msilib >> -;; test_multiprocessing test_ossaudiodev test_pep277 >> -;; test_scriptpackages >> -;; 7 skips unexpected on linux2: >> -;; test_ascii_formatd test_bsddb test_bsddb3 test_ctypes >> test_gdb >> -;; test_ioctl test_multiprocessing >> -;; One of the typical errors: >> -;; test_unicode >> -;; test test_unicode crashed -- : >> [Errno 2] No such file or directory > > You removed the three lines above but didn't replace them with > anything analogous. They seem useful. > >> - #:test-target "test" >> + ;; 268 tests OK. >> + ;; 103 tests failed: >> + ;; test_distutils test_shutil test_signal test_site >> test_slice >> + ;; test_smtplib test_smtpnet test_socket test_socketserver >> + ;; test_softspace test_sort test_spwd test_sqlite test_ssl >> + ;; test_startfile test_stat test_str test_strftime >> test_string >> + ;; test_stringprep test_strop test_strptime test_strtod >> test_struct >> + ;; test_structmembers test_structseq test_subprocess >> test_sunau >> + ;; test_sunaudiodev test_sundry test_symtable test_syntax >> test_sys >> + ;; test_sys_setprofile test_sys_settrace test_sysconfig >> test_tarfile >> + ;; test_tcl test_telnetlib test_tempfile test_textwrap >> test_thread >> + ;; test_threaded_import test_threadedtempfile test_threading >> + ;; test_threading_local test_threadsignals test_time >> test_timeit >> + ;; test_timeout test_tk test_tokenize test_tools test_trace >> + ;; test_traceback test_transformer test_ttk_guionly >> test_ttk_textonly >> + ;; test_tuple test_typechecks test_ucn test_unary >> + ;; test_undocumented_details test_unicode test_unicode_file >> + ;; test_unicodedata test_univnewlines test_univnewlines2k >> test_unpack >> + ;; test_urllib test_urllib2 test_urllib2_localnet >> test_urllib2net >> + ;; test_urllibnet test_urlparse test_userdict test_userlist >> + ;; test_userstring test_uu test_uuid test_wait3 test_wait4 >> + ;; test_warnings test_wave test_weakref test_weakset >> test_whichdb >> + ;; test_winreg test_winsound test_with test_wsgiref >> test_xdrlib >> + ;; test_xml_etree test_xml_etree_c test_xmllib test_xmlrpc >> + ;; test_xpickle test_xrange test_zipfile test_zipfile64 >> + ;; test_zipimport test_zipimport_support test_zlib >> + ;; 30 tests skipped: >> + ;; test_aepack test_al test_applesingle test_bsddb >> test_bsddb185 >> + ;; test_bsddb3 test_cd test_cl test_codecmaps_cn >> test_codecmaps_hk >> + ;; test_codecmaps_jp test_codecmaps_kr test_codecmaps_tw >> test_crypt >> + ;; test_curses test_dl test_gdb test_gl test_idle >> test_imageop >> + ;; test_imgfile test_ioctl test_kqueue test_linuxaudiodev >> test_macos >> + ;; test_macostools test_msilib test_nis test_ossaudiodev >> + ;; test_scriptpackages >> + ;; 6 skips unexpected on linux2: >> + ;; test_bsddb test_bsddb3 test_crypt test_gdb test_idle >> test_ioctl >> + #:test-target "test" >> #:configure-flags > > The large comment and the #:test-target line should be aligned with the > existing #:tests? #f and #:configure-flags lines. In other words, two > more spaces are needed before each comment line above, and three more > spaces are needed before #:test-target. > > Can you send an updated patch? > > Thanks! > Mark