From mboxrd@z Thu Jan 1 00:00:00 1970 From: ludo@gnu.org (Ludovic =?utf-8?Q?Court=C3=A8s?=) Subject: Re: [PATCH] gnu: Add ldc-1.1.0-beta6 Date: Mon, 09 Jan 2017 15:41:40 +0100 Message-ID: <87y3ykmh23.fsf@gnu.org> References: Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:47724) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cQb8t-0000Br-M0 for guix-devel@gnu.org; Mon, 09 Jan 2017 09:41:48 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cQb8p-0005xX-Pb for guix-devel@gnu.org; Mon, 09 Jan 2017 09:41:47 -0500 In-Reply-To: (Frederick Muriithi's message of "Fri, 6 Jan 2017 18:04:38 +0300") List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: "Guix-devel" To: Frederick Muriithi Cc: guix-devel@gnu.org Hello! Sorry for the late reply! Frederick Muriithi skribis: > Based on thread > https://lists.gnu.org/archive/html/guix-devel/2017-01/msg00465.html > > - Fixed some issues raised by Danny Milosavljevic on thread above > - Updated the beta version to 6, up from 4 > - Started up a thread on ldc forum to have a version flag added to > deactivate network tests when building ldc > https://forum.dlang.org/post/zmdbdgnzrxyvtpqafvyg@forum.dlang.org Nice! > From a0185dc1f5881bae8094643251335b04560900a0 Mon Sep 17 00:00:00 2001 > From: Muriithi Frederick Muriuki > Date: Fri, 6 Jan 2017 17:51:18 +0300 > Subject: [PATCH] gnu: Add ldc-1.1.0-beta6 > > * gnu/packages/ldc.scm (ldc-1.1.0-beta6): New variable > * gnu/packages/ldc.scm (ldc-beta): New variable > * gnu/packages/patches/ldc1.1.0-disable-dmd-tests.patch: New patch > * gnu/packages/patches/ldc1.1.0-disable-phobos-tests.patch: New patch Overall the patch looks good to me. One question: We usually avoid packaging software that has no release or has an =E2=80=9Calpha=E2=80=9D or =E2=80=9Cbeta=E2=80=9D label. Do you thi= nk we could wait for 1.1.0 to be officially released? Or are there good reasons why we should not wait? Some minor issues: > +(define-public ldc-1.1.0-beta6 > + (let ((older-version "1.1.0-beta4")) [...] > + ("phobos-src" > + ,(origin > + (method url-fetch) > + (uri (string-append > + "https://github.com/ldc-developers/phobos/archive/ldc-v" > + older-version ".tar.gz")) > + (sha256 > + (base32 > + "1iwy5rs0rqkicj1zfsa5yqvk8ard99bfr8g69qmhlbzb98q0kpks")) > + (patches (search-patches "ldc1.1.0-disable-phobos-tests.patch= ")))) > + ("druntime-src" > + ,(origin > + (method url-fetch) > + (uri (string-append > + "https://github.com/ldc-developers/druntime/archive/ldc= -v" > + older-version ".tar.gz")) > + (sha256 > + (base32 > + "1qsiw5lz1pr8ms9myjf8b94nqi7f1781k226jvxwnhkjg11d0s63")))) > + ("dmd-testsuite-src" > + ,(origin > + (method url-fetch) > + (uri (string-append > + "https://github.com/ldc-developers/dmd-testsuite/archiv= e/ldc-v" > + older-version ".tar.gz")) > + (sha256 > + (base32 > + "0jp54hyi75i9g41rvgmm3zg21yzv57q8dghrhb432rb0n9j15mbp")) > + ;; Deactivate some failing gdb tests. Most other gdb tests pa= ss > + (patches (search-patches "ldc1.1.0-disable-dmd-tests.patch"))= ))))))) Could you add a comment explaining why the previous version of these is needed, instead of the current version? > --- /dev/null > +++ b/gnu/packages/patches/ldc1.1.0-disable-dmd-tests.patch Could you add a line or two explaining at the top of patch explaining what it does and why, and what its upstream status is? For example, I think this one disables a test that would require GDB, which is not an input (?), and presumably it won=E2=80=99t be submitted upstream. > @@ -0,0 +1,28 @@ > +diff --git a/d_do_test.d b/d_do_test.d > +index aa67169..7a4dcc1 100755 > +--- a/d_do_test.d > ++++ b/d_do_test.d > +@@ -645,8 +645,8 @@ int main(string[] args) > + auto gdb_output =3D execute(fThisRun, command, true= , result_path); > + if (testArgs.gdbMatch !is null) > + { > +- enforce(match(gdb_output, regex(testArgs.gdbMat= ch)), > +- "\nGDB regex: '"~testArgs.gdbMatch~"' d= idn't match output:\n----\n"~gdb_output~"\n----\n"); > ++ //enforce(match(gdb_output, regex(testArgs.gdbM= atch)), > ++ //"\nGDB regex: '"~testArgs.gdbMatch~"'= didn't match output:\n----\n"~gdb_output~"\n----\n"); I think it=E2=80=99s better to just delete the two lines instead of comment= ing them out: that makes the patch easier to read. Hope this makes sense. Thanks for your contribution! Ludo=E2=80=99.