1.8 ‘send’ bug + re-engagement

1.8 ‘send’ bug + re-engagement

[Thien-Thi Nguyen]

[-- Attachment #1: Type: text/plain, Size: 3016 bytes --]

Looking to move WIKID[0] out of the Guile 1.4.x ghetto (which is pretty
cozy, i must say), i ran into a Guile 1.8 problem.  Apparently, ‘send’
gratuitously demands its MESSAGE arg (a string) be writable.  This loses
if, e.g., MESSAGE is the result of ‘symbol->string’.  Here is the fix:

 libguile/socket.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/libguile/socket.c b/libguile/socket.c
index ddc9b0a..964f72c 100644
--- a/libguile/socket.c
+++ b/libguile/socket.c
@@ -1482,7 +1482,7 @@ SCM_DEFINE (scm_send, "send", 2, 1, 0,
   fd = SCM_FPORT_FDES (sock);
 
   len = scm_i_string_length (message);
-  src = scm_i_string_writable_chars (message);
+  src = scm_i_string_chars (message);
   SCM_SYSCALL (rv = send (fd, src, len, flg));
   scm_i_string_stop_writing ();
 
Since i've been graciously (re-)granted write privs, i'd like to apply
the fix myself (in the savannah repo), onto ‘branch_release-1-8’.
However, in a fit of thoroughness, i couldn't resist doing some other
changes locally:

$ glog branch_release-1-8.. --reverse
ee70ab6 2012-08-22 Rename configure.in to configure.ac, twice
04d7f4a 2012-08-22 configure, int: Add abstraction: CONFIG_SCRIPT
4bfdc6b 2012-08-23 Delete EOL whitespace; nfc
4099d14 2012-08-23 configure: Dose configure.ac w/ "proper" m4-quoting
037b7f6 2012-08-22 configure, int: Remove EOF "Local Variables" block; nfc
b3ee840 2012-08-23 configure, int: Add more 'AC_LANG_PROGRAM' calls
ccb98a3 2012-08-23 Delete EOL whitespace; nfc
60a29ff 2012-08-23 libguile: Fix bug: Don't expect 'send' string to be writable
d70f9c8 2012-08-23 Update years in copyright notice; nfc

Note that the fix is the penultimate change (60a29ff).  How about i push
this to ‘ttn-back-in-the-saddle’ for review?  The branch name reflects a
desire to start doing the 1.8 curation that Someone should be doing.
The changes are done in a hybrid ttn-gnu maintainance style.

I imagine if this particular fix goes smoothly, i will be motivated to
continue w/ this kind of maintenance work, where the focus is on
continuity and stability (perhaps likewise showing 1.6 and 1.4 some
love, as well).

I remain confused (and slightly put off) by the "we're worried you'll be
cavalier" warnings mixed with what i perceive as cavalier design
decisions and code changes by the warners, but anyway wish to reassure
everyone that i will limit myself to cleanups (such as those above
listed), bug fixes and doc improvements, all of which are far from
(deep) design.

What do the maintainers think?

[0] http://www.gnuvola.org/software/wikid/
-- 
Thien-Thi Nguyen ..................................... GPG key: 4C807502
.                  NB: ttn at glug dot org is not me                   .
.                 (and has not been since 2007 or so)                  .
.                        ACCEPT NO SUBSTITUTES                         .
........... please send technical questions to mailing lists ...........

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: 1.8 ‘send’ bug + re-engagement

[Ludovic Courtès]

Hi Thien-Thi!

Thien-Thi Nguyen <ttn@gnuvola.org> skribis:

> Looking to move WIKID[0] out of the Guile 1.4.x ghetto (which is pretty
> cozy, i must say),

(Speaking of which, do let me know when rpx has left the ghetto, too. :-))

> i ran into a Guile 1.8 problem.  Apparently, ‘send’ gratuitously
> demands its MESSAGE arg (a string) be writable.  This loses if, e.g.,
> MESSAGE is the result of ‘symbol->string’.  Here is the fix:
>
>  libguile/socket.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

Sounds great.  There are ‘sendto’ unit tests, but no ‘send’ tests.  As a
bonus, if you feel like adding a couple of these, you’re welcome.  :-)

