Opened 12 years ago

Closed 11 years ago

#36135 closed enhancement (fixed)

gnuplot enhancement to support emacs-app

Reported by: jrhope Owned by: mojca (Mojca Miklavec)
Priority: Normal Milestone:
Component: ports Version: 2.1.2
Keywords: haspatch Cc:
Port: gnuplot

Description

The attached Portfile patch adds a new emacs_app variant which is like the emacs variant but uses Emacs.app. It almost suffices to just modify the existing "emacs" variant replacing

    depends_build-append    path:bin/emacs:emacs

with

    depends_build-append    path:share/emacs/site-lisp:emacs

but then it ends up using the (older) system /usr/bin/emacs for compilation (which works, but is Wrong).

The "configure.env-append" part sets $EMACS to the Emacs.app executable. We also have to update $PATH because of the gnuplot ./lisp/configure script which does this

EMACS=`basename $EMACS`

and then proceeds to call

$EMACS --version

which, depending upon file system case sensitivity, either erroneously finds /usr/bin/emacs (as /usr/bin/Emacs) or fails outright.

The "pre-configure" block edits ./lisp/configure to recognize "Emacs" as well as "emacs" as a valid executable name for GNU Emacs.

Attachments (3)

Portfile-gnuplot.diff (1.3 KB) - added by jrhope 12 years ago.
gnuplot-emacs_app.patch (3.9 KB) - added by mojca (Mojca Miklavec) 12 years ago.
Patch to allow adding support to Emacs.app
gnuplot-emacs_app.2.patch (894 bytes) - added by mojca (Mojca Miklavec) 11 years ago.
Allow using emacs-app[-devel] port to compile emacs support for gnuplot

Download all attachments as: .zip

Change History (21)

Changed 12 years ago by jrhope

Attachment: Portfile-gnuplot.diff added

comment:1 Changed 12 years ago by ryandesign (Ryan Carsten Schmidt)

Keywords: haspatch added
Owner: changed from macports-tickets@… to mojca.miklavec.lists@…

comment:2 Changed 12 years ago by mojca (Mojca Miklavec)

Thanks a lot for the patch and diagnosis. I would like to try to avoid setting PATH and fix the configure script instead, and above all, to get the change upstream (the version 4.6.1 is soon going to be released, but the team of developers is usually reluctant to do any last-minute changes).

https://sourceforge.net/tracker/?func=detail&aid=3567659&group_id=2055&atid=102055

The patch testing for emacs vs. Emacs in lisp/configure is not really needed since it just tries to patch emacs 20.2 and 20.3. My suggestion for the patch would be

--- lisp/configure.in.orig
+++ lisp/configure.in
@@ -13,8 +13,6 @@ AC_SET_MAKE
 AC_PROG_INSTALL
 AM_PATH_LISPDIR
 
-EMACS=`basename $EMACS`
-
 AC_CHECK_PROGS(DVIPS, dvips, no)
 AC_CHECK_PROGS(LATEX, latex latex2e, no)
 AC_PATH_PROG(MAKEINFO, makeinfo, no)
@@ -75,7 +73,7 @@ AC_MSG_RESULT([$emacs_version])
 vnum=`echo $emacs_version |awk -F\. '{print 100*$1+$2}'`
 
 AC_MSG_CHECKING([whether info-look.el is needed])
-if test "$EMACS" = emacs ; then
+if test `basename $EMACS` = emacs ; then
   if test "$vnum" -ge 2030 ; then
     info_look="not needed with emacs $emacs_version"
     INFO_LOOK_ELC=
@@ -83,7 +81,7 @@ if test "$EMACS" = emacs ; then
     info_look="using info-look.20.2.el"
     cp info-look.20.2.el info-look.el
   fi
-elif test "$EMACS" = xemacs ; then
+elif test `basename $EMACS` = xemacs ; then
   if test "$vnum" -ge 2000 ; then
     info_look="using info-look.20.3.el"
     cp info-look.20.3.el info-look.el

This doesn't check for Emacs, but if needed, the test could be extended to check for both. However, this would only patch emacs 20.2/20.3, so I wouldn't really try to fix that as it doesn't bring anything.

Just curious: do you actually use this emacs extensions?

comment:3 in reply to:  2 ; Changed 12 years ago by jrhope

