request reversion regarding scm_i_* removal

request reversion regarding scm_i_* removal

[Andy Wingo]

Hi,

The removal of the scm_i_* functions is an ABI break in the stable 1.8
series. It should be reverted. (It's a great fix for master though.)

It breaks apps compiled with older versions of guile 1.8. Guile-gnome is
not affected too much -- a couple instance of scm_i_symbol_length (not
something that can currently be dealt with with version #ifdeffing, as
the version is still 1.8.5), but more seriously the port table, with the
scm_i_port_table_mutex. (The port API sucks, I know.)

Thanks,

Andy
-- 
http://wingolog.org/

Re: request reversion regarding scm_i_* removal

[Ludovic Courtès]

Hi,

Andy Wingo <wingo@pobox.com> writes:

> The removal of the scm_i_* functions is an ABI break in the stable 1.8
> series. It should be reverted. (It's a great fix for master though.)

The "i" always stood for "internal", but let's see what can be done...
;-)

Assuming the above, one could say that it's not an ABI break, since no
public function was added, modified nor removed.

Now, since the public API did lack a few necessary functions, we could
probably make a few exceptions, leaving crucial `_i_' functions public.
I suggested such exceptions and asked for more back then:

  http://thread.gmane.org/gmane.lisp.guile.devel/7246
  http://thread.gmane.org/gmane.lisp.guile.devel/7285

> It breaks apps compiled with older versions of guile 1.8. Guile-gnome is
> not affected too much -- a couple instance of scm_i_symbol_length (not
> something that can currently be dealt with with version #ifdeffing, as
> the version is still 1.8.5),

I don't mind leaving `scm_i_symbol_length ()' public for the sake of ABI
compatibility, just like `scm_i_string_chars ()' and a few others.

Eventually, application code should have a configure check, allowing it
to use either `_i_' or `_c_'.

> but more seriously the port table, with the
> scm_i_port_table_mutex. (The port API sucks, I know.)

What do you mean?  What code outside `libguile' uses
`scm_i_port_table_mutex'?

Thanks,
Ludovic.

Re: request reversion regarding scm_i_* removal

[Andy Wingo]

Good day!

Still on holiday, but the train provides a lovely hacktime.

On Mon 11 Aug 2008 01:10, ludo@gnu.org (Ludovic Courtès) writes:

> Andy Wingo <wingo@pobox.com> writes:
>
>> The removal of the scm_i_* functions is an ABI break in the stable 1.8
>> series. It should be reverted. (It's a great fix for master though.)
>
> The "i" always stood for "internal", but let's see what can be done...
> ;-)

Yeah, I know. Bad me. (And others, of course.) But I don't think "We
told you so" is a good reason to change within a stable series -- and
in this particular case I don't think its cost is worth its benefits.

I do think it's a super fix for master, though! :)

> I don't mind leaving `scm_i_symbol_length ()' public for the sake of ABI
> compatibility, just like `scm_i_string_chars ()' and a few others.
>
> Eventually, application code should have a configure check, allowing it
> to use either `_i_' or `_c_'.

Yeah, I agree.

>> but more seriously the port table, with the
>> scm_i_port_table_mutex. (The port API sucks, I know.)
>
> What do you mean?  What code outside `libguile' uses
> `scm_i_port_table_mutex'?

Guile-gnome does, when registering a new port type, for gnome-vfs. The
port API isn't threadsafe, and also sucks as I mentioned ;) From
gnome-vfs/gnome/gw/gnome-vfs-port.c:

#define LOCK scm_i_pthread_mutex_lock (&scm_i_port_table_mutex)
#define UNLOCK scm_i_pthread_mutex_unlock (&scm_i_port_table_mutex)

    SCM 
    scm_gnome_vfs_handle_to_port (GnomeVFSHandle *handle, GnomeVFSOpenMode mode,
                                  const gchar* uri)
    #define FUNC_NAME "scm_gnome_vfs_handle_to_port"
    {
        long mode_bits = vfs_mode_bits (mode);
        SCM port;
        scm_t_port *pt;

        LOCK;

        port = scm_new_port_table_entry (scm_tc16_vport);
        SCM_SET_CELL_TYPE(port, scm_tc16_vport | mode_bits);
        pt = SCM_PTAB_ENTRY(port);
        pt->rw_random = mode & GNOME_VFS_OPEN_RANDOM;
        SCM_SETSTREAM (port, handle);
        if (mode_bits & SCM_BUF0)
            scm_vport_buffer_add (port, 0, 0);
        else
            scm_vport_buffer_add (port, -1, -1);
        SCM_SET_FILENAME (port, scm_makfrom0str (uri));

        UNLOCK;

        return port;
    }
    #undef FUNC_NAME

Also there's the bit in the flush() vfunc about scm_i_terminating.
Nastiness all around.

Regards,

Andy
-- 
http://wingolog.org/

Re: request reversion regarding scm_i_* removal

[Ludovic Courtès]

Hi!

Andy Wingo <wingo@pobox.com> writes:

> Guile-gnome does, when registering a new port type, for gnome-vfs. The
> port API isn't threadsafe, and also sucks as I mentioned ;) From
> gnome-vfs/gnome/gw/gnome-vfs-port.c:
>
> #define LOCK scm_i_pthread_mutex_lock (&scm_i_port_table_mutex)
> #define UNLOCK scm_i_pthread_mutex_unlock (&scm_i_port_table_mutex)

Then would it be sufficient for Guile-GNOME if `scm_i_port_table_mutex'
and `scm_i_symbol_length ()' were kept public?

> Also there's the bit in the flush() vfunc about scm_i_terminating.

Normally, `really_cleanup_for_exit ()' calls `scm_flush_all_ports ()'
upon exit.  A quick test through GDB shows that `scm_i_terminating',
whose sole purpose is to indicate fport code that exception handling
cannot be used, is always zero.  Did you encounter situations where it
was needed?

Thanks,
Ludo'.

Re: request reversion regarding scm_i_* removal

[Ludovic Courtès]

Hello,

Andy Wingo <wingo@pobox.com> writes:

> The removal of the scm_i_* functions is an ABI break in the stable 1.8
> series. It should be reverted. (It's a great fix for master though.)

After discussion with Andy on IRC and looking for uses for `scm_i_' in
Google's codesearch, I've become convinced that we'd better not make
this change in 1.8 so that we don't break existing code.  That means
reverting these commits:

bc566d672f0d21f1567b83ddd732e4089983a2a8 Fix `SCM_INTERNAL' with GCC 4.3.
f0c64044d311ac31f491f959daf1b3a2163ee883 Add `SCM_INTERNAL' macro, use it.

If nobody complains, I'll revert them in 1.8 by the end of the next
week.

Nevertheless, the change *will* appear in the next stable release.  That
means we must strive to understand which `scm_i_' symbols have been used
and how the API can be improved to avoid them.

Thanks,
Ludo'.

Re: request reversion regarding scm_i_* removal

[Neil Jerram]

2008/8/30 Ludovic Courtès <ludo@gnu.org>:
>
> Nevertheless, the change *will* appear in the next stable release.

Do you mean the next 1.8.x release, or the first 1.10.x release
(whenever that might occur)?

Regards,
       Neil

Re: request reversion regarding scm_i_* removal

[Ludovic Courtès]

Hi Neil,

"Neil Jerram" <neiljerram@googlemail.com> writes:

> 2008/8/30 Ludovic Courtès <ludo@gnu.org>:
>>
>> Nevertheless, the change *will* appear in the next stable release.
>
> Do you mean the next 1.8.x release, or the first 1.10.x release
> (whenever that might occur)?

I meant 1.10.x, sorry for the confusion.

Thanks,
Ludo'.

Re: request reversion regarding scm_i_* removal

[Ludovic Courtès]

Hi,

ludo@gnu.org (Ludovic Courtès) writes:

> After discussion with Andy on IRC and looking for uses for `scm_i_' in
> Google's codesearch, I've become convinced that we'd better not make
> this change in 1.8 so that we don't break existing code.  That means
> reverting these commits:
>
> bc566d672f0d21f1567b83ddd732e4089983a2a8 Fix `SCM_INTERNAL' with GCC 4.3.
> f0c64044d311ac31f491f959daf1b3a2163ee883 Add `SCM_INTERNAL' macro, use it.
>
> If nobody complains, I'll revert them in 1.8 by the end of the next
> week.

Done!

Ludo'.