From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ricardo Wurmus Subject: [PATCH] Add Elixir (was: ) Date: Mon, 25 Jul 2016 08:13:33 +0200 Message-ID: <87mvl66ya6.fsf@elephly.net> References: <579027b7.VHXjhpPxQC3AAmeY%pjotr.public12@email> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:36722) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bRZ96-0003MT-Ih for guix-devel@gnu.org; Mon, 25 Jul 2016 02:13:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bRZ91-000067-UR for guix-devel@gnu.org; Mon, 25 Jul 2016 02:13:43 -0400 Received: from sender163-mail.zoho.com ([74.201.84.163]:24752) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bRZ91-00005a-MP for guix-devel@gnu.org; Mon, 25 Jul 2016 02:13:39 -0400 In-reply-to: <579027b7.VHXjhpPxQC3AAmeY%pjotr.public12@email> 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: Pjotr Prins Cc: guix-devel@gnu.org Hi Pjotr > From 5fd8f64794b27f59f6688177a7a9e532b5d57f01 Mon Sep 17 00:00:00 2001 > Date: Tue, 19 Jul 2016 11:13:27 +0000 > Subject: [PATCH] gnu: Add elixir. > To: guix-devel@gnu.org > From: Pjotr Prins > References: <578e47d0.i8Ovns6KhzHqzVNC%pjotr.public12@thebird.nl> > > * gnu/packages/elixir.scm: New file. > * gnu/local.mk (GNU_SYSTEM_MODULES): Add it. back to the original patch, which I didn’t look at as the ensuing discussion on review process and registry proposals took up more time than I anticipated. I’m a little uncertain on how to proceed. I have a couple of things here that I’d like to change before commiting. I’ll add some comments below. Indentation changes won’t be mentioned ;) 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? > + (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")))) This has been mentioned already and I’d like to move these to the “source” field after identifying the reason why the build fails otherwise. I see that you’re doing this in order to patch after the build phase. Let’s see if this can be avoided. > + (add-after 'build 'disable-breaking-elixir-tests > + ;; when patching tests as part of source the build breaks, so 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-failing-tests"))) > + (zero? (system* "patch" "--force" "-p1" "-i" > + (assoc-ref inputs "patch/elixir-disable-mix-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")))) “delete-file” has an unspecified return value, so chaining it up in “and” isn’t guaranteed to work. Should this patch-after-build business turn out to be unavoidable I suggest just deleting the files first, then and-ing the “zero?” expressions. > + (replace 'check > + (lambda _ > + (zero? (system* "make" "test"))))) ;; 3124 tests, 0 failures, 11 skipped We can use “#:test-target "test"” instead of replacing the “check” phase. > + #:make-flags (list (string-append "PREFIX=" %output)))) > + (home-page "http://elixir-lang.org/") > + (synopsis "The Elixir programming language") s/The// > + (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))) Looks good! > 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 While I’m generally okay with disabling failing tests (see the embarassing situation we have with the “icedtea” packages), I think these can be fixed with little effort. Many of them seem to be related to the location of the temp directory. > +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 > + > + test "start/3 and stop/0" do > +- assert Node.stop == {:error, :not_found} > +- assert {:ok, _} = Node.start(:hello, :shortnames, 15000) > +- assert Node.stop() == :ok > ++ IO.puts "Skipping test because GNU Guix does not allow the HOME environment variable." > ++ > ++ # assert Node.stop == {:error, :not_found} > ++ # assert {:ok, _} = Node.start(:hello, :shortnames, 15000) > ++ # assert Node.stop() == :ok > + end > + end This was already addressed earlier. We can probably just setenv HOME before the tests. Some of the remaining tests don’t seem to have any obvious fixes, so I’ll get to them after making the above changes first. Maybe the failures disappear then. 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. ~~ Ricardo PS: Elixir is big and getting it accepted in Guix upstream is a precondition for more Elixir packages. This is why I think it’s worth picking this up. For other patches this amount of effort may not be justified (as I cannot get other stuff done at the same time). I do hope that we can find a way to upstream the 140+ bioinfo packages you have locally, even it it’s at a trickle rate :)