> Since i've been graciously (re-)granted write privs, i'd like to apply
> the fix myself (in the savannah repo), onto ‘branch_release-1-8’.

Yes, please do!  Can you apply it to stable-2.0 as well?

> $ glog branch_release-1-8.. --reverse

“glog”?  :-)

> ee70ab6 2012-08-22 Rename configure.in to configure.ac, twice
> 04d7f4a 2012-08-22 configure, int: Add abstraction: CONFIG_SCRIPT
> 4bfdc6b 2012-08-23 Delete EOL whitespace; nfc
> 4099d14 2012-08-23 configure: Dose configure.ac w/ "proper" m4-quoting
> 037b7f6 2012-08-22 configure, int: Remove EOF "Local Variables" block; nfc
> b3ee840 2012-08-23 configure, int: Add more 'AC_LANG_PROGRAM' calls
> ccb98a3 2012-08-23 Delete EOL whitespace; nfc
> 60a29ff 2012-08-23 libguile: Fix bug: Don't expect 'send' string to be writable
> d70f9c8 2012-08-23 Update years in copyright notice; nfc
>
> Note that the fix is the penultimate change (60a29ff).  How about i push
> this to ‘ttn-back-in-the-saddle’ for review?

Makes sense, yes.

> The branch name reflects a desire to start doing the 1.8 curation that
> Someone should be doing.

Good idea.  I think I once said we SHOULD release 1.8.9 RSN.  Would you
like to take care of that?

> The changes are done in a hybrid ttn-gnu maintainance style.

I’m not sure what you mean here.  :-)  I prefer a impersonal GNUish
style.

> I imagine if this particular fix goes smoothly, i will be motivated to
> continue w/ this kind of maintenance work, where the focus is on
> continuity and stability (perhaps likewise showing 1.6 and 1.4 some
> love, as well).

Hmm, I’d find it more important to help fix any issues that prevent
current 1.8 users from switching to 2.0, FWIW.

> I remain confused (and slightly put off) by the "we're worried you'll be
> cavalier" warnings mixed with what i perceive as cavalier design
> decisions and code changes by the warners, but anyway wish to reassure
> everyone that i will limit myself to cleanups (such as those above
> listed), bug fixes and doc improvements, all of which are far from
> (deep) design.
>
> What do the maintainers think?

I think this is excellent news, thank you!

Ludo’.

Re: 1.8 ‘send’ bug + re-engagement

[Mike Gran]

> From: Ludovic Courtès <ludo@gnu.org>


>>  I imagine if this particular fix goes smoothly, i will be motivated to
>>  continue w/ this kind of maintenance work, where the focus is on
>>  continuity and stability (perhaps likewise showing 1.6 and 1.4 some
>>  love, as well).
> 
> Hmm, I’d find it more important to help fix any issues that prevent
> current 1.8 users from switching to 2.0, FWIW.

The big issues with moving to 2.0 that I've seen expressed are
non-portability, number of dependencies, API churn, and general dislike of
pkg-config.  Since some of these are religious issues, I think 1.8
is going to be around for quite a while, since it takes a while for
people to lose their faith.  Another 1.8 release might be warranted.

Still no packaged 2.x in Fedora, for example.

-Mike

Re: 1.8 ‘send’ bug + re-engagement

[Ludovic Courtès]

Hi,

Mike Gran <spk121@yahoo.com> skribis:

> The big issues with moving to 2.0 that I've seen expressed are
> non-portability,

I don’t think 2.0 is less portable than 1.8; there’s room for
improvement (MinGW, pthread support), but it’s not a disaster either.

> number of dependencies,

There’s are two on-going things that should help this:
gl_LIBUNISTRING_OPTIONAL, and mini-gmp.

> API churn,

Some of the API has been deprecated, but usually in a
backwards-compatible way.  So we’d need to see concrete issues (we’ve
seen some already, and most or all were addressed, IIRC.)

> and general dislike of pkg-config.

Well, one can still do without it.

Anyway, people should evaluate the cost/benefit ratio.  For me, the
benefits clearly outweigh the cost, but some people seem to be on the
half-empty-glass side.  ;-)

>   Since some of these are religious issues, I think 1.8 is going to be
> around for quite a while, since it takes a while for people to lose
> their faith.  Another 1.8 release might be warranted.

Yeah.

Thanks for your feedback,
Ludo’.

Re: 1.8 ‘send’ bug + re-engagement

[Thien-Thi Nguyen]

[-- Attachment #1: Type: text/plain, Size: 3849 bytes --]

() ludo@gnu.org (Ludovic Courtès)
() Sat, 25 Aug 2012 16:24:26 +0200

   > Looking to move WIKID[0] out of the Guile 1.4.x ghetto (which is
   > pretty cozy, i must say),

   (Speaking of which, do let me know when rpx has left the ghetto,
   too. :-))

OK, will do.  If the world doesn't end, a couple more years, maybe...

   > ‘send’ gratuitously demands its MESSAGE arg (a string) be writable.
   > [...]  Here is the fix:

   Sounds great.  There are ‘sendto’ unit tests, but no ‘send’ tests.
   As a bonus, if you feel like adding a couple of these, you’re
   welcome.  :-)

I plan to add some tests, yes.

   > i'd like to apply the fix myself (in the savannah repo), onto
   > ‘branch_release-1-8’.

   Yes, please do!  Can you apply it to stable-2.0 as well?

Sorry, no; i lack sufficient bandwidth.

   > $ glog branch_release-1-8.. --reverse

   “glog”?  :-)

A small script, here in its entirety:

#!/bin/sh
exec git log --date=short --format='%h %ad %s' "$@"

   > [...]
   > 60a29ff [...] libguile: Fix bug: Don't expect 'send' [...]
   > d70f9c8 [...]
   >
   > Note that the fix is the penultimate change (60a29ff).  How about i
   > push this to ‘ttn-back-in-the-saddle’ for review?

   Makes sense, yes.

OK, just pushed.  I await your review.

   I think I once said we SHOULD release 1.8.9 RSN.  Would you
   like to take care of that?

Yes.  For 1.8.9, i plan to:
- audit libguile for this (writable/read-only string) class of bug
- add tests
- back/forward-port tests
- back/forward-port docs

Further 1.8 releases will be similar, the goal being to transition from
point-fixes to systemic improvements.  On the side, i plan to improve
the build environment (makefiles / configure.ac / doc-mangling).

   > The changes are done in a hybrid ttn-gnu maintainance style.

   I’m not sure what you mean here.  :-)

For example, the TITLE in a ChangeLog entry (in git jargon, the commit's
"subject line") has no prescribed format.  Guile HACKING refers to this
only as "1-line description of change" (pretty loose), whereas ttn-style
is to use a certain format, the germane part of which is the trailing
"; nfc" to indicate that the change does not warrant a ChangeLog entry.
(NFC stands for non-functionality -- i.e., cosmetic -- change.)

Anyway, i see 1.8.8 tarball does not even include a proper ChangeLog, so
that's an opportunity to backport the ‘gen-ChangeLog’ stuff...

Other ttn-style stuff: no EOL whitespace, use SPC for indentation,
strict EOF "ends here" comment maintenance.  Where a file is not stylish
enough :-D, the MO is to do these changes opportunistically (preceding
the actual required change) in their own "nfc" commits.  This MO is
evidenced in the pushed branch.

   > I imagine if this particular fix goes smoothly, i will be motivated
   > to continue w/ this kind of maintenance work, where the focus is on
   > continuity and stability (perhaps likewise showing 1.6 and 1.4 some
   > love, as well).

   Hmm, I’d find it more important to help fix any issues that prevent
   current 1.8 users from switching to 2.0, FWIW.

Well, that's why i'm here -- to do the non-important-for-ludo stuff
(that happens to be important for others).  I think the more you move
your mind away from "switching" (XOR) and towards "stepping" (XOR, brief
(or not) IOR, XOR), the more you will see value in what i do.

-- 
Thien-Thi Nguyen ..................................... GPG key: 4C807502
.                  NB: ttn at glug dot org is not me                   .
.                 (and has not been since 2007 or so)                  .
.                        ACCEPT NO SUBSTITUTES                         .
........... please send technical questions to mailing lists ...........


[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: 1.8 ‘send’ bug + re-engagement

[Ludovic Courtès]

Hi!

Thanks for the update!

Thien-Thi Nguyen <ttn@gnuvola.org> skribis:

>    > i'd like to apply the fix myself (in the savannah repo), onto
>    > ‘branch_release-1-8’.
>
>    Yes, please do!  Can you apply it to stable-2.0 as well?
>
> Sorry, no; i lack sufficient bandwidth.

OK, I’ll do it if nobody beats me at it.

>    > [...]
>    > 60a29ff [...] libguile: Fix bug: Don't expect 'send' [...]
>    > d70f9c8 [...]
>    >
>    > Note that the fix is the penultimate change (60a29ff).  How about i
>    > push this to ‘ttn-back-in-the-saddle’ for review?
>
>    Makes sense, yes.
>
> OK, just pushed.  I await your review.

  commit ee70ab60d0cf2f995b8ec513024a309610f3cb78
  Author: Thien-Thi Nguyen <ttn@gnuvola.org>
  Date:   Wed Aug 22 14:51:33 2012 +0200

      Rename configure.in to configure.ac, twice

OK.

  commit 04d7f4a80fb84a7333bb58c831591a59f42ab59b
  Author: Thien-Thi Nguyen <ttn@gnuvola.org>
  Date:   Wed Aug 22 15:01:34 2012 +0200

      configure, int: Add abstraction: CONFIG_SCRIPT

OK, in principle.  Perhaps CONFIG_SCRIPT could take a list of files and
m4_foreach on them?

However, this kind of patches *must* be applied to stable-2.0 as well,
to avoid gratuitous divergence.

I’m not sure I can reasonably commit to keeping track of each single
commit that needs backporting, so I’d be happy if you would volunteer,
at least for this kind of simple patch.  WDYT?

  commit 4bfdc6b88ea7a794ee7dac87e4f9fe71c3c064b1
  Author: Thien-Thi Nguyen <ttn@gnuvola.org>
  Date:   Thu Aug 23 17:26:10 2012 +0200

      Delete EOL whitespace; nfc

Hmm, that reminds me of a discussion we had before.  That’s fine only as
long as it doesn’t prevent merges.  How would Git handle that?

  commit 4099d14b93fc60ff6c8bd0d3f409bf30772d939f
  Author: Thien-Thi Nguyen <ttn@gnuvola.org>
  Date:   Thu Aug 23 17:43:47 2012 +0200

      configure: Dose configure.ac w/ "proper" m4-quoting

OK, but must also be adjusted & applied to other branches.

  commit 037b7f6a16d06073ae2b1f5cd3ad2588b2685455
  Author: Thien-Thi Nguyen <ttn@gnuvola.org>
  Date:   Wed Aug 22 18:16:23 2012 +0200

      configure, int: Remove EOF "Local Variables" block; nfc

OK.

  commit b3ee8403f89b21b1f9bda7fa93ef43861726fe5e
  Author: Thien-Thi Nguyen <ttn@gnuvola.org>
  Date:   Thu Aug 23 17:15:32 2012 +0200

      configure, int: Add more 'AC_LANG_PROGRAM' calls

OK, needs porting to other branches.

  commit ccb98a34d4871f356e7673e86be63b507e91fa8d
  Author: Thien-Thi Nguyen <ttn@gnuvola.org>
  Date:   Thu Aug 23 18:09:17 2012 +0200

      Delete EOL whitespace; nfc

As above.

  commit 60a29ffea0b596ae19b3a5bafbcc47bfec50b3b9
  Author: Thien-Thi Nguyen <ttn@gnuvola.org>
  Date:   Thu Aug 23 18:17:08 2012 +0200

      libguile: Fix bug: Don't expect 'send' string to be writable

OK.

  commit d70f9c8869aa46898e52124c4e42d0d0059210d7
  Author: Thien-Thi Nguyen <ttn@gnuvola.org>
  Date:   Thu Aug 23 18:17:45 2012 +0200

      Update years in copyright notice; nfc

Currently we enumerate years.  To use ranges, I think we have to update
our README to allow that.  In ‘stable-2.0’, README says:

  See the LICENSE file for the specific terms that apply to Guile.  Note
  that for any copyright year range specified as YYYY-ZZZZ in this
  package, the range specifies every single year in that closed interval.

Can you apply that as well?

In general, I find that copyright year updates, whitespace cleanups, M4
quoting, and the likes may be good ideas in principle, but they also
cause a lot of “noise” on the commit list.  So as a rule of thumb, I’d
try to keep to signal-to-noise ratio good enough.

> Yes.  For 1.8.9, i plan to:
> - audit libguile for this (writable/read-only string) class of bug
> - add tests
> - back/forward-port tests
> - back/forward-port docs

Great.

> Further 1.8 releases will be similar, the goal being to transition from
> point-fixes to systemic improvements.  On the side, i plan to improve
> the build environment (makefiles / configure.ac / doc-mangling).

OK.

> For example, the TITLE in a ChangeLog entry (in git jargon, the commit's
> "subject line") has no prescribed format.  Guile HACKING refers to this
> only as "1-line description of change" (pretty loose), whereas ttn-style
> is to use a certain format, the germane part of which is the trailing
> "; nfc" to indicate that the change does not warrant a ChangeLog entry.
> (NFC stands for non-functionality -- i.e., cosmetic -- change.)

I’m happy with “; nfc”.

> Anyway, i see 1.8.8 tarball does not even include a proper ChangeLog, so
> that's an opportunity to backport the ‘gen-ChangeLog’ stuff...

Indeed.

> Other ttn-style stuff: no EOL whitespace, use SPC for indentation,
> strict EOF "ends here" comment maintenance.

There’s a couple of .dir-locals.el in stable-2.0, to use space for
indentation in Scheme code.  For the rest I don’t care.

I have ‘show-trailing-whitespace’ set, FWIW, but again, fewer
whitespace-cleanup changes is better, as far as I’m concerned.

>    > I imagine if this particular fix goes smoothly, i will be motivated
>    > to continue w/ this kind of maintenance work, where the focus is on
>    > continuity and stability (perhaps likewise showing 1.6 and 1.4 some
>    > love, as well).
>
>    Hmm, I’d find it more important to help fix any issues that prevent
>    current 1.8 users from switching to 2.0, FWIW.
>
> Well, that's why i'm here -- to do the non-important-for-ludo stuff
> (that happens to be important for others).  I think the more you move
> your mind away from "switching" (XOR) and towards "stepping" (XOR, brief
> (or not) IOR, XOR), the more you will see value in what i do.

As I think this very review shows, the problem is not much whether I
care about it (and I actually do), but whether we can handle the
bandwidth increase without compromising on the project’s forward-looking
activity.

IOW: I find it important to maintain 1.8, but not at the cost of
reducing hack power on 2.0/2.1.

Thanks,
Ludo’.