Replying to mojca.miklavec.lists@…:

Thanks a lot for the patch and diagnosis. I would like to try to avoid setting PATH and fix the configure script instead, and above all, to get the change upstream (the version 4.6.1 is soon going to be released, but the team of developers is usually reluctant to do any last-minute changes).


Yeah, the exact nature of my changes didn't strike me as ideal, either, they're mostly due to the order in which I noticed problems. :-)

https://sourceforge.net/tracker/?func=detail&aid=3567659&group_id=2055&atid=102055

The patch testing for emacs vs. Emacs in lisp/configure is not really needed since it just tries to patch emacs 20.2 and 20.3. My suggestion for the patch would be

[snip]

This doesn't check for Emacs, but if needed, the test could be extended to check for both. However, this would only patch emacs 20.2/20.3, so I wouldn't really try to fix that as it doesn't bring anything.


I'd have to try again with your modified patch, but as I recall, without the check for Emacs, I got a build failure. Something like "no rule to build info-look.elc". I think the problem is the INFO_LOOK_ELC=info-look.elc immediately above the binary name check. For Emacs.app we need to make sure that INFO_LOOK_ELC is cleared, as it is in the "$vnum" -ge 2030 check. This is probably another bug with the configure script, actually. If I'm interpreting it correctly, this

else
  info_look="using none"
fi

should probably be

else
  info_look="using none"
  INFO_LOOK_ELC=
fi

instead. It seems like the contract is intended to be that if INFO_LOOK_ELC is not "", then info-look.el exists.

Just curious: do you actually use this emacs extensions?


To be honest, I didn't know it existed before discovering the gnuplot emacs variant yesterday. But I use Emacs every day and occasionally have to edit gnuplot scripts, so I intend to start using it!

comment:4 in reply to:  3 ; Changed 12 years ago by mojca (Mojca Miklavec)

Replying to jrh@…:

I'd have to try again with your modified patch, but as I recall, without the check for Emacs, I got a build failure. Something like "no rule to build info-look.elc". I think the problem is the INFO_LOOK_ELC=info-look.elc immediately above the binary name check. For Emacs.app we need to make sure that INFO_LOOK_ELC is cleared, as it is in the "$vnum" -ge 2030 check. This is probably another bug with the configure script, actually. If I'm interpreting it correctly, this

else
  info_look="using none"
fi

should probably be

else
  info_look="using none"
  INFO_LOOK_ELC=
fi

instead. It seems like the contract is intended to be that if INFO_LOOK_ELC is not "", then info-look.el exists.

Oh, thanks for this extra diagnosis. I forgot to run "make" when testing earlier (outside of macports, I didn't really want random files ending up inside Emacs.app :).



Just curious: do you actually use this emacs extensions?


To be honest, I didn't know it existed before discovering the gnuplot emacs variant yesterday. But I use Emacs every day and occasionally have to edit gnuplot scripts, so I intend to start using it!

I would be curious to get some feedback. I have no idea what those lisp scripts do.

Mojca

comment:5 in reply to:  4 Changed 12 years ago by jrhope

Replying to mojca.miklavec.lists@…:

Oh, thanks for this extra diagnosis. I forgot to run "make" when testing earlier (outside of macports, I didn't really want random files ending up inside Emacs.app :).

Well, the way I wrote the emacs_app variant, the files end up in ${prefix}/share/emacs/site-lisp, not inside Emacs.app. IIRC that used to be required, but at some point the emacs-app maintainers fixed its load-path to include both Emacs.app/Contents/Resources and ${prefix}/share/emacs. I think the theory was that eventually we wouldn't need separate "emacs" and "emacs_app" variants for other packages, but clearly there are still some issues when we need the executable itself and not just the location of the site-lisp directory.

Just curious: do you actually use this emacs extensions?


To be honest, I didn't know it existed before discovering the gnuplot emacs variant yesterday. But I use Emacs every day and occasionally have to edit gnuplot scripts, so I intend to start using it!

I would be curious to get some feedback. I have no idea what those lisp scripts do.

In my brief experimenting so far, I've found gnuplot-mode, which offers the typical syntax highlighting, and the handy gnuplot-send-buffer-to-gnuplot, which does what it says, launching gnuplot as an inferior process. There are a few other gnuplot-send-*-to-gnuplot functions as well. It appears to be the same Gnuplot Mode described here.

-Jamie

Changed 12 years ago by mojca (Mojca Miklavec)

Attachment: gnuplot-emacs_app.patch added

Patch to allow adding support to Emacs.app

comment:6 Changed 12 years ago by mojca (Mojca Miklavec)

I attached my suggestion for the patch based on your original patch. Can you please review it? I'm not sure about this line:

depends_build-append    path:share/emacs/site-lisp:emacs-app

Doesn't the normal emacs also satisfy this?

One part of the patch (lisp/configure[.in]) should be applied upstream anyway. The version 4.6.1 is soon going to be released. I would like to see if developers are willing to take the patch upstream before committing anything to MacPorts repository.

Now, given that ${prefix}/share/emacs/site-lisp works for both normal emacs and Emacs.app I would like to find a way to avoid two different variants if possible. This doesn't do anything but install a few files to that location. In case of user having just Emacs.app installed it just boils down to setting EMACS variable to do the job. The result is more or less the same, I believe.

Is there any Emacs PortGroup to take care of differences between emacs and emacs-app? ;)

comment:7 in reply to:  6 Changed 12 years ago by jrhope

Replying to mojca.miklavec.lists@…:

I attached my suggestion for the patch based on your original patch. Can you please review it? I'm not sure about this line:

depends_build-append    path:share/emacs/site-lisp:emacs-app

Doesn't the normal emacs also satisfy this?

The normal emacs does, too, yes. Within an "emacs_app" variant, it makes more sense to have that as it is, of course, but if we come to the point where the gnuplot port doesn't have to care about emacs vs. emacs-app, then it would perhaps be less intrusive to use

depends_build-append    path:share/emacs/site-lisp:emacs

(i.e. the "normal" == CLI-only emacs) in the one emacs variant.

One part of the patch (lisp/configure[.in]) should be applied upstream anyway. The version 4.6.1 is soon going to be released. I would like to see if developers are willing to take the patch upstream before committing anything to MacPorts repository.

Now, given that ${prefix}/share/emacs/site-lisp works for both normal emacs and Emacs.app I would like to find a way to avoid two different variants if possible.

That would be nice, yes.

This doesn't do anything but install a few files to that location. In case of user having just Emacs.app installed it just boils down to setting EMACS variable to do the job. The result is more or less the same, I believe.

Well, it installs *.el files to share/emacs/site-lisp, but it also uses EMACS to compile them to *.elc files:

$ port contents gnuplot
Port gnuplot contains:
  ...
  /opt/local/share/emacs/site-lisp/gnuplot-gui.el
  /opt/local/share/emacs/site-lisp/gnuplot-gui.elc
  /opt/local/share/emacs/site-lisp/gnuplot.el
  /opt/local/share/emacs/site-lisp/gnuplot.elc
  /opt/local/share/emacs/site-lisp/info-look.20.2.el
  /opt/local/share/emacs/site-lisp/info-look.20.3.el
  ...

At the very least I think we want to make sure to use an Emacs from MacPorts to compile those files, either ${prefix}/bin/emacs or Emacs.app -- in particular, not Apple's outdated /usr/bin/emacs.

Is there fancy Portfile syntax that can set EMACS to the first that it finds of ${prefix}/bin/emacs or ${applications_dir}/Emacs.app/Contents/MacOS/Emacs? If so, then the One True Variant need only look like

variant emacs description "An emacs mode for working with gnuplot." {
    depends_build-append    path:share/emacs/site-lisp:emacs
    configure.args-delete   --without-lisp-files
    configure.args-append   --with-lisp-files --with-lispdir=${prefix}/share/emacs/site-lisp
    configure.env-append    EMACS=<magic goes here>
}

Is there any Emacs PortGroup to take care of differences between emacs and emacs-app? ;)

That would indeed be nice. I'm sure there are a boatload of ports that would benefit from some systematic Emacs agnosticism.

comment:8 Changed 12 years ago by mojca (Mojca Miklavec)

A few hours ago a commit has been made upstream which removes those weird ancient info-look patches. That's good news as it means that patching won't be needed. The line EMACS=`basename $EMACS` is still there, but hopefully it was by accident and will be removed soon.

I suggest to postpone the patch to at least after 4.6.1 release (which will happen very soon anyway), but the open question about Emacs PortGroup still stands.

What is the difference between *.elc files compiled by different versions of emacs?

comment:9 in reply to:  8 Changed 12 years ago by jrhope

Replying to mojca.miklavec.lists@…:

A few hours ago a commit has been made upstream which removes those weird ancient info-look patches. That's good news as it means that patching won't be needed. The line EMACS=`basename $EMACS` is still there, but hopefully it was by accident and will be removed soon.

That's good news.

I suggest to postpone the patch to at least after 4.6.1 release (which will happen very soon anyway), but the open question about Emacs PortGroup still stands.

What is the difference between *.elc files compiled by different versions of emacs?

I don't know exactly. I haven't been able to find a clear record of how the Emacs Lisp bytecode spec has changed over time. (I don't consider wading through bzr commit histories to be a *clear* record.) I'm sure it has evolved some, as my recently-built gnuplot.elc has this comment near the top:

;;; Compiled by root@<host> on Tue Sep 18 19:46:04 2012
;;; from file <path>/gnuplot.el
;;; in Emacs version 24.2.1
;;; with all optimizations.

;;; This file uses dynamic docstrings, first added in Emacs 19.29.

;;; This file does not contain utf-8 non-ASCII characters,
;;; and so can be loaded in Emacs versions earlier than 23.

So at the very least there were some compatibility breaks at 19.29 and 23. The 23.1 release notes have this to say:

During byte-compilation, Emacs 23 uses `utf-8-emacs' to write files.
As a result, byte-compiled files containing non-ASCII characters can't
be read by earlier versions of Emacs.  Files compiled by Emacs 20, 21,
or 22 are loaded correctly as `emacs-mule' (whether or not they
contain multibyte characters).  This takes somewhat more time, so it
may be worth recompiling existing .elc files which don't need to be
shared with older Emacsen.

comment:10 Changed 12 years ago by mojca (Mojca Miklavec)

I have the port file for 4.6.1 ready.

Using Emacs.app doesn't require patching gnuplot sources any more. My main remaining question (apart from PortGroups etc) is: do emacs users really care that much if they have to install emacs in addition to emacs-app to get the gnuplot mode for emacs working in Emacs.app, so that it would justify an extra option (additional complexity) for the port?

comment:11 in reply to:  10 Changed 12 years ago by jrhope

Replying to mojca.miklavec.lists@…:

My main remaining question (apart from PortGroups etc) is: do emacs users really care that much if they have to install emacs in addition to emacs-app to get the gnuplot mode for emacs working in Emacs.app, so that it would justify an extra option (additional complexity) for the port?

Well, it does strike me as a bit annoying to have to build/install emacs twice (and then keep updating both). But I understand your perspective, too. It seems like if there's any complexity to be had, it should be handled by the emacs packages and not ones merely with emacs dependencies (gnuplot, php-mode.el, etc).

Does your 4.6.1 Portfile specify path:share/emacs/site-lisp:emacs instead of path:bin/emacs:emacs? With that in place, perhaps it would be enough if I set EMACS in my environment before calling port install gnuplot +emacs. (Does the build inherit the caller's environment?)

It seems like all this could go away if:

  1. The emacs-app port were listed as a conflict with the emacs port (the way xemacs is).
  2. It installed something like this script into ${prefix}/bin/emacs:
    #!/bin/sh
    exec /Applications/MacPorts/Emacs.app/Contents/MacOS/Emacs -nw "$@"
    

But maybe I'm missing something that's really obvious to the true Emacs weenies.

comment:12 Changed 11 years ago by mojca (Mojca Miklavec)

What about the following?

variant emacs description "An emacs mode for working with gnuplot" {
    if {[catch {registry_active emacs-app}] == 0} {
        depends_build-append port:emacs-app
        configure.env-append EMACS=${applications_dir}/Emacs.app/Contents/MacOS/Emacs
    } else {
        depends_build-append path:bin/emacs:emacs
    }
    configure.args-delete   --without-lisp-files
    configure.args-append   --with-lisp-files --with-lispdir=${prefix}/share/emacs/site-lisp
}

If that works for you - can you please ask on the macports-devel list if that's an acceptable solution?

My preference would still be something along the following lines though:

variant emacs description "An emacs mode for working with gnuplot" {
    PortGroup emacs 1.0
    configure.env-append    EMACS=${emacs.binary}
    configure.args-delete   --without-lisp-files
    configure.args-append   --with-lisp-files --with-lispdir=${emacs.lispfiles} # or simply current string
}

comment:13 Changed 11 years ago by mojca (Mojca Miklavec)

I'm sorry. I now see that I forgot about emacs-app-devel. Any idea how to incorporate that one? (A slightly different "if" is needed, I guess ...)

The following comes to my mind, but is probably suboptimal:

variant emacs description "An emacs mode for working with gnuplot" {
    if {[catch {registry_active emacs-app}] == 0} {
        depends_build-append port:emacs-app
        configure.env-append EMACS=${applications_dir}/Emacs.app/Contents/MacOS/Emacs
    } else if {[catch {registry_active emacs-app-devel}] == 0} {
        depends_build-append port:emacs-app-devel
        configure.env-append EMACS=${applications_dir}/Emacs.app/Contents/MacOS/Emacs
    } else {
        depends_build-append path:bin/emacs:emacs
    }
    configure.args-delete   --without-lisp-files
    configure.args-append   --with-lisp-files --with-lispdir=${prefix}/share/emacs/site-lisp
}

comment:14 Changed 11 years ago by jrhope

Eh, I have the impression that emacs-app-devel is obsolete anyway. It's a CVS snapshot from 2009, and the Emacs devs don't even use that repository anymore. IIRC, originally it was ahead of the point release used by emacs-app, providing early access to new functionality that eventually made it into Emacs 23. But now I'm not really sure what purpose it serves.

Regardless, since either way the binary ends up in the same place, that gave me the idea to see how "port provides $filename" works. What about something like:

variant emacs description "An emacs mode for working with gnuplot" {
    set app_binary ${applications_dir}/Emacs.app/Contents/MacOS/Emacs
    set app_provider [registry_file_registered $app_binary]
    if {[file exists $app_binary] && $app_provider != 0} {
        depends_build-append port:${app_provider}
        configure.env-append EMACS=${app_binary}
    } else {
        depends_build-append path:bin/emacs:emacs
    }
    configure.args-delete   --without-lisp-files
    configure.args-append   --with-lisp-files --with-lispdir=${prefix}/share/emacs/site-lisp
}

That appears to work for me with emacs-app:

$ port deps gnuplot +emacs
Full Name: gnuplot @4.6.0_2+aquaterm+emacs+luaterm+pangocairo+x11
Build Dependencies:   emacs-app
Library Dependencies: expat, fontconfig, gd2, jpeg, pdflib, libiconv, libpng,
                      ncurses, readline, zlib, aquaterm, xpm, lua, pango

If I edit $app_binary to point to a nonexistent file, then the build dependency changes to emacs.

comment:15 Changed 11 years ago by mojca (Mojca Miklavec)

The patch seems OK to me. It would still be nice to have a dedicated PortGroup for emacs, but until then the current form seems totally reasonable: it supports all flavours of emacs without adding any additional overhead to user by offering unnecessary extra options.

I don't have commit rights and I don't have enough experience to be able to tell if the code is consistent with MacPorts philosophy, so all that's needed is some experienced MacPorts developer's blessing and commit.

(See also #36502 & #33375.)

Changed 11 years ago by mojca (Mojca Miklavec)

Attachment: gnuplot-emacs_app.2.patch added

Allow using emacs-app[-devel] port to compile emacs support for gnuplot

comment:16 Changed 11 years ago by mojca (Mojca Miklavec)

Owner: changed from mojca.miklavec.lists@… to mojca@…

comment:17 Changed 11 years ago by mojca (Mojca Miklavec)

I just realized that I wrote the patch when I didn't have commit rights yet. Now I have commit rights and gnuplot 4.6.2 is expected soon. It would make sense to include that patch in 4.6.2, but I don't have enough experience to tell if gnuplot-emacs_app.2.patch​ is ok.

I realize that #36502 has been rejected, but I don't remember any more how exactly that ticket influenced this one. Maybe I only had problems if I made a symlink which is not the case in MacPorts.

comment:18 Changed 11 years ago by mojca (Mojca Miklavec)

Resolution: fixed
Status: newclosed

Committed in r104175.

Note: See TracTickets for help on using tickets.