kawa 2.3 #8301

duncanmak · 2016-12-29T18:56:21Z

The existing formula builds kawa using incorrect configure flags. The
resulting jar file is missing functionality (can't import SRFI
libraries). The installation also doesn't include the new JARs that's
part of the distribution, and domterm.jar, jline.jar and servlet.jar,
which means the REPL is crippled without line editing functionality.

Fix the formula to use the official binary that's distributed by the
maintainer and add test for importing built-in libraries.

Have you followed the guidelines for contributing?
Have you checked that there aren't other open pull requests for the same formula update/change?
Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

ilovezfs · 2016-12-29T19:00:44Z

I'm not thrilled with the notion that make install requires upstream magic to work. What exactly is the problem with the configure flags?

duncanmak · 2016-12-29T19:11:49Z

@ilovezfs If you run the existing formula, the REPL is broken, because of missing the jline functionality for line editing. The official distribution runs correctly.

There is already a change for make install in the existing formula, I just added another case to look in libexec instead of lib.

I wrote to the maintainer and he recommends using the official binary release - https://sourceware.org/ml/kawa/2016-q4/msg00089.html

ilovezfs · 2016-12-29T19:13:11Z

I think it just needs ENV.deparallelize so that the test runs correctly.

ilovezfs · 2016-12-29T19:15:25Z

Specifically, please try this:

diff --git a/Formula/kawa.rb b/Formula/kawa.rb
index 0ee7a2c915..55a6c461e2 100644
--- a/Formula/kawa.rb
+++ b/Formula/kawa.rb
@@ -15,6 +15,7 @@ class Kawa < Formula
   depends_on :java
 
   def install
+    ENV.deparallelize
     system "./configure", "--disable-debug",
                           "--disable-dependency-tracking",
                           "--disable-silent-rules",
@@ -27,6 +28,6 @@ class Kawa < Formula
   end
 
   test do
-    system bin/"kawa", "--help"
+    system bin/"kawa", "-e", "(import (srfi 1))"
   end
 end

duncanmak · 2016-12-29T19:21:53Z

I'll give it a try, that might fix the library import issue.

To get line editing to work in the REPL, I think there are more flags missing to configure.

From kawa-2.2's Makefile:

dist-kawa.jar: dist
        srcdir=`(cd $(srcdir); /bin/pwd)` \
          && rm -rf tmpdir \
          && mkdir -p tmpdir tmpdir/lib \
          && cd tmpdir \
          && tar xzf ../$(distdir).tar.gz \
          && rm -f ./patch-source-list \
          && ./kawa-$(VERSION)/configure --with-java-source=7 \
            --with-domterm="@WITH_DOMTERM_ARG@" \
            --with-jline$(JLINE_VERSION_MAJOR)="@WITH_JLINE_PATH@" \
            --with-servlet=@WITH_SERVLET_ARG@ --with-javafx=@WITH_JAVAFX_ARG@ \
          && CLASSPATH=@conf_classpath@.$(CLASSPATH_SEPARATOR)$$CLASSPATH $(MAKE) all JAVAC="$(JAVAC) -source 7 -target 7" \
          && mv lib/kawa.jar ../dist-kawa.jar \
          && cd .. && rm -rf tmpdir

ZIP_VERSION = $(VERSION)
kawa.zip: kawa-$(ZIP_VERSION).zip
kawa-snapshot.zip:
        $(MAKE) kawa.zip ZIP_VERSION=$(VERSION)-`date +%Y%m%d`
kawa-$(ZIP_VERSION).zip: dist-kawa.jar doc/kawa-manual.epub bin/kawa.bat
        mkdir -p kawa-$(VERSION) kawa-$(VERSION)/lib  kawa-$(VERSION)/bin kawa-$(VERSION)/doc
        cd kawa-$(VERSION)/lib; \
          $(LN_S) ../../dist-kawa.jar kawa.jar; \
          $(LN_S) @WITH_DOMTERM_ARG@ domterm.jar; \
          $(LN_S) @WITH_SERVLET_ARG@ servlet.jar; \
          $(LN_S) @WITH_SERVLET_ARG@ servlet.jar; \
          $(LN_S) @WITH_JLINE_PATH@ jline.jar
        docsrcdir="`cd $(srcdir); pwd`/doc"; \
          cd kawa-$(VERSION)/doc; \
          $(LN_S) ../../doc/kawa-manual.epub kawa-manual.epub; \
          $(LN_S) $$docsrcdir/README-epub README-epub; \
          $(LN_S) $$docsrcdir/+default+ +default+
        cd kawa-$(VERSION)/bin; \
          $(LN_S) ../../bin/kawa.sh kawa; \
          $(LN_S) ../../bin/kawa.bat kawa.bat
        zip $@ kawa-$(VERSION)/bin/kawa -r kawa-$(VERSION)

ilovezfs · 2016-12-29T19:23:11Z

Right, we'd need to build https://github.com/jline/jline3 etc.

duncanmak · 2016-12-29T19:24:24Z

I don't understand the reluctance to use the existing binary distribution - similar projects like jruby and apache-spark and scala all use the project-official binary distribution.

ilovezfs · 2016-12-29T19:27:24Z

I'd prefer we build those too ;)

duncanmak · 2016-12-29T19:46:14Z

I really just want a working installation of kawa out of homebrew, and this patch got me there. Also, even by building it and then distributing binary Bottles of the build, it's all quite circular.

Either way, I tried your suggestion, it did not fix the two issues. (1) Importing SRFIs, (2) line editing in REPL.

I'm gonna go back to hacking in Kawa for now. I think other Kawa users on MacOS will also appreciate having a working installation, so please consider merging this in.

Let me know if there's another else I can do. Thanks!

duncan@furigana:Formula (master)$ git --no-pager diff
diff --git a/Formula/kawa.rb b/Formula/kawa.rb
index 0ee7a2c91..483989e3c 100644
--- a/Formula/kawa.rb
+++ b/Formula/kawa.rb
@@ -15,6 +15,8 @@ class Kawa < Formula
   depends_on :java
 
   def install
+    ENV.deparallelize
+
     system "./configure", "--disable-debug",
                           "--disable-dependency-tracking",
                           "--disable-silent-rules",
@@ -27,6 +29,6 @@ class Kawa < Formula
   end
 
   test do
-    system bin/"kawa", "--help"
+    system bin/"kawa", "-e", "(import (srfi 1))"
   end
 end
duncan@furigana:Formula (master)$ brew uninstall --force kawa
Uninstalling kawa... (19 files, 3.5M)
duncan@furigana:Formula (master)$ brew install kawa
==> Downloading https://homebrew.bintray.com/bottles/kawa-2.2.el_capitan.bottle.tar.gz
Already downloaded: /Users/duncan/Library/Caches/Homebrew/kawa-2.2.el_capitan.bottle.tar.gz
==> Pouring kawa-2.2.el_capitan.bottle.tar.gz
🍺  /usr/local/Cellar/kawa/2.2: 19 files, 3.5M
duncan@furigana:Formula (master)$ brew test kawa
Testing kawa
==> Using the sandbox
==> /usr/local/Cellar/kawa/2.2/bin/kawa -e (import (srfi 1))
Last 15 lines from /Users/duncan/Library/Logs/Homebrew/kawa/test.01.kawa:
2016-12-29 14:41:19 -0500

/usr/local/Cellar/kawa/2.2/bin/kawa
-e
(import (srfi 1))

<string>:1:9: unknown library (srfi 1)
Error: kawa: failed

ilovezfs · 2016-12-29T19:47:55Z

You re-poured the bottle and did not test the diff.

duncanmak · 2016-12-29T19:51:06Z

Okay, Env. deparallelize fixed the importing of SRFIs.

PerBothner · 2016-12-29T20:58:34Z

[The maintainer - i.e. me] recommends using the official binary release

I did not mean to necessarily recommend that - though that is one option. If building from source, what I do suggest is that Homebrew should install Kawa in a directory layout similar to the Kawa binary releases, and then optionally creating symlinks to (files in) that directory. (I'm being vague because I know little about MacOS and less about Homebrew.)

For example on Linux you might install into /usr/share/kawa/{bin,lib,doc} and then create a symlink from /usr/bin/kawa to /usr/share/kawa/bin/kawa. Change those paths as appropriate for Homebrew.

(You can still use the old readline front-end, but it has fewer features, and is no longer the default.)

MikeMcQuaid · 2016-12-30T13:42:32Z

Formula/kawa.rb

+    inreplace "bin/kawa" do |s|
+      s.gsub! /thisfile=`type -p \$0`/, "thisfile=#{bin}/kawa"
+      s.gsub! %r{\$kawadir/lib}, "$kawadir/libexec"
+    end


Would be good to submit these patches upstream; we'd rather not have a patch like this indefinitely.

A better approach may be to install bin and lib to libexec and use a symlink or wrapper script which looks like it could avoid all this patching.

MikeMcQuaid · 2016-12-30T13:43:56Z

Formula/kawa.rb

-  end
+  url "https://ftpmirror.gnu.org/kawa/kawa-2.2.zip"
+  mirror "https://ftp.gnu.org/gnu/kawa/kawa-2.2.zip"
+  sha256 "eb2b8301786d5e983769c2e29ba7caf69afcb14977eac16d544471b9a3c79f79"

  depends_on :java


Is there a specific version we could add here e.g. depends_on :java => "1.7+"?

Yeah, I see from the release notes that since Kawa 2.0, it targets Java 7.

From the release notes:

The default is now Java 7, rather than Java 6. This means the checked-in source code is pre-processed for Java 7, and future binary releases will require Java 7.

MikeMcQuaid · 2016-12-30T13:44:44Z

I'm fine with us using the binary release. We typically do so for Java stuff as there's no benefit to building from source (either optimisation or cross-platformability).

duncanmak · 2016-12-30T20:15:50Z

Thanks for the review @MikeMcQuaid!

I think this is good to go. I have updated the PR with your suggestions and rebased it on top of the latest master.

ilovezfs · 2016-12-30T21:11:35Z

The original reason I had for converting this to use a source build is that the published source tarball had triggered brew livecheck to detect a new version, but the zip file was not posted. That throws a monkey wrench into the automation since it will attempt to retrieve the URL for the new version and then 404. At that point it goes in the pile of failed updates, never to be heard from again until someone (me if I circle back possibly weeks later) comes along to dig it out of the pile if by then the binary jar files version has appeared.

Rather than ending up in this situation repeatedly, I thought it would be better to switch us over to use the source tarball, which at the time I converted it was in fact the only one actually available for the latest tag.

MikeMcQuaid · 2016-12-31T17:07:54Z

Rather than ending up in this situation repeatedly, I thought it would be better to switch us over to use the source tarball, which at the time I converted it was in fact the only one actually available for the latest tag.

Thanks @ilovezfs. I'd probably like to hear some upstream thoughts on this before we merge this.

duncanmak · 2017-01-02T17:33:31Z

@PerBothner is the Kawa maintainer and he cuts the releases, so he can speak to the timeliness of the binary releases.

Note: I volunteer to take over the maintenance of this Formula. I'm active on the Kawa mailing list and I can work with Per to make sure this gets updated whenever he makes new Kawa releases.

MikeMcQuaid · 2017-01-02T17:38:25Z

Note: I volunteer to take over the maintenance of this Formula. I'm active on the Kawa mailing list and I can work with Per to make sure this gets updated whenever he makes new Kawa releases.

@duncanmak Homebrew doesn't really work like that, I'm afraid. You can't maintain the formula as you don't have commit access to do so. You can submit updates but we'll still run into problems with e.g. @ilovezfs's script.

PerBothner · 2017-01-04T00:16:56Z

The Kawa release process changed a lot last year, in addition to the source repository moving to gitlab. A "binary release" used to be just a jar file; it is now a zip that includes the jar file, a shell script, helper libraries and documentation. I think homebrew should just grab a recent binary release and unzip it. The issue is figuring out which binary release and when it grab it.

What is unclear to me is how homebrew handles updates - i.e. new releases. Does a homebrew maintainer perform any kind of update/release? Is it handled when a user types 'brew update'? If there is a central database of versions, is it updated manually or is there some mechanism to do it automatically?

If it is automatic, I can establish a convention with either a file naming a "current release" or a symlink to the current release.

duncanmak · 2017-01-04T20:03:19Z

@MikeMcQuaid Is there anything else I need to do to this PR in the Ruby code?

MikeMcQuaid · 2017-01-05T18:33:02Z

@duncanmak I'd like to hear the thoughts of @ilovezfs first.

ilovezfs · 2017-01-16T12:38:44Z

Looks like 2.3 is out now according to livecheck though the website is saying 2.2.

PerBothner · 2017-01-16T17:17:25Z

I made the official Kawa 2.3 release this morning, including updating the website and sending an announcement to the Kawa mailing list.

(I re-uploaded the release to fix a small but very visible documentation bug. I re-used the 2.3 version number since it was only a slight doc change.)

I suggest using the latest ftp://ftp.gnu.org/pub/gnu/kawa/kawa-VERSION.zip (where VERSION is an actual version number, not latest) as the base for Homebrew going forward.

ilovezfs · 2017-01-17T05:37:23Z

Formula/kawa.rb

-      'while test -L "$thisfile"; do thisfile=$(readlink -f "$thisfile"); done',
-      "thisfile=#{pkgshare}/bin/kawa"
+    rm Dir["bin/*.bat"]
+


remove blank lines 14 and 17 please

ilovezfs · 2017-01-17T05:38:15Z

If you can squash this down to one commit with the commit subject

kawa 2.3

that would be good.

bottle block was replaced with :bottle_unneeded as requested

ilovezfs · 2017-01-17T05:43:00Z

LGTM.

ilovezfs · 2017-01-17T06:00:27Z

🚀

Thanks @duncanmak for spotting the problems with this formula and fixing them, and for your patience.

PerBothner · 2017-01-17T06:04:43Z

A couple of things I suggest testing, if you haven't:

kawa --browse-manual

That should bring up a browser for the Kawa manual. See browse-manual in https://www.gnu.org/software/kawa/Options.html

kawa -w

Brings up a new window, preferably using DomTerm. (See manual for details.)

By default the REPL should be doing input editing using the jline library.

I haven't done any testing on MacOS, so if any of these fail, it may be an actual Kawa limitation (that should be fixed), rather than homebrew-related.

ilovezfs · 2017-01-17T06:06:15Z

kawa: --browse-manual failed: /usr/local/Cellar/kawa/2.3/libexec/doc/kawa-manual.epub does not exist

PerBothner · 2017-01-17T06:07:47Z

One reason for getting DomTerm working is so the Composable Pictures support works. There are other reasons - see https://www.gnu.org/software/kawa/REPL-Console.html

duncanmak · 2017-01-17T06:08:39Z

I noticed that kawa -w is broken on the Mac because of the terminal support. I've been trying to fix it, but I haven't had time.

…

On Tue, Jan 17, 2017 at 1:06 AM, ilovezfs ***@***.***> wrote: kawa: --browse-manual failed: /usr/local/Cellar/kawa/2.3/libexec/doc/kawa-manual.epub does not exist — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#8301 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAASt9ouyqBMaxOM6O1RS2bw8XhUto3Rks5rTFpYgaJpZM4LXxp1> .

-- Duncan.

duncanmak · 2017-01-17T06:09:22Z

This is what I see on my screen - it hasn't changed from Kawa 2.2.

ilovezfs · 2017-01-17T06:10:40Z

duncanmak · 2017-01-17T06:11:46Z

The path to the epub file is /usr/local/Cellar/kawa/2.3/share/doc/kawa/kawa-manual.epub.

Right now, it's looking for it at /usr/local/Cellar/kawa/2.3/libexec/doc/kawa-manual.epub.

PerBothner · 2017-01-17T06:12:11Z

I guessed there might be an issue with --browse-manual, because it didn't look like the doc directory was installed as a sibling directory to the bin directory. Specifically, the kawa.home property should point to a directory with the lib, bin, and doc sub-directories. That is why I recommended unziping the entire binary release (kawa-VERSION.zip) into a single tree and then adding symlinks as desired.

ilovezfs · 2017-01-17T06:23:32Z

ideally

prefix.install buildpath.children

would do the entire job with everything in the standard locations and no string replacements needed :)

