From mboxrd@z Thu Jan 1 00:00:00 1970 From: Pjotr Prins Subject: Re: [PATCH] Add Elixir (was: ) Date: Mon, 25 Jul 2016 08:31:40 +0200 Message-ID: <20160725063140.GA25432@thebird.nl> References: <579027b7.VHXjhpPxQC3AAmeY%pjotr.public12@email> <87mvl66ya6.fsf@elephly.net> 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]:39876) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bRZTy-0008KE-40 for guix-devel@gnu.org; Mon, 25 Jul 2016 02:35:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bRZTu-0004CT-UX for guix-devel@gnu.org; Mon, 25 Jul 2016 02:35:18 -0400 Received: from mail.thebird.nl ([95.154.246.10]:47246) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bRZTu-0004BO-M3 for guix-devel@gnu.org; Mon, 25 Jul 2016 02:35:14 -0400 Content-Disposition: inline In-Reply-To: <87mvl66ya6.fsf@elephly.net> 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: Ricardo Wurmus Cc: guix-devel@gnu.org, Pjotr Prins On Mon, Jul 25, 2016 at 08:13:33AM +0200, Ricardo Wurmus wrote: > back to the original patch, which I didn=E2=80=99t look at as the ensui= ng > discussion on review process and registry proposals took up more time > than I anticipated. :) > I=E2=80=99m a little uncertain on how to proceed. I have a couple of t= hings > here that I=E2=80=99d like to change before commiting. I=E2=80=99ll ad= d some comments > below. Indentation changes won=E2=80=99t be mentioned ;) >=20 > If you are okay with the proposed changes I can apply them on top of > your patch and resubmit the squashed patch to the ML for a final > look-over. Deal? Sure. I have no ego in this. I am happy if you take over. > > + (native-inputs > > + `(("patch" ,patch) > > + ("patch/elixir-disable-failing-tests" > > + ,(search-patch "elixir-disable-failing-tests.patch")) > > + ("patch/elixir-disable-mix-tests" > > + ,(search-patch "elixir-disable-mix-tests.patch")))) >=20 > This has been mentioned already and I=E2=80=99d like to move these to t= he > =E2=80=9Csource=E2=80=9D field after identifying the reason why the bui= ld fails > otherwise. I see that you=E2=80=99re doing this in order to patch afte= r the > build phase. Let=E2=80=99s see if this can be avoided. I tried and failed. Elixir people do not know either: https://github.com/elixir-lang/elixir/issues/5043 I think it is Mix magic. Probably tracking files in some way. > > + (add-after 'build 'disable-breaking-elixir-tests > > + ;; when patching tests as part of source the build breaks, s= o we do > > + ;; it after the build phase > > + (lambda* (#:key inputs #:allow-other-keys) > > + (and > > + (zero? (system* "patch" "--force" "-p1" "-i" > > + (assoc-ref inputs "patch/elixir-disable-f= ailing-tests"))) > > + (zero? (system* "patch" "--force" "-p1" "-i" > > + (assoc-ref inputs "patch/elixir-disable-m= ix-tests"))) > > + ;; Tests currently fail in these two files: > > + (delete-file "./lib/mix/test/mix/tasks/deps.git_test.exs"= ) > > + (delete-file "./lib/mix/test/mix/shell_test.exs")))) >=20 > =E2=80=9Cdelete-file=E2=80=9D has an unspecified return value, so chain= ing it up in > =E2=80=9Cand=E2=80=9D isn=E2=80=99t guaranteed to work. Should this pa= tch-after-build business > turn out to be unavoidable I suggest just deleting the files first, the= n > and-ing the =E2=80=9Czero?=E2=80=9D expressions. Cool. > > + (replace 'check > > + (lambda _ > > + (zero? (system* "make" "test"))))) ;; 3124 tests, 0= failures, 11 skipped >=20 > We can use =E2=80=9C#:test-target "test"=E2=80=9D instead of replacing = the =E2=80=9Ccheck=E2=80=9D phase. Yes, I forgot. > > + #:make-flags (list (string-append "PREFIX=3D" %output)))) > > + (home-page "http://elixir-lang.org/") > > + (synopsis "The Elixir programming language") >=20 > s/The// >=20 > > + (description "Elixir is a dynamic, functional language used to > > +build scalable and maintainable applications. Elixir leverages the > > +Erlang VM, known for running low-latency, distributed and > > +fault-tolerant systems, while also being successfully used in web > > +development and the embedded software domain.") > > + (license license:asl2.0))) >=20 > Looks good! >=20 > > diff --git a/gnu/packages/patches/elixir-disable-failing-tests.patch = b/gnu/packages/patches/elixir-disable-failing-tests.patch > > new file mode 100644 > > index 0000000..802cb1e > > --- /dev/null > > +++ b/gnu/packages/patches/elixir-disable-failing-tests.patch >=20 > While I=E2=80=99m generally okay with disabling failing tests (see the > embarassing situation we have with the =E2=80=9Cicedtea=E2=80=9D packag= es), I think > these can be fixed with little effort. Many of them seem to be related > to the location of the temp directory. Note we are talking a rather small minority of tests. 11 out of 2000+ for Elixir. For Mix 10% fails, mostly because of git. The Elixir people wrote there should be no network access involved. > > +diff --git a/lib/elixir/test/elixir/node_test.exs b/lib/elixir/test/= elixir/node_test.exs > > +index d1f1fe6..5c2d469 100644 > > +--- a/lib/elixir/test/elixir/node_test.exs > > ++++ b/lib/elixir/test/elixir/node_test.exs > > +@@ -6,8 +6,10 @@ defmodule NodeTest do > > + doctest Node > > +=20 > > + test "start/3 and stop/0" do > > +- assert Node.stop =3D=3D {:error, :not_found} > > +- assert {:ok, _} =3D Node.start(:hello, :shortnames, 15000) > > +- assert Node.stop() =3D=3D :ok > > ++ IO.puts "Skipping test because GNU Guix does not allow the HOME= environment variable." > > ++ > > ++ # assert Node.stop =3D=3D {:error, :not_found} > > ++ # assert {:ok, _} =3D Node.start(:hello, :shortnames, 15000) > > ++ # assert Node.stop() =3D=3D :ok > > + end > > + end >=20 > This was already addressed earlier. We can probably just setenv HOME > before the tests. >=20 > Some of the remaining tests don=E2=80=99t seem to have any obvious fixe= s, so > I=E2=80=99ll get to them after making the above changes first. Maybe t= he > failures disappear then. >=20 > Thanks again for the patch. I hope you are willing to provide some > guidance when I have some problems understanding certain bits of the > build. I am happy to help out if you take the lead. > PS: Elixir is big and getting it accepted in Guix upstream is a > precondition for more Elixir packages. This is why I think it=E2=80=99= s worth > picking this up. =20 Yes, very visible language and rapidly growing community. Pj. --=20