Re: YA readline 4.2 patch

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(dot)Tishler(at)dothill(dot)com
Hazlet, NJ 07730 USA WWW: http://www.dothill.com

Jason Tishler <Jason(dot)Tishler(at)dothill(dot)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

Tom,

On Wed, Jun 06, 2001 at 01:46:14PM -0400, Tom Lane wrote:
> Jason Tishler <Jason(dot)Tishler(at)dothill(dot)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(dot)Tishler(at)dothill(dot)com
Hazlet, NJ 07730 USA WWW: http://www.dothill.com

Jason Tishler <Jason(dot)Tishler(at)dothill(dot)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

Tom,

On Wed, Jun 06, 2001 at 02:08:21PM -0400, Tom Lane wrote:
> Jason Tishler <Jason(dot)Tishler(at)dothill(dot)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(dot)Tishler(at)dothill(dot)com
Hazlet, NJ 07730 USA WWW: http://www.dothill.com

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(dot)Tishler(at)dothill(dot)com
> Hazlet, NJ 07730 USA WWW: http://www.dothill.com

[ Attachment, skipping... ]

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

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

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(dot)Tishler(at)dothill(dot)com
Hazlet, NJ 07730 USA WWW: http://www.dothill.com

> 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(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

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(dot)Tishler(at)dothill(dot)com
> Hazlet, NJ 07730 USA WWW: http://www.dothill.com

[ Attachment, skipping... ]

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

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