PerBothner · 2017-01-17T06:24:05Z

Looks like some interaction between the DomTerm support and the JLine supprt. (Note promptTemplate2 should only be for continuation lines, so that is strange.)To only test DomTerm, without JLine, you can try:

kawa console:use-jline=no -w

ilovezfs · 2017-01-17T06:31:59Z

duncanmak · 2017-01-17T06:37:59Z

@ilovezfs the string replacement is to avoid using readlink -f, which is not supported on the Mac. That is in the Kawa shell wrapper code, part of the distribution. The patch (both your old one and my current one) makes the path absolute, so the readlink -f call is not needed.

It was @MikeMcQuaid who suggested installing to libexec (#8301 (comment)), so I just followed along.

I don't know what the norm is with Homebrew, but let's change it if that's not necessary.

ilovezfs · 2017-01-17T06:41:29Z

@duncanmak right. For portability, it "shouldn't" be using readlink -f

I don't know what the norm is with Homebrew, but let's change it if that's not necessary.

It is necessary here because the standard doc is $prefix/share/doc/kawa not $prefix/doc

PerBothner · 2017-01-17T07:24:04Z

If someone is willing to debug the -w Exception(s), they probably need to re-compile from source.

What is supposed to happen is shown by this log, after I added 'new Error(XXX).printStacTrace()' in appropriate places:

java.lang.Error: setCurL:kawa.standard.Scheme@1a86f2f1 th:Thread[main,5,main] p1:#|kawa:%N|#  p2:#|%P.%N|# 
	at gnu.expr.Language.setCurrentLanguage(Language.java:55)
	at gnu.expr.Language.setDefaults(Language.java:1206)
	at kawa.repl.getLanguage(repl.java:227)
	at kawa.repl.processArgs(repl.java:383)
	at kawa.repl.main(repl.java:820)
Started web server on port 41145.  Browse http://127.0.0.1:41145/
java.lang.Error: DomTerm.run th:Thread[Thread-9,5,main] p1:#|kawa:%N|#  p2:#|%P.%N|# 
	at kawa.DomTermBackend.run(DomTermBackend.java:132)
	at java.lang.Thread.run(Thread.java:745)

I.e the CheckConsole.prompt1 and CheckConsole.prompt2 ThreadLocations (which is a wrapper around InheritableThreadLocal) should be set in Language.setCurrentLanguage. While that is a different thread than the one running DomTermBackend.run, then latter is a child of the former, so it "should" work.

PerBothner · 2017-01-22T04:27:41Z

FYI I uploaded a binary snapshot of the development "invoke" branch kawa-2.91_invoke-20170121.zip. This should not be installed by HomeBrew. I don't know how livecheck determines a new version/release, but this is not a release, just a snapshot.

ilovezfs · 2017-01-22T05:22:32Z

@PerBothner Thanks. I've made it

  livecheck :url => "https://ftp.gnu.org/gnu/kawa",
            :regex => /href="kawa-(\d+\.\d+(\.\d+)?)\.zip/

so we should be OK.

PerBothner · 2017-01-22T15:53:30Z

That looks good.

duncanmak force-pushed the fix-kawa-2.2 branch from ce26d7d to a58d462 Compare December 29, 2016 19:00

ilovezfs added the do not merge label Dec 29, 2016

duncanmak force-pushed the fix-kawa-2.2 branch from a58d462 to 461c403 Compare December 29, 2016 19:04

duncanmak force-pushed the fix-kawa-2.2 branch from 461c403 to 7c69640 Compare December 29, 2016 19:13

MikeMcQuaid previously requested changes Dec 30, 2016

View reviewed changes

ilovezfs removed the do not merge label Dec 30, 2016

duncanmak force-pushed the fix-kawa-2.2 branch from 7c69640 to a18654a Compare December 30, 2016 20:03

ilovezfs reviewed Jan 17, 2017

View reviewed changes

ilovezfs changed the title ~~kawa: use official binary distribution instead~~ kawa 2.3 Jan 17, 2017

ilovezfs approved these changes Jan 17, 2017

View reviewed changes

duncanmak force-pushed the fix-kawa-2.2 branch from 859bcd3 to b50e6bd Compare January 17, 2017 05:42

kawa 2.3

e69fccb

ilovezfs force-pushed the fix-kawa-2.2 branch from b50e6bd to e69fccb Compare January 17, 2017 05:50

ilovezfs merged commit 46fc25e into Homebrew:master Jan 17, 2017

duncanmak deleted the fix-kawa-2.2 branch January 17, 2017 06:04

Homebrew locked and limited conversation to collaborators May 4, 2018

kawa 2.3 #8301

kawa 2.3 #8301

Conversation

duncanmak commented Dec 29, 2016 • edited Loading

ilovezfs commented Dec 29, 2016

duncanmak commented Dec 29, 2016

ilovezfs commented Dec 29, 2016

ilovezfs commented Dec 29, 2016

duncanmak commented Dec 29, 2016

ilovezfs commented Dec 29, 2016

duncanmak commented Dec 29, 2016

ilovezfs commented Dec 29, 2016

duncanmak commented Dec 29, 2016

ilovezfs commented Dec 29, 2016

duncanmak commented Dec 29, 2016

PerBothner commented Dec 29, 2016

MikeMcQuaid Dec 30, 2016

Choose a reason for hiding this comment

MikeMcQuaid Dec 30, 2016

Choose a reason for hiding this comment

MikeMcQuaid Dec 30, 2016

Choose a reason for hiding this comment

duncanmak Dec 30, 2016

Choose a reason for hiding this comment

MikeMcQuaid commented Dec 30, 2016

duncanmak commented Dec 30, 2016

ilovezfs commented Dec 30, 2016

MikeMcQuaid commented Dec 31, 2016

duncanmak commented Jan 2, 2017

MikeMcQuaid commented Jan 2, 2017

PerBothner commented Jan 4, 2017

duncanmak commented Jan 4, 2017

MikeMcQuaid commented Jan 5, 2017

ilovezfs commented Jan 16, 2017 • edited Loading

PerBothner commented Jan 16, 2017

ilovezfs Jan 17, 2017

Choose a reason for hiding this comment

ilovezfs commented Jan 17, 2017

ilovezfs commented Jan 17, 2017

ilovezfs commented Jan 17, 2017

PerBothner commented Jan 17, 2017

ilovezfs commented Jan 17, 2017

PerBothner commented Jan 17, 2017

duncanmak commented Jan 17, 2017 via email

duncanmak commented Jan 17, 2017

ilovezfs commented Jan 17, 2017

duncanmak commented Jan 17, 2017

PerBothner commented Jan 17, 2017

ilovezfs commented Jan 17, 2017

PerBothner commented Jan 17, 2017

ilovezfs commented Jan 17, 2017

duncanmak commented Jan 17, 2017

ilovezfs commented Jan 17, 2017

PerBothner commented Jan 17, 2017

PerBothner commented Jan 22, 2017

ilovezfs commented Jan 22, 2017 • edited Loading

PerBothner commented Jan 22, 2017

duncanmak commented Dec 29, 2016 •

edited

Loading

ilovezfs commented Jan 16, 2017 •

edited

Loading

ilovezfs commented Jan 22, 2017 •

edited

Loading