pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feed!USE_WIDE_UPPER_LOWER compile errors in v10+
7+ messages / 3 participants
[nested] [flat]
* !USE_WIDE_UPPER_LOWER compile errors in v10+
@ 2017-09-21 05:29 Noah Misch <noah@leadboat.com>
0 siblings, 2 replies; 7+ messages in thread
From: Noah Misch @ 2017-09-21 05:29 UTC (permalink / raw)
To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; +Cc: Peter Geoghegan <pg@bowt.ie>; pgsql-hackers
On Sat, Sep 16, 2017 at 03:33:53PM -0700, Peter Geoghegan wrote:
> On a related note, am I the only one that finds it questionable that
> str_tolower() has an "#ifdef USE_WIDE_UPPER_LOWER" block that itself
> contains an "#ifdef USE_ICU" block? It seems like those two things
> might get conflated on some platforms. We don't want lower() to ever
> not use the ICU infrastructure when an ICU collation is used, and yet
> it's not obvious that that's impossible. I understand that the code in
> regc_pg_locale.c kind of insists on using USE_WIDE_UPPER_LOWER
> facilities, and that that was always accepted as legacy that ICU had
> to live with. Maybe a static assertion is all that we need here (ICU
> builds must also be USE_WIDE_UPPER_LOWER builds).
I checked !USE_WIDE_UPPER_LOWER by configuring v10 as follows:
./configure -C --prefix=$HOME/sw/nopath/pg10 --enable-debug \
--enable-cassert --enable-depend --enable-tap-tests --with-libxml \
--with-gssapi --with-openssl ac_cv_func_towlower=no
The result fails to compile:
$ make
formatting.c: In function ‘str_tolower’:
formatting.c:1623:10: error: ‘mylocale’ undeclared (first use in this function)
if (mylocale)
^
... snipped other errors ...
A --with-icu build fails similarly. PG9.6 builds and passes "make check".
Perhaps it is time to require HAVE_WCSTOMBS and HAVE_TOWLOWER, removing
USE_WIDE_UPPER_LOWER? Every buildfarm fossil has both. Solaris 2.5.1 (out of
support for 12 years) might be the most interesting OS affected:
https://www.gnu.org/software/gnulib/manual/html_node/towlower.html
https://www.gnu.org/software/gnulib/manual/html_node/wcstombs.html
The above-described topic is currently a PostgreSQL 10 open item. Peter
(Eisentraut), since you committed the patch believed to have created it, you
own this open item. If some other commit is more relevant or if this does not
belong as a v10 open item, please let us know. Otherwise, please observe the
policy on open item ownership[1] and send a status update within three
calendar days of this message. Include a date for your subsequent status
update. Thanks.
[1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: !USE_WIDE_UPPER_LOWER compile errors in v10+
@ 2017-09-21 13:50 Tom Lane <tgl@sss.pgh.pa.us>
parent: Noah Misch <noah@leadboat.com>
1 sibling, 1 reply; 7+ messages in thread
From: Tom Lane @ 2017-09-21 13:50 UTC (permalink / raw)
To: Noah Misch <noah@leadboat.com>; +Cc: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; Peter Geoghegan <pg@bowt.ie>; pgsql-hackers
Noah Misch <noah@leadboat.com> writes:
> Perhaps it is time to require HAVE_WCSTOMBS and HAVE_TOWLOWER, removing
> USE_WIDE_UPPER_LOWER? Every buildfarm fossil has both.
+1 ... if nothing else, there's the problem that untested code is likely
to be broken. You just proved it *is* broken, of course, but my point
is that even if we repaired the immediate damage we could have little
confidence in it staying fixed.
I think the USE_WIDE_UPPER_LOWER split was originally my code, so I'm
willing to take care of removing it if there's consensus that that's
what to do.
I'm not sure that we need to treat this as a v10 open item, though.
The premise of removing !USE_WIDE_UPPER_LOWER is that nobody cares
anymore, therefore it shouldn't matter to users whether we remove it in
v10. There's an argument that having only two states of the relevant
code, not three, in the live back branches is worth something for
maintenance --- but should that outweigh the risk of breaking something
post-rc1?
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: !USE_WIDE_UPPER_LOWER compile errors in v10+
@ 2017-09-21 18:44 Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
parent: Noah Misch <noah@leadboat.com>
1 sibling, 0 replies; 7+ messages in thread
From: Peter Eisentraut @ 2017-09-21 18:44 UTC (permalink / raw)
To: Noah Misch <noah@leadboat.com>; +Cc: Peter Geoghegan <pg@bowt.ie>; pgsql-hackers
On 9/21/17 01:29, Noah Misch wrote:
> I checked !USE_WIDE_UPPER_LOWER by configuring v10 as follows:
>
> ./configure -C --prefix=$HOME/sw/nopath/pg10 --enable-debug \
> --enable-cassert --enable-depend --enable-tap-tests --with-libxml \
> --with-gssapi --with-openssl ac_cv_func_towlower=no
>
> The result fails to compile:
Yeah, the placement of the ifdef blocks was pretty bogus. This patch
fixes it.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 9e51e4e52ae565503934c97c84ee5a7d9bcb2dcc Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Thu, 21 Sep 2017 14:42:10 -0400
Subject: [PATCH] Fix build with !USE_WIDE_UPPER_LOWER
The placement of the ifdef blocks in formatting.c was pretty bogus, so
the code failed to compile if USE_WIDE_UPPER_LOWER was not defined.
---
src/backend/utils/adt/formatting.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c
index 46f45f6654..2bf484cda3 100644
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -1528,7 +1528,6 @@ str_tolower(const char *buff, size_t nbytes, Oid collid)
{
result = asc_tolower(buff, nbytes);
}
-#ifdef USE_WIDE_UPPER_LOWER
else
{
pg_locale_t mylocale = 0;
@@ -1566,6 +1565,7 @@ str_tolower(const char *buff, size_t nbytes, Oid collid)
else
#endif
{
+#ifdef USE_WIDE_UPPER_LOWER
if (pg_database_encoding_max_length() > 1)
{
wchar_t *workspace;
@@ -1603,8 +1603,8 @@ str_tolower(const char *buff, size_t nbytes, Oid collid)
wchar2char(result, workspace, result_size, mylocale);
pfree(workspace);
}
-#endif /* USE_WIDE_UPPER_LOWER */
else
+#endif /* USE_WIDE_UPPER_LOWER */
{
char *p;
@@ -1652,7 +1652,6 @@ str_toupper(const char *buff, size_t nbytes, Oid collid)
{
result = asc_toupper(buff, nbytes);
}
-#ifdef USE_WIDE_UPPER_LOWER
else
{
pg_locale_t mylocale = 0;
@@ -1690,6 +1689,7 @@ str_toupper(const char *buff, size_t nbytes, Oid collid)
else
#endif
{
+#ifdef USE_WIDE_UPPER_LOWER
if (pg_database_encoding_max_length() > 1)
{
wchar_t *workspace;
@@ -1727,8 +1727,8 @@ str_toupper(const char *buff, size_t nbytes, Oid collid)
wchar2char(result, workspace, result_size, mylocale);
pfree(workspace);
}
-#endif /* USE_WIDE_UPPER_LOWER */
else
+#endif /* USE_WIDE_UPPER_LOWER */
{
char *p;
@@ -1777,7 +1777,6 @@ str_initcap(const char *buff, size_t nbytes, Oid collid)
{
result = asc_initcap(buff, nbytes);
}
-#ifdef USE_WIDE_UPPER_LOWER
else
{
pg_locale_t mylocale = 0;
@@ -1815,6 +1814,7 @@ str_initcap(const char *buff, size_t nbytes, Oid collid)
else
#endif
{
+#ifdef USE_WIDE_UPPER_LOWER
if (pg_database_encoding_max_length() > 1)
{
wchar_t *workspace;
@@ -1864,8 +1864,8 @@ str_initcap(const char *buff, size_t nbytes, Oid collid)
wchar2char(result, workspace, result_size, mylocale);
pfree(workspace);
}
-#endif /* USE_WIDE_UPPER_LOWER */
else
+#endif /* USE_WIDE_UPPER_LOWER */
{
char *p;
--
2.14.1
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Attachments:
[text/plain] 0001-Fix-build-with-USE_WIDE_UPPER_LOWER.patch (2.7K, ../../509cd374-4693-4592-5787-38696814f3e6@2ndquadrant.com/2-0001-Fix-build-with-USE_WIDE_UPPER_LOWER.patch)
download | inline diff:
From 9e51e4e52ae565503934c97c84ee5a7d9bcb2dcc Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Thu, 21 Sep 2017 14:42:10 -0400
Subject: [PATCH] Fix build with !USE_WIDE_UPPER_LOWER
The placement of the ifdef blocks in formatting.c was pretty bogus, so
the code failed to compile if USE_WIDE_UPPER_LOWER was not defined.
---
src/backend/utils/adt/formatting.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c
index 46f45f6654..2bf484cda3 100644
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -1528,7 +1528,6 @@ str_tolower(const char *buff, size_t nbytes, Oid collid)
{
result = asc_tolower(buff, nbytes);
}
-#ifdef USE_WIDE_UPPER_LOWER
else
{
pg_locale_t mylocale = 0;
@@ -1566,6 +1565,7 @@ str_tolower(const char *buff, size_t nbytes, Oid collid)
else
#endif
{
+#ifdef USE_WIDE_UPPER_LOWER
if (pg_database_encoding_max_length() > 1)
{
wchar_t *workspace;
@@ -1603,8 +1603,8 @@ str_tolower(const char *buff, size_t nbytes, Oid collid)
wchar2char(result, workspace, result_size, mylocale);
pfree(workspace);
}
-#endif /* USE_WIDE_UPPER_LOWER */
else
+#endif /* USE_WIDE_UPPER_LOWER */
{
char *p;
@@ -1652,7 +1652,6 @@ str_toupper(const char *buff, size_t nbytes, Oid collid)
{
result = asc_toupper(buff, nbytes);
}
-#ifdef USE_WIDE_UPPER_LOWER
else
{
pg_locale_t mylocale = 0;
@@ -1690,6 +1689,7 @@ str_toupper(const char *buff, size_t nbytes, Oid collid)
else
#endif
{
+#ifdef USE_WIDE_UPPER_LOWER
if (pg_database_encoding_max_length() > 1)
{
wchar_t *workspace;
@@ -1727,8 +1727,8 @@ str_toupper(const char *buff, size_t nbytes, Oid collid)
wchar2char(result, workspace, result_size, mylocale);
pfree(workspace);
}
-#endif /* USE_WIDE_UPPER_LOWER */
else
+#endif /* USE_WIDE_UPPER_LOWER */
{
char *p;
@@ -1777,7 +1777,6 @@ str_initcap(const char *buff, size_t nbytes, Oid collid)
{
result = asc_initcap(buff, nbytes);
}
-#ifdef USE_WIDE_UPPER_LOWER
else
{
pg_locale_t mylocale = 0;
@@ -1815,6 +1814,7 @@ str_initcap(const char *buff, size_t nbytes, Oid collid)
else
#endif
{
+#ifdef USE_WIDE_UPPER_LOWER
if (pg_database_encoding_max_length() > 1)
{
wchar_t *workspace;
@@ -1864,8 +1864,8 @@ str_initcap(const char *buff, size_t nbytes, Oid collid)
wchar2char(result, workspace, result_size, mylocale);
pfree(workspace);
}
-#endif /* USE_WIDE_UPPER_LOWER */
else
+#endif /* USE_WIDE_UPPER_LOWER */
{
char *p;
--
2.14.1
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: !USE_WIDE_UPPER_LOWER compile errors in v10+
@ 2017-09-21 21:38 Tom Lane <tgl@sss.pgh.pa.us>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 2 replies; 7+ messages in thread
From: Tom Lane @ 2017-09-21 21:38 UTC (permalink / raw)
To: Noah Misch <noah@leadboat.com>; +Cc: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; Peter Geoghegan <pg@bowt.ie>; pgsql-hackers
I wrote:
> Noah Misch <noah@leadboat.com> writes:
>> Perhaps it is time to require HAVE_WCSTOMBS and HAVE_TOWLOWER, removing
>> USE_WIDE_UPPER_LOWER? Every buildfarm fossil has both.
> +1 ... if nothing else, there's the problem that untested code is likely
> to be broken. You just proved it *is* broken, of course, but my point
> is that even if we repaired the immediate damage we could have little
> confidence in it staying fixed.
Further notes about that:
* The Single Unix Spec v2 (a/k/a POSIX 1997), which has been our minimum
portability spec for quite awhile, requires wcstombs() and towlower(),
and further requires the latter to be declared in <wctype.h>.
* Surveying the buildfarm, I agree with your conclusion that every active
member has wcstombs() and towlower(). gaur/pademelon is the lone member
that lacks <wctype.h>; it declares towlower() in <wchar.h> instead. It's
not so surprising that that system adheres to a pre-1997 idea of where to
put that, because its /usr/include files mostly date from 1996 ...
Meanwhile, I see that Peter has posted a fix for the immediate problem.
I propose that Peter should apply his fix in HEAD and v10, and then
I'll rip out the !USE_WIDE_UPPER_LOWER code paths in HEAD only.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: !USE_WIDE_UPPER_LOWER compile errors in v10+
@ 2017-09-22 03:08 Noah Misch <noah@leadboat.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 0 replies; 7+ messages in thread
From: Noah Misch @ 2017-09-22 03:08 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; Peter Geoghegan <pg@bowt.ie>; pgsql-hackers
On Thu, Sep 21, 2017 at 05:38:13PM -0400, Tom Lane wrote:
> I wrote:
> > Noah Misch <noah@leadboat.com> writes:
> >> Perhaps it is time to require HAVE_WCSTOMBS and HAVE_TOWLOWER, removing
> >> USE_WIDE_UPPER_LOWER? Every buildfarm fossil has both.
>
> > +1 ... if nothing else, there's the problem that untested code is likely
> > to be broken. You just proved it *is* broken, of course, but my point
> > is that even if we repaired the immediate damage we could have little
> > confidence in it staying fixed.
That would be easy enough to ensure by adding ac_cv_func_towlower=no to some
buildfarm animal. But the real-world need for said code is clearly dead and
gone. Good riddance.
> Meanwhile, I see that Peter has posted a fix for the immediate problem.
> I propose that Peter should apply his fix in HEAD and v10, and then
> I'll rip out the !USE_WIDE_UPPER_LOWER code paths in HEAD only.
That works for me.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: !USE_WIDE_UPPER_LOWER compile errors in v10+
@ 2017-09-22 13:29 Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 1 reply; 7+ messages in thread
From: Peter Eisentraut @ 2017-09-22 13:29 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; Noah Misch <noah@leadboat.com>; +Cc: Peter Geoghegan <pg@bowt.ie>; pgsql-hackers
On 9/21/17 17:38, Tom Lane wrote:
> Meanwhile, I see that Peter has posted a fix for the immediate problem.
> I propose that Peter should apply his fix in HEAD and v10, and then
done
> I'll rip out the !USE_WIDE_UPPER_LOWER code paths in HEAD only.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: !USE_WIDE_UPPER_LOWER compile errors in v10+
@ 2017-09-22 15:02 Tom Lane <tgl@sss.pgh.pa.us>
parent: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
0 siblings, 0 replies; 7+ messages in thread
From: Tom Lane @ 2017-09-22 15:02 UTC (permalink / raw)
To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; +Cc: Noah Misch <noah@leadboat.com>; Peter Geoghegan <pg@bowt.ie>; pgsql-hackers
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> On 9/21/17 17:38, Tom Lane wrote:
>> Meanwhile, I see that Peter has posted a fix for the immediate problem.
>> I propose that Peter should apply his fix in HEAD and v10, and then
> done
>> I'll rip out the !USE_WIDE_UPPER_LOWER code paths in HEAD only.
And done.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2017-09-22 15:02 UTC | newest]
Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2017-09-21 05:29 !USE_WIDE_UPPER_LOWER compile errors in v10+ Noah Misch <noah@leadboat.com>
2017-09-21 13:50 ` Tom Lane <tgl@sss.pgh.pa.us>
2017-09-21 21:38 ` Tom Lane <tgl@sss.pgh.pa.us>
2017-09-22 03:08 ` Noah Misch <noah@leadboat.com>
2017-09-22 13:29 ` Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2017-09-22 15:02 ` Tom Lane <tgl@sss.pgh.pa.us>
2017-09-21 18:44 ` Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox