agora inbox for [email protected]  
help / color / mirror / Atom feed
pgsql: Add configure infrastructure to detect support for C99's restric
16+ messages / 4 participants
[nested] [flat]

* pgsql: Add configure infrastructure to detect support for C99's restric
@ 2017-10-11 23:11 Andres Freund <[email protected]>
  2017-10-12 00:13 ` Re: pgsql: Add configure infrastructure to detect support for C99's restric Andres Freund <[email protected]>
  0 siblings, 1 reply; 16+ messages in thread

From: Andres Freund @ 2017-10-11 23:11 UTC (permalink / raw)
  To: [email protected]

Add configure infrastructure to detect support for C99's restrict.

Will be used in later commits improving performance for a few key
routines where information about aliasing allows for significantly
better code generation.

This allows to use the C99 'restrict' keyword without breaking C89, or
for that matter C++, compilers. If not supported it's defined to be
empty.

Author: Andres Freund
Discussion: https://postgr.es/m/[email protected]

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/0b974dba2d6b5581ce422ed883209de46f313fb6

Modified Files
--------------
configure                     | 46 +++++++++++++++++++++++++++++++++++++++++++
configure.in                  |  1 +
src/include/pg_config.h.in    | 14 +++++++++++++
src/include/pg_config.h.win32 | 11 +++++++++++
4 files changed, 72 insertions(+)


-- 
Sent via pgsql-committers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers


^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: pgsql: Add configure infrastructure to detect support for C99's restric
  2017-10-11 23:11 pgsql: Add configure infrastructure to detect support for C99's restric Andres Freund <[email protected]>
@ 2017-10-12 00:13 ` Andres Freund <[email protected]>
  2017-10-12 04:59   ` Re: [COMMITTERS] pgsql: Add configure infrastructure to detect support for C99's restric Andres Freund <[email protected]>
  0 siblings, 1 reply; 16+ messages in thread

From: Andres Freund @ 2017-10-12 00:13 UTC (permalink / raw)
  To: [email protected]

Hi,

On 2017-10-11 23:11:15 +0000, Andres Freund wrote:
> Add configure infrastructure to detect support for C99's restrict.
> 
> Will be used in later commits improving performance for a few key
> routines where information about aliasing allows for significantly
> better code generation.
> 
> This allows to use the C99 'restrict' keyword without breaking C89, or
> for that matter C++, compilers. If not supported it's defined to be
> empty.

Woodlouse doesn't like this, erroring out with:
C:\buildfarm\buildenv\HEAD\pgsql.build\src\include\libpq/pqformat.h(47): error C2219: syntax error : type qualifier must be after '*' (src/backend/access/common/printsimple.c) [C:\buildfarm\buildenv\HEAD\pgsql.build\postgres.vcxproj]

It's MSVC being super peculiar about error checks. I think msvc is just
confused by the pointer hiding typedef. Using some online MSVC (and
other compilers) frontend:
https://godbolt.org/g/TD3nmA

I confirmed that removing the pointer hiding typedef indeed resolves the
isssue.

I can't quite decide whether msvc just has taste and dislikes pointer
hiding typedefs as much as I do, or whether it's incredibly stupid ;)

I'll add a comment and use StringInfoData *.

Greetings,

Andres Freund


-- 
Sent via pgsql-committers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers



^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: [COMMITTERS] pgsql: Add configure infrastructure to detect support for C99's restric
  2017-10-11 23:11 pgsql: Add configure infrastructure to detect support for C99's restric Andres Freund <[email protected]>
  2017-10-12 00:13 ` Re: pgsql: Add configure infrastructure to detect support for C99's restric Andres Freund <[email protected]>
@ 2017-10-12 04:59   ` Andres Freund <[email protected]>
  2017-10-12 05:05     ` Re: [HACKERS] Re: pgsql: Add configure infrastructure to detect support for C99's restric Andres Freund <[email protected]>
  2017-10-12 15:30     ` Re: pgsql: Add configure infrastructure to detect support for C99's restric Tom Lane <[email protected]>
  0 siblings, 2 replies; 16+ messages in thread

From: Andres Freund @ 2017-10-12 04:59 UTC (permalink / raw)
  To: [email protected]; pgsql-hackers

On 2017-10-11 17:13:20 -0700, Andres Freund wrote:
> Hi,
> 
> On 2017-10-11 23:11:15 +0000, Andres Freund wrote:
> > Add configure infrastructure to detect support for C99's restrict.
> > 
> > Will be used in later commits improving performance for a few key
> > routines where information about aliasing allows for significantly
> > better code generation.
> > 
> > This allows to use the C99 'restrict' keyword without breaking C89, or
> > for that matter C++, compilers. If not supported it's defined to be
> > empty.
> 
> Woodlouse doesn't like this, erroring out with:
> C:\buildfarm\buildenv\HEAD\pgsql.build\src\include\libpq/pqformat.h(47): error C2219: syntax error : type qualifier must be after '*' (src/backend/access/common/printsimple.c) [C:\buildfarm\buildenv\HEAD\pgsql.build\postgres.vcxproj]
> 
> It's MSVC being super peculiar about error checks. I think msvc is just
> confused by the pointer hiding typedef. Using some online MSVC (and
> other compilers) frontend:
> https://godbolt.org/g/TD3nmA
> 
> I confirmed that removing the pointer hiding typedef indeed resolves the
> isssue.
> 
> I can't quite decide whether msvc just has taste and dislikes pointer
> hiding typedefs as much as I do, or whether it's incredibly stupid ;)
> 
> I'll add a comment and use StringInfoData *.

That fixed that problem I think. But unfortunately since then another
problem has been reported by some other animals, all with older msvc
versions afaict (thrips - vs 2012, bowerbird - vs 2012). Those report
that the defining of restrict to __restrict interfers with some system
headers:
C:\Program Files (x86)\Microsoft Visual Studio 11.0\VC\include\stdlib.h(619): error C2485: '__restrict' : unrecognized extended attribute (src/backend/access/brin/brin.c) [c:\prog\bf\root\HEAD\pgsql.build\postgres.vcxproj]

Presumably that is because the headers contain __declspec(restrict) on
some function declarations.

I've temporarily silenced that error by moving the stdlib.h include
before the definition of restrict, but that seems fairly fragile. I
primarily wanted to see whether there's other problems.  At least thrips
is is now happy.

I see a number of options to continue:
- only define restrict on msvc 2013+ - for some reason woodlouse didn't
  complain about this problem, but I'm very doubtful that that's
  actually reliable.
- rename restrict to pg_restrict. That's annoying because we'd have to
  copy the autoconf test.
- live with the hack of including stdlib.h early, in pg_config.h.win32.
- $better_idea

Comments?

Greetings,

Andres Freund


-- 
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: [HACKERS] Re: pgsql: Add configure infrastructure to detect support for C99's restric
  2017-10-11 23:11 pgsql: Add configure infrastructure to detect support for C99's restric Andres Freund <[email protected]>
  2017-10-12 00:13 ` Re: pgsql: Add configure infrastructure to detect support for C99's restric Andres Freund <[email protected]>
  2017-10-12 04:59   ` Re: [COMMITTERS] pgsql: Add configure infrastructure to detect support for C99's restric Andres Freund <[email protected]>
@ 2017-10-12 05:05     ` Andres Freund <[email protected]>
  1 sibling, 0 replies; 16+ messages in thread

From: Andres Freund @ 2017-10-12 05:05 UTC (permalink / raw)
  To: [email protected]; pgsql-hackers

On 2017-10-11 21:59:53 -0700, Andres Freund wrote:
> That fixed that problem I think. But unfortunately since then another
> problem has been reported by some other animals, all with older msvc
> versions afaict (thrips - vs 2012, bowerbird - vs 2012).

Correction, thrips is vs 2010, not 2012.


-- 
Sent via pgsql-committers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers



^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: pgsql: Add configure infrastructure to detect support for C99's restric
  2017-10-11 23:11 pgsql: Add configure infrastructure to detect support for C99's restric Andres Freund <[email protected]>
  2017-10-12 00:13 ` Re: pgsql: Add configure infrastructure to detect support for C99's restric Andres Freund <[email protected]>
  2017-10-12 04:59   ` Re: [COMMITTERS] pgsql: Add configure infrastructure to detect support for C99's restric Andres Freund <[email protected]>
@ 2017-10-12 15:30     ` Tom Lane <[email protected]>
  2017-10-12 17:45       ` Re: pgsql: Add configure infrastructure to detect support for C99's restric Andres Freund <[email protected]>
  1 sibling, 1 reply; 16+ messages in thread

From: Tom Lane @ 2017-10-12 15:30 UTC (permalink / raw)
  To: Andres Freund <[email protected]>; +Cc: [email protected]

Andres Freund <[email protected]> writes:
> I've temporarily silenced that error by moving the stdlib.h include
> before the definition of restrict, but that seems fairly fragile. I
> primarily wanted to see whether there's other problems.  At least thrips
> is is now happy.

> I see a number of options to continue:
> - only define restrict on msvc 2013+ - for some reason woodlouse didn't
>   complain about this problem, but I'm very doubtful that that's
>   actually reliable.
> - rename restrict to pg_restrict. That's annoying because we'd have to
>   copy the autoconf test.
> - live with the hack of including stdlib.h early, in pg_config.h.win32.
> - $better_idea

I don't actually see why you need a #define at all --- "restrict" is the
standard spelling of the keyword no?

I really do not like the stdlib.h hack: that amounts to assuming that
only stdlib.h does or ever will contain declspec(restrict).  Maybe
you could get away with that if you were applying it only to long-dead
MSVC versions, but doing it "#if (_MSC_VER >= 1500)" is clearly going
to break someday.

			regards, tom lane


-- 
Sent via pgsql-committers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers



^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: pgsql: Add configure infrastructure to detect support for C99's restric
  2017-10-11 23:11 pgsql: Add configure infrastructure to detect support for C99's restric Andres Freund <[email protected]>
  2017-10-12 00:13 ` Re: pgsql: Add configure infrastructure to detect support for C99's restric Andres Freund <[email protected]>
  2017-10-12 04:59   ` Re: [COMMITTERS] pgsql: Add configure infrastructure to detect support for C99's restric Andres Freund <[email protected]>
  2017-10-12 15:30     ` Re: pgsql: Add configure infrastructure to detect support for C99's restric Tom Lane <[email protected]>
@ 2017-10-12 17:45       ` Andres Freund <[email protected]>
  2017-10-12 17:55         ` Re: pgsql: Add configure infrastructure to detect support for C99's restric Tom Lane <[email protected]>
  0 siblings, 1 reply; 16+ messages in thread

From: Andres Freund @ 2017-10-12 17:45 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: [email protected]

On 2017-10-12 11:30:00 -0400, Tom Lane wrote:
> Andres Freund <[email protected]> writes:
> > I've temporarily silenced that error by moving the stdlib.h include
> > before the definition of restrict, but that seems fairly fragile. I
> > primarily wanted to see whether there's other problems.  At least thrips
> > is is now happy.
> 
> > I see a number of options to continue:
> > - only define restrict on msvc 2013+ - for some reason woodlouse didn't
> >   complain about this problem, but I'm very doubtful that that's
> >   actually reliable.
> > - rename restrict to pg_restrict. That's annoying because we'd have to
> >   copy the autoconf test.
> > - live with the hack of including stdlib.h early, in pg_config.h.win32.
> > - $better_idea
> 
> I don't actually see why you need a #define at all --- "restrict" is the
> standard spelling of the keyword no?

It is, but a lot of compilers name it differently, e.g. __restrict in
the case of msvc.


> I really do not like the stdlib.h hack: that amounts to assuming that
> only stdlib.h does or ever will contain declspec(restrict).  Maybe
> you could get away with that if you were applying it only to long-dead
> MSVC versions, but doing it "#if (_MSC_VER >= 1500)" is clearly going
> to break someday.

Yea, I dislike it quite a bit too. Unfortunately not even defining
restrict to empty as if it were unsupported looks viable to me :(

Greetings,

Andres Freund


-- 
Sent via pgsql-committers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers



^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: pgsql: Add configure infrastructure to detect support for C99's restric
  2017-10-11 23:11 pgsql: Add configure infrastructure to detect support for C99's restric Andres Freund <[email protected]>
  2017-10-12 00:13 ` Re: pgsql: Add configure infrastructure to detect support for C99's restric Andres Freund <[email protected]>
  2017-10-12 04:59   ` Re: [COMMITTERS] pgsql: Add configure infrastructure to detect support for C99's restric Andres Freund <[email protected]>
  2017-10-12 15:30     ` Re: pgsql: Add configure infrastructure to detect support for C99's restric Tom Lane <[email protected]>
  2017-10-12 17:45       ` Re: pgsql: Add configure infrastructure to detect support for C99's restric Andres Freund <[email protected]>
@ 2017-10-12 17:55         ` Tom Lane <[email protected]>
  2017-10-12 18:03           ` Re: pgsql: Add configure infrastructure to detect support for C99's restric Andres Freund <[email protected]>
  0 siblings, 1 reply; 16+ messages in thread

From: Tom Lane @ 2017-10-12 17:55 UTC (permalink / raw)
  To: Andres Freund <[email protected]>; +Cc: [email protected]

Andres Freund <[email protected]> writes:
> On 2017-10-12 11:30:00 -0400, Tom Lane wrote:
>> I don't actually see why you need a #define at all --- "restrict" is the
>> standard spelling of the keyword no?

> It is, but a lot of compilers name it differently, e.g. __restrict in
> the case of msvc.

It's 2017 and they're still not C99 compliant?  Oh well.

TBH, I really doubt that restrict buys us enough performance to justify
dealing with this.  I'd just revert that change altogether.

Or, if you insist on having it, we're going to have to go the pg_restrict
route.  I don't see why that means duplicating any configure logic: on
non-Windows we can use the autoconf probe and then write
"#define pg_restrict restrict".

			regards, tom lane


-- 
Sent via pgsql-committers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers



^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: pgsql: Add configure infrastructure to detect support for C99's restric
  2017-10-11 23:11 pgsql: Add configure infrastructure to detect support for C99's restric Andres Freund <[email protected]>
  2017-10-12 00:13 ` Re: pgsql: Add configure infrastructure to detect support for C99's restric Andres Freund <[email protected]>
  2017-10-12 04:59   ` Re: [COMMITTERS] pgsql: Add configure infrastructure to detect support for C99's restric Andres Freund <[email protected]>
  2017-10-12 15:30     ` Re: pgsql: Add configure infrastructure to detect support for C99's restric Tom Lane <[email protected]>
  2017-10-12 17:45       ` Re: pgsql: Add configure infrastructure to detect support for C99's restric Andres Freund <[email protected]>
  2017-10-12 17:55         ` Re: pgsql: Add configure infrastructure to detect support for C99's restric Tom Lane <[email protected]>
@ 2017-10-12 18:03           ` Andres Freund <[email protected]>
  2017-10-12 22:35             ` Re: pgsql: Add configure infrastructure to detect support for C99's restric Andres Freund <[email protected]>
  0 siblings, 1 reply; 16+ messages in thread

From: Andres Freund @ 2017-10-12 18:03 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: [email protected]

Hi,


On 2017-10-12 13:55:07 -0400, Tom Lane wrote:
> Andres Freund <[email protected]> writes:
> > On 2017-10-12 11:30:00 -0400, Tom Lane wrote:
> >> I don't actually see why you need a #define at all --- "restrict" is the
> >> standard spelling of the keyword no?
> 
> > It is, but a lot of compilers name it differently, e.g. __restrict in
> > the case of msvc.
> 
> It's 2017 and they're still not C99 compliant?  Oh well.

...


> TBH, I really doubt that restrict buys us enough performance to justify
> dealing with this.  I'd just revert that change altogether.

It's quite noticeable, and there's plenty of other case where it seems
likely to be beneficial.


> Or, if you insist on having it, we're going to have to go the pg_restrict
> route.  I don't see why that means duplicating any configure logic: on
> non-Windows we can use the autoconf probe and then write
> "#define pg_restrict restrict".

Yea, that should work. I'll try to come up with a patch.

Greetings,

Andres Freund


-- 
Sent via pgsql-committers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers



^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: pgsql: Add configure infrastructure to detect support for C99's restric
  2017-10-11 23:11 pgsql: Add configure infrastructure to detect support for C99's restric Andres Freund <[email protected]>
  2017-10-12 00:13 ` Re: pgsql: Add configure infrastructure to detect support for C99's restric Andres Freund <[email protected]>
  2017-10-12 04:59   ` Re: [COMMITTERS] pgsql: Add configure infrastructure to detect support for C99's restric Andres Freund <[email protected]>
  2017-10-12 15:30     ` Re: pgsql: Add configure infrastructure to detect support for C99's restric Tom Lane <[email protected]>
  2017-10-12 17:45       ` Re: pgsql: Add configure infrastructure to detect support for C99's restric Andres Freund <[email protected]>
  2017-10-12 17:55         ` Re: pgsql: Add configure infrastructure to detect support for C99's restric Tom Lane <[email protected]>
  2017-10-12 18:03           ` Re: pgsql: Add configure infrastructure to detect support for C99's restric Andres Freund <[email protected]>
@ 2017-10-12 22:35             ` Andres Freund <[email protected]>
  2017-10-12 23:14               ` Re: pgsql: Add configure infrastructure to detect support for C99's restric Tom Lane <[email protected]>
  0 siblings, 1 reply; 16+ messages in thread

From: Andres Freund @ 2017-10-12 22:35 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: [email protected]

Hi,

On 2017-10-12 11:03:34 -0700, Andres Freund wrote:
> On 2017-10-12 13:55:07 -0400, Tom Lane wrote:
> > Or, if you insist on having it, we're going to have to go the pg_restrict
> > route.  I don't see why that means duplicating any configure logic: on
> > non-Windows we can use the autoconf probe and then write
> > "#define pg_restrict restrict".
> 
> Yea, that should work. I'll try to come up with a patch.

We can't do so unconditionally in c.h or such, because that'd again
cause conflicts with __declspec(restrict) on MSVC versions that don't
support restrict, because it'd require restrict to be defined empty.

But it's easy to do so in configure, and then have a separate definition
in pg_config.h.win32. Done so in the attached commit. It's slightly ugly
to have two definitions of restrict in pg_config.h.in, but whatever.

Greetings,

Andres Freund


^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: pgsql: Add configure infrastructure to detect support for C99's restric
  2017-10-11 23:11 pgsql: Add configure infrastructure to detect support for C99's restric Andres Freund <[email protected]>
  2017-10-12 00:13 ` Re: pgsql: Add configure infrastructure to detect support for C99's restric Andres Freund <[email protected]>
  2017-10-12 04:59   ` Re: [COMMITTERS] pgsql: Add configure infrastructure to detect support for C99's restric Andres Freund <[email protected]>
  2017-10-12 15:30     ` Re: pgsql: Add configure infrastructure to detect support for C99's restric Tom Lane <[email protected]>
  2017-10-12 17:45       ` Re: pgsql: Add configure infrastructure to detect support for C99's restric Andres Freund <[email protected]>
  2017-10-12 17:55         ` Re: pgsql: Add configure infrastructure to detect support for C99's restric Tom Lane <[email protected]>
  2017-10-12 18:03           ` Re: pgsql: Add configure infrastructure to detect support for C99's restric Andres Freund <[email protected]>
  2017-10-12 22:35             ` Re: pgsql: Add configure infrastructure to detect support for C99's restric Andres Freund <[email protected]>
@ 2017-10-12 23:14               ` Tom Lane <[email protected]>
  0 siblings, 0 replies; 16+ messages in thread

From: Tom Lane @ 2017-10-12 23:14 UTC (permalink / raw)
  To: Andres Freund <[email protected]>; +Cc: [email protected]

Andres Freund <[email protected]> writes:
> But it's easy to do so in configure, and then have a separate definition
> in pg_config.h.win32. Done so in the attached commit. It's slightly ugly
> to have two definitions of restrict in pg_config.h.in, but whatever.

WFM.

			regards, tom lane


-- 
Sent via pgsql-committers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers



^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* [PATCH v2 06/11] distro agnostic
@ 2020-09-23 03:55 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 16+ messages in thread

From: Justin Pryzby @ 2020-09-23 03:55 UTC (permalink / raw)

---
 doc/src/sgml/sepgsql.sgml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/src/sgml/sepgsql.sgml b/doc/src/sgml/sepgsql.sgml
index 9961569afc..15417bf19d 100644
--- a/doc/src/sgml/sepgsql.sgml
+++ b/doc/src/sgml/sepgsql.sgml
@@ -88,7 +88,7 @@ Policy from config file:        targeted
   <para>
    To build this module, include the option <literal>--with-selinux</literal> in
    your PostgreSQL <literal>configure</literal> command.  Be sure that the
-   <filename>libselinux-devel</filename> RPM is installed at build time.
+   <filename>libselinux-devel</filename> package is installed at build time.
   </para>
 
   <para>
@@ -177,7 +177,7 @@ $ for DBNAME in template0 template1 postgres; do
    Makefile on your system; the path shown below is only an example.
    (This Makefile is usually supplied by the
    <filename>selinux-policy-devel</filename> or
-   <filename>selinux-policy</filename> RPM.)
+   <filename>selinux-policy</filename> package.)
    Once built, install this policy package using the
    <command>semodule</command> command, which loads supplied policy packages
    into the kernel.  If the package is correctly installed,
-- 
2.17.0


--kH8JNVvasRCCW1Oz
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v2-0007-style-and-consistency.patch"



^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* [PATCH v3 07/12] distro agnostic
@ 2020-09-23 03:55 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 16+ messages in thread

From: Justin Pryzby @ 2020-09-23 03:55 UTC (permalink / raw)

---
 doc/src/sgml/sepgsql.sgml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/src/sgml/sepgsql.sgml b/doc/src/sgml/sepgsql.sgml
index e896a44ce5..f7fa65da30 100644
--- a/doc/src/sgml/sepgsql.sgml
+++ b/doc/src/sgml/sepgsql.sgml
@@ -88,7 +88,7 @@ Policy from config file:        targeted
   <para>
    To build this module, include the option <literal>--with-selinux</literal> in
    your PostgreSQL <literal>configure</literal> command.  Be sure that the
-   <filename>libselinux-devel</filename> RPM is installed at build time.
+   <filename>libselinux-devel</filename> package is installed at build time.
   </para>
 
   <para>
@@ -177,7 +177,7 @@ $ for DBNAME in template0 template1 postgres; do
    Makefile on your system; the path shown below is only an example.
    (This Makefile is usually supplied by the
    <filename>selinux-policy-devel</filename> or
-   <filename>selinux-policy</filename> RPM.)
+   <filename>selinux-policy</filename> package.)
    Once built, install this policy package using the
    <command>semodule</command> command, which loads supplied policy packages
    into the kernel.  If the package is correctly installed,
-- 
2.17.0


--6zdv2QT/q3FMhpsV
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v3-0008-style-and-consistency.patch"



^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* [PATCH v1 5/8] distro agnostic
@ 2020-09-23 03:55 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 16+ messages in thread

From: Justin Pryzby @ 2020-09-23 03:55 UTC (permalink / raw)

---
 doc/src/sgml/sepgsql.sgml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/src/sgml/sepgsql.sgml b/doc/src/sgml/sepgsql.sgml
index 9961569afc..15417bf19d 100644
--- a/doc/src/sgml/sepgsql.sgml
+++ b/doc/src/sgml/sepgsql.sgml
@@ -88,7 +88,7 @@ Policy from config file:        targeted
   <para>
    To build this module, include the option <literal>--with-selinux</literal> in
    your PostgreSQL <literal>configure</literal> command.  Be sure that the
-   <filename>libselinux-devel</filename> RPM is installed at build time.
+   <filename>libselinux-devel</filename> package is installed at build time.
   </para>
 
   <para>
@@ -177,7 +177,7 @@ $ for DBNAME in template0 template1 postgres; do
    Makefile on your system; the path shown below is only an example.
    (This Makefile is usually supplied by the
    <filename>selinux-policy-devel</filename> or
-   <filename>selinux-policy</filename> RPM.)
+   <filename>selinux-policy</filename> package.)
    Once built, install this policy package using the
    <command>semodule</command> command, which loads supplied policy packages
    into the kernel.  If the package is correctly installed,
-- 
2.17.0


--FFoLq8A0u+X9iRU8
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v1-0006-style-and-consistency.patch"



^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: tests fail on windows with default git settings
@ 2024-07-07 10:30 Andrew Dunstan <[email protected]>
  2024-07-08 20:16 ` Re: tests fail on windows with default git settings Andres Freund <[email protected]>
  0 siblings, 1 reply; 16+ messages in thread

From: Andrew Dunstan @ 2024-07-07 10:30 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; Andres Freund <[email protected]>; +Cc: pgsql-hackers; Dave Page <[email protected]>


On 2024-07-07 Su 1:26 AM, Tom Lane wrote:
> Andres Freund <[email protected]> writes:
>> Do we want to support checking out with core.autocrlf?
> -1.  It would be a constant source of breakage, and you could never
> expect that (for example) making a tarball from such a checkout
> would match anyone else's results.


Yeah, totally agree.


>> If we do not want to support that, ISTM we ought to raise an error somewhere?
> +1, if we can figure out how.
>
> 			



ISTM the right fix is probably to use PG_BINARY_R mode instead of "r" 
when opening the files, at least in the case if the test_json_parser tests.


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com







^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: tests fail on windows with default git settings
  2024-07-07 10:30 Re: tests fail on windows with default git settings Andrew Dunstan <[email protected]>
@ 2024-07-08 20:16 ` Andres Freund <[email protected]>
  2024-07-08 20:35   ` Re: tests fail on windows with default git settings Tom Lane <[email protected]>
  0 siblings, 1 reply; 16+ messages in thread

From: Andres Freund @ 2024-07-08 20:16 UTC (permalink / raw)
  To: Andrew Dunstan <[email protected]>; +Cc: Tom Lane <[email protected]>; pgsql-hackers; Dave Page <[email protected]>

On 2024-07-07 06:30:57 -0400, Andrew Dunstan wrote:
> 
> On 2024-07-07 Su 1:26 AM, Tom Lane wrote:
> > Andres Freund <[email protected]> writes:
> > > Do we want to support checking out with core.autocrlf?
> > -1.  It would be a constant source of breakage, and you could never
> > expect that (for example) making a tarball from such a checkout
> > would match anyone else's results.

> Yeah, totally agree.
> 
> 
> > > If we do not want to support that, ISTM we ought to raise an error somewhere?
> > +1, if we can figure out how.
> > 
> > 			
> 
> 
> 
> ISTM the right fix is probably to use PG_BINARY_R mode instead of "r" when
> opening the files, at least in the case if the test_json_parser tests.

That does seem like it'd fix this issue, assuming the parser can cope with
\r\n.

I'm actually mildly surprised that the tests don't fail when *not* using
autocrlf, because afaict test_json_parser_incremental.c doesn't set stdout to
binary and thus we presumably end up with \r\n in the output? Except that that
can't be true, because the test does pass on repos without autocrlf...


That approach does seem to mildly conflict with Tom and your preference for
fixing this by disallowing core.autocrlf? If we do so, the test never ought to
see a crlf?

Greetings,

Andres Freund






^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: tests fail on windows with default git settings
  2024-07-07 10:30 Re: tests fail on windows with default git settings Andrew Dunstan <[email protected]>
  2024-07-08 20:16 ` Re: tests fail on windows with default git settings Andres Freund <[email protected]>
@ 2024-07-08 20:35   ` Tom Lane <[email protected]>
  0 siblings, 0 replies; 16+ messages in thread

From: Tom Lane @ 2024-07-08 20:35 UTC (permalink / raw)
  To: Andres Freund <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; pgsql-hackers; Dave Page <[email protected]>

Andres Freund <[email protected]> writes:
> On 2024-07-07 06:30:57 -0400, Andrew Dunstan wrote:
>> ISTM the right fix is probably to use PG_BINARY_R mode instead of "r" when
>> opening the files, at least in the case if the test_json_parser tests.

> That approach does seem to mildly conflict with Tom and your preference for
> fixing this by disallowing core.autocrlf? If we do so, the test never ought to
> see a crlf?

Is this code that will *never* be applied to user-supplied files?
We certainly should tolerate \r\n in the general case (we even
have written-down project policy about that!).  While I wouldn't
complain too hard about assuming that our own test files don't
contain \r\n, if the code might get copied into a non-test
scenario then it could create problems later.

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 16+ messages in thread


end of thread, other threads:[~2024-07-08 20:35 UTC | newest]

Thread overview: 16+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2017-10-11 23:11 pgsql: Add configure infrastructure to detect support for C99's restric Andres Freund <[email protected]>
2017-10-12 00:13 ` Andres Freund <[email protected]>
2017-10-12 04:59   ` Andres Freund <[email protected]>
2017-10-12 05:05     ` Andres Freund <[email protected]>
2017-10-12 15:30     ` Tom Lane <[email protected]>
2017-10-12 17:45       ` Andres Freund <[email protected]>
2017-10-12 17:55         ` Tom Lane <[email protected]>
2017-10-12 18:03           ` Andres Freund <[email protected]>
2017-10-12 22:35             ` Andres Freund <[email protected]>
2017-10-12 23:14               ` Tom Lane <[email protected]>
2020-09-23 03:55 [PATCH v2 06/11] distro agnostic Justin Pryzby <[email protected]>
2020-09-23 03:55 [PATCH v3 07/12] distro agnostic Justin Pryzby <[email protected]>
2020-09-23 03:55 [PATCH v1 5/8] distro agnostic Justin Pryzby <[email protected]>
2024-07-07 10:30 Re: tests fail on windows with default git settings Andrew Dunstan <[email protected]>
2024-07-08 20:16 ` Re: tests fail on windows with default git settings Andres Freund <[email protected]>
2024-07-08 20:35   ` Re: tests fail on windows with default git settings Tom Lane <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox