Thread: YA readline 4.2 patch

YA readline 4.2 patch

From
Jason Tishler
Date:
The attached patch enables PostgreSQL CVS to build cleanly under Cygwin
when built against readline 4.2.  Specifically, it handles the deprecation
of

    filename_completion_function()

with preference for

    rl_filename_completion_function()

Although, I was motivated by Cygwin support, IMO this patch is appropriate
for all platforms.  To quote from the readline source:

    #if 0
    /* Backwards compatibility (compat.c).  These will go away sometime. */
    ...
    extern READLINE_EXPORT(char, *filename_completion_function) ...
    #endif

Note that this patch is modeled after the one by Peter Eisentraut for
completion_matches():

    http://www.ca.postgresql.org/~petere/readline42.html

I tested this patch under the following environments:

    Cygwin with readline 4.1
    Cygwin with readline 4.2
    Linux with readline 2.2.1
    Linux with readline 4.2

and it behaved as expected.

Thanks,
Jason

--
Jason Tishler
Director, Software Engineering       Phone: 732.264.8770 x235
Dot Hill Systems Corp.               Fax:   732.264.8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

Attachment

Re: YA readline 4.2 patch

From
Tom Lane
Date:
Jason Tishler <Jason.Tishler@dothill.com> writes:
> The attached patch enables PostgreSQL CVS to build cleanly under Cygwin
> when built against readline 4.2.

I'm confused.  The current code already builds cleanly against readline
4.2.  Why do we need to change it?

            regards, tom lane

Re: YA readline 4.2 patch

From
Tom Lane
Date:
Jason Tishler <Jason.Tishler@dothill.com> writes:
>> I'm confused.  The current code already builds cleanly against readline
>> 4.2.  Why do we need to change it?

> 1.  The current code does *not* build cleanly against readline 4.2
> under Cygwin.

Why not?  It builds cleanly for me, under gcc.  I don't understand why
Cygwin would be different.

            regards, tom lane

Re: YA readline 4.2 patch

From
Jason Tishler
Date:
Tom,

On Wed, Jun 06, 2001 at 01:46:14PM -0400, Tom Lane wrote:
> Jason Tishler <Jason.Tishler@dothill.com> writes:
> > The attached patch enables PostgreSQL CVS to build cleanly under Cygwin
> > when built against readline 4.2.
>
> I'm confused.  The current code already builds cleanly against readline
> 4.2.  Why do we need to change it?

For the following reasons:

1.  The current code does *not* build cleanly against readline 4.2
under Cygwin.

2. The current code is using a deprecate function,
filename_completion_function(), which as of readline 4.2, is no longer
declared in readline.h:

    #if 0
    /* Backwards compatibility (compat.c).  These will go away sometime. */
    ...
    extern READLINE_EXPORT(char, *filename_completion_function) ...
    #endif

This is why it is (locally) declared on line 63 of
src/bin/psql/tab-complete.c.

3. Peter has already committed changes to CVS to handle
completion_matches() in exactly the same way.

4. Sometime in the future (readline 4.3?), this will have to be dealt
with when filename_completion_function() is finally removed from readline.

Jason

--
Jason Tishler
Director, Software Engineering       Phone: 732.264.8770 x235
Dot Hill Systems Corp.               Fax:   732.264.8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

Re: YA readline 4.2 patch

From
Jason Tishler
Date:
Tom,

On Wed, Jun 06, 2001 at 02:08:21PM -0400, Tom Lane wrote:
> Jason Tishler <Jason.Tishler@dothill.com> writes:
> >> I'm confused.  The current code already builds cleanly against readline
> >> 4.2.  Why do we need to change it?
>
> > 1.  The current code does *not* build cleanly against readline 4.2
> > under Cygwin.
>
> Why not?  It builds cleanly for me, under gcc.  I don't understand why
> Cygwin would be different.

I thought that we have been here before...

Remember that DLLs under Windows are *not* the same as shared libraries
under UNIX.

In order for Cygwin PostgreSQL to link cleanly against Cygwin readline
(which is DLL), the functions imported must be decorated with:

    __declspec(dllimport)

So, declaring filename_completion_function() with:

    extern char *filename_completion_function();

is not sufficient.  Instead, it must be declared as follows:

    extern __declspec(dllimport) char *filename_completion_function(const char *, int);

My patch accomplishes the above (without affecting other platforms),
but conditionally uses filename_completion_function() or
rl_filename_completion_function() as appropriate.

Jason

--
Jason Tishler
Director, Software Engineering       Phone: 732.264.8770 x235
Dot Hill Systems Corp.               Fax:   732.264.8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

Re: YA readline 4.2 patch

From
Bruce Momjian
Date:
Your patch has been added to the PostgreSQL unapplied patches list at:

    http://candle.pha.pa.us/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

> The attached patch enables PostgreSQL CVS to build cleanly under Cygwin
> when built against readline 4.2.  Specifically, it handles the deprecation
> of
>
>     filename_completion_function()
>
> with preference for
>
>     rl_filename_completion_function()
>
> Although, I was motivated by Cygwin support, IMO this patch is appropriate
> for all platforms.  To quote from the readline source:
>
>     #if 0
>     /* Backwards compatibility (compat.c).  These will go away sometime. */
>     ...
>     extern READLINE_EXPORT(char, *filename_completion_function) ...
>     #endif
>
> Note that this patch is modeled after the one by Peter Eisentraut for
> completion_matches():
>
>     http://www.ca.postgresql.org/~petere/readline42.html
>
> I tested this patch under the following environments:
>
>     Cygwin with readline 4.1
>     Cygwin with readline 4.2
>     Linux with readline 2.2.1
>     Linux with readline 4.2
>
> and it behaved as expected.
>
> Thanks,
> Jason
>
> --
> Jason Tishler
> Director, Software Engineering       Phone: 732.264.8770 x235
> Dot Hill Systems Corp.               Fax:   732.264.8798
> 82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
> Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Re: YA readline 4.2 patch

From
Jason Tishler
Date:
Bruce,

On Mon, Jun 11, 2001 at 12:31:08AM -0400, Bruce Momjian wrote:
> Your patch has been added to the PostgreSQL unapplied patches list at:
>
>     http://candle.pha.pa.us/cgi-bin/pgpatches
>
> I will try to apply it within the next 48 hours.

Does the above indicate that Tom Lane concurs too?

Thanks,
Jason

--
Jason Tishler
Director, Software Engineering       Phone: 732.264.8770 x235
Dot Hill Systems Corp.               Fax:   732.264.8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

Re: YA readline 4.2 patch

From
Bruce Momjian
Date:
> Bruce,
>
> On Mon, Jun 11, 2001 at 12:31:08AM -0400, Bruce Momjian wrote:
> > Your patch has been added to the PostgreSQL unapplied patches list at:
> >
> >     http://candle.pha.pa.us/cgi-bin/pgpatches
> >
> > I will try to apply it within the next 48 hours.
>
> Does the above indicate that Tom Lane concurs too?

Well, after you explained why it was needed, Tom didn't say anything
else so I assume he agrees.  You have a strong point that we can slip by
for a little longer under Unix but not under Cygwin and that this change
is inevitable.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Re: YA readline 4.2 patch

From
Bruce Momjian
Date:
Patch applied and autoconf run.

> The attached patch enables PostgreSQL CVS to build cleanly under Cygwin
> when built against readline 4.2.  Specifically, it handles the deprecation
> of
>
>     filename_completion_function()
>
> with preference for
>
>     rl_filename_completion_function()
>
> Although, I was motivated by Cygwin support, IMO this patch is appropriate
> for all platforms.  To quote from the readline source:
>
>     #if 0
>     /* Backwards compatibility (compat.c).  These will go away sometime. */
>     ...
>     extern READLINE_EXPORT(char, *filename_completion_function) ...
>     #endif
>
> Note that this patch is modeled after the one by Peter Eisentraut for
> completion_matches():
>
>     http://www.ca.postgresql.org/~petere/readline42.html
>
> I tested this patch under the following environments:
>
>     Cygwin with readline 4.1
>     Cygwin with readline 4.2
>     Linux with readline 2.2.1
>     Linux with readline 4.2
>
> and it behaved as expected.
>
> Thanks,
> Jason
>
> --
> Jason Tishler
> Director, Software Engineering       Phone: 732.264.8770 x235
> Dot Hill Systems Corp.               Fax:   732.264.8798
> 82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
> Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026