PATCH for guile-readline/Makefile.am

PATCH for guile-readline/Makefile.am

[Richard Todd]

[-- Attachment #1.1.1: Type: text/plain, Size: 998 bytes --]

Please consider the following patch for inclusion in guile.

Problem: The guile-readline/Makefile unconditionally removes and
remakes a link from ice-9/readline.scm to ../readline.scm.  There are
two problems with this:

1) When you run, say, 'sudo make install', the link is remade as root!
2) When you run a VPATH build, ../readline.scm does not exist.

You'd only notice (2) if you wanted to test (ice-9 readline) prior to
installation.  You notice (1) quite often if you want to remove your
build tree, or even if you 'sudo make install' and then 'make' as a
normal user again.

The patch fixes both these issues.  It was a bit tricky, and I could
not find a single 'ln -s' line that worked for both in-tree builds and
VPATH builds (it's harder than it sounds!).  The patch just does an
'if' on $(top_srcdir) == $(top_builddir) and issues the correct
command.

If you don't like this method and can find a better way, please do so,
because we need a change to this file!

thanks,

Richard Todd

[-- Attachment #1.1.2: rdln.patch --]
[-- Type: text/plain, Size: 1015 bytes --]

--- orig/guile-core/guile-readline/ChangeLog
+++ mod/guile-core/guile-readline/ChangeLog
@@ -1,3 +1,9 @@
+2004-01-07  Richard Todd  <richardt@vzavenue.net>
+
+	* Makefile.am (ice-9/readline.scm): link to $srcdir, so VPATH builds 
+	work.  Make the link depend on the source file, so that it doesn't 
+	get re-made as a root link during 'sudo make install', for instance.
+
 2003-05-04  Marius Vollmer  <mvo@zagadka.de>
 
 	* configure.in: When checking whether readline clears SA_RESTART,


--- orig/guile-core/guile-readline/Makefile.am
+++ mod/guile-core/guile-readline/Makefile.am
@@ -57,11 +57,15 @@
 
 CLEANFILES = *.x
 
-ice-9/readline.scm:
+ice-9/readline.scm: readline.scm
 	-rm ice-9/readline.scm
 	-rmdir ice-9
 	mkdir ice-9
-	cd ice-9 && ln -s ../readline.scm
+	if [ "$(top_srcdir)" == "$(top_builddir)" ] ; then \
+             (cd ice-9 && $(LN_S) ../$<)  \
+        else \
+             (cd ice-9 && $(LN_S) $<) \
+        fi
 
 all-local: ice-9/readline.scm
 




[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: PATCH for guile-readline/Makefile.am

[Kevin Ryde]

Richard Todd <richardt@vzavenue.net> writes:
>
> Problem: The guile-readline/Makefile unconditionally removes and
> remakes a link from ice-9/readline.scm to ../readline.scm.

What's it for anyway?  Running ./pre-inst-guile?

> If you don't like this method and can find a better way, please do so,
> because we need a change to this file!

A radical approach would be to drop the guile-readline sub-configure
and put readline.scm under ice-9 in the first place.  (Could still
have a separate dlmodule if desired, or just add to libguile.)


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: PATCH for guile-readline/Makefile.am

[Marius Vollmer]

Kevin Ryde <user42@zip.com.au> writes:

> A radical approach would be to drop the guile-readline sub-configure
> and put readline.scm under ice-9 in the first place.  (Could still
> have a separate dlmodule if desired, or just add to libguile.)

The main problem with this would be that readline is GPL while guile
is LGPL.  It seems best to keep guile-readline and the rest of guile
clearly separate.

-- 
GPG: D5D4E405 - 2F9B BCCC 8527 692A 04E3  331E FAF8 226A D5D4 E405


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: PATCH for guile-readline/Makefile.am

[Marius Vollmer]

Richard Todd <richardt@vzavenue.net> writes:

> If you don't like this method and can find a better way, please do so,
> because we need a change to this file!

What about moving readline.scm to ice-9/readline.scm in the source
tree?  I.e., we place readline.scm directly in ice-9 and remove it
from guile-readline once and for all.  Then the symlink is not needed.

-- 
GPG: D5D4E405 - 2F9B BCCC 8527 692A 04E3  331E FAF8 226A D5D4 E405


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: PATCH for guile-readline/Makefile.am

[Rob Browning]

Marius Vollmer <mvo@zagadka.de> writes:

> What about moving readline.scm to ice-9/readline.scm in the source
> tree?  I.e., we place readline.scm directly in ice-9 and remove it
> from guile-readline once and for all.  Then the symlink is not
> needed.

That seems OK.

Also, if we really did want to keep everything in guile-readline/,
then I suspect we could just move readline.scm to a new subdir,
i.e. guile-readline/ice-9/readline.scm, and then augment the
pre-inst-guile GUILE_LOAD_PATH as appropriate.  I've done something
similar for other projects that were basically supposed to be
"unioned" with existing GUILE_LOAD_PATH dirs.

-- 
Rob Browning
rlb @defaultvalue.org and @debian.org; previously @cs.utexas.edu
GPG starting 2002-11-03 = 14DD 432F AE39 534D B592  F9A0 25C8 D377 8C7E 73A4


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: PATCH for guile-readline/Makefile.am

[Marius Vollmer]

Rob Browning <rlb@defaultvalue.org> writes:

> Marius Vollmer <mvo@zagadka.de> writes:
>
>> What about moving readline.scm to ice-9/readline.scm in the source
>> tree?  I.e., we place readline.scm directly in ice-9 and remove it
>> from guile-readline once and for all.  Then the symlink is not
>> needed.
>
> That seems OK.

I think I was unclear: what I have in mind is to create a new
directory "guile-core/guile-readline/ice-9" and put readline.scm
there.  It shouldn't be put into guile-core/ice-9/.

-- 
GPG: D5D4E405 - 2F9B BCCC 8527 692A 04E3  331E FAF8 226A D5D4 E405


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: PATCH for guile-readline/Makefile.am

[Rob Browning]

Marius Vollmer <mvo@zagadka.de> writes:

> I think I was unclear: what I have in mind is to create a new
> directory "guile-core/guile-readline/ice-9" and put readline.scm
> there.  It shouldn't be put into guile-core/ice-9/.

Ahh, then we were probably thinking about the same thing.

-- 
Rob Browning
rlb @defaultvalue.org and @debian.org; previously @cs.utexas.edu
GPG starting 2002-11-03 = 14DD 432F AE39 534D B592  F9A0 25C8 D377 8C7E 73A4


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: PATCH for guile-readline/Makefile.am

[Kevin Ryde]

Richard Todd <richardt@vzavenue.net> writes:
>
> If you don't like this method and can find a better way, please do so,

How about

	ln -s $(srcdir)/readline.scm ice-9/readline.scm

$(srcdir) is right for both in-place and separate builds.


> +             (cd ice-9 && $(LN_S) ../$<)  \
> +        else \
> +             (cd ice-9 && $(LN_S) $<) \

Alas, $< is not portable in ordinary rules (see the autoconf manual).


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: PATCH for guile-readline/Makefile.am

[Richard Todd]

Kevin Ryde wrote:
> Richard Todd <richardt@vzavenue.net> writes:
> 
>>If you don't like this method and can find a better way, please do so,
> 
> 
> How about
> 
> 	ln -s $(srcdir)/readline.scm ice-9/readline.scm
> 
> $(srcdir) is right for both in-place and separate builds.

Yeah, I tried that, but for an in-tree build, $(srcdir) == ".".  On the 
two systems I tried it on, that command leads to:
   readline.scm -> ./readline.scm  ( a link ot itself!)

That's why I said it's trickier than it sounds! :)


> 
>>+             (cd ice-9 && $(LN_S) ../$<)  \
>>+        else \
>>+             (cd ice-9 && $(LN_S) $<) \
> 
> 
> Alas, $< is not portable in ordinary rules (see the autoconf manual).
> 

Thanks for pointing this out.  I'll remember that.

For what it's worth, I liked merging readline into guile-core the best, 
but if the license issue is too much to overcome, then I don't see much 
difference in quality between the 'link' approaches and the 'new 
directory' approach.  If there were going to be lots more ice-9/xx.scm 
files for readline, then the new directory would definitlely by cleaner.

thanks,
Richard




_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: PATCH for guile-readline/Makefile.am

[Rob Browning]

Richard Todd <richardt@vzavenue.net> writes:

> For what it's worth, I liked merging readline into guile-core the
> best, but if the license issue is too much to overcome, then I don't
> see much difference in quality between the 'link' approaches and the
> 'new directory' approach.  If there were going to be lots more
> ice-9/xx.scm files for readline, then the new directory would
> definitlely by cleaner.

I'd favor the guile-readline/ice-9/readline.scm approach as well.
Thought about trying it out yesterday evening, but ran out of time...

-- 
Rob Browning
rlb @defaultvalue.org and @debian.org; previously @cs.utexas.edu
GPG starting 2002-11-03 = 14DD 432F AE39 534D B592  F9A0 25C8 D377 8C7E 73A4


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: PATCH for guile-readline/Makefile.am

[Rob Browning]

Marius Vollmer <mvo@zagadka.de> writes:

> I think I was unclear: what I have in mind is to create a new
> directory "guile-core/guile-readline/ice-9" and put readline.scm
> there.  It shouldn't be put into guile-core/ice-9/.

OK.  I got this working in my tree last night, and I also updated
our guile-readline configure.in to use the non-deprecated AC_INIT,
automake, etc. syntax.

  AC_INIT(guile-readline, 1.7.0)
  AC_CONFIG_SRCDIR(readline.c)
  AM_INIT_AUTOMAKE([foreign no-define])

Though that raises an issue that I haven't figured out how we want to
handle yet.  As above, autoconf now wants the package name and version
to be literals.  Hence our trick of using GUILE-VERSION and $PACKAGE
and $VERSION won't work in this new arrangement.

One possibility would be to make configure.in the canonical source for
versioning information and just write GUILE-VERSION from there, and I
guess guile-readlne would do the same for LIBGUILEREADLINE-VERSION.
Although this would mean that we'd have to update the version in
configure.in and guile-readline/configure.in independently, by-hand.

That doesn't seem like a showstopper to me, though.  I can just add it
to the release procedure.

-- 
Rob Browning
rlb @defaultvalue.org and @debian.org; previously @cs.utexas.edu
GPG starting 2002-11-03 = 14DD 432F AE39 534D B592  F9A0 25C8 D377 8C7E 73A4


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: PATCH for guile-readline/Makefile.am

[Kevin Ryde]

Rob Browning <rlb@defaultvalue.org> writes:
>
> Though that raises an issue that I haven't figured out how we want to
> handle yet.  As above, autoconf now wants the package name and version
> to be literals.  Hence our trick of using GUILE-VERSION and $PACKAGE
> and $VERSION won't work in this new arrangement.

You can generate a version argument using m4 esyscmd, patsubst,
whatever, if you like.  Autoconf never knows it was an m4 expansion
and not a literal. :)

We get some joy with this in gmp, see GMP_VERSION in acinclude.m4
grepping gmp-h.in.  (All copyright FSF of course, if you want to cut
and paste.)


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: PATCH for guile-readline/Makefile.am

[Rob Browning]

Kevin Ryde <user42@zip.com.au> writes:

> You can generate a version argument using m4 esyscmd, patsubst,
> whatever, if you like.  Autoconf never knows it was an m4 expansion
> and not a literal. :)

Ahhh.  Great.  I didn't know about that m4 command -- looks like it
should work fine.

Thanks
-- 
Rob Browning
rlb @defaultvalue.org and @debian.org; previously @cs.utexas.edu
GPG starting 2002-11-03 = 14DD 432F AE39 534D B592  F9A0 25C8 D377 8C7E 73A4


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: PATCH for guile-readline/Makefile.am

[Rob Browning]

Rob Browning <rlb@defaultvalue.org> writes:

> Ahhh.  Great.  I didn't know about that m4 command -- looks like it
> should work fine.

The code below seems to work.  Although now the literal version string
is embedded in guile-readline/configure at autoconf time.  I think in
the old code, it was variable, and so changing GUILE-VERSION would
take effect after the next configure run.  With the new code, when you
change ../GUILE-VERSION you'll need to run autoconf for everything to
propagate.

  AC_INIT(guile-readline,
          m4_esyscmd(. ../GUILE-VERSION && echo -n ${GUILE_VERSION}))
  AC_CONFIG_SRCDIR(readline.c)
  AM_INIT_AUTOMAKE([foreign no-define])

  . $srcdir/../GUILE-VERSION

  AC_PROG_INSTALL
  ...

If I get a chance, I'll also check and see how gmp does things.

In addition, with readline.scm in guile-readline/ice-9 we have to
play around a bit to define the right ice-9 location in
guile-readline/ice-9/Makefile.am:

  guile_pdd = $(patsubst %/guile-readline,%/guile,$(pkgdatadir))
  ice9dir = $(guile_pdd)/$(GUILE_EFFECTIVE_VERSION)/ice-9
  ice9_DATA = readline.scm

-- 
Rob Browning
rlb @defaultvalue.org and @debian.org; previously @cs.utexas.edu
GPG starting 2002-11-03 = 14DD 432F AE39 534D B592  F9A0 25C8 D377 8C7E 73A4


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: PATCH for guile-readline/Makefile.am

[Kevin Ryde]

Rob Browning <rlb@defaultvalue.org> writes:
>
> If I get a chance, I'll also check and see how gmp does things.

Yep, a re-autoconf is needed to pick up a new version number.  Myself
I haven't found that a problem, one normally does at least one fresh
autoconf run between releases :-).


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel