pgsql: Skip some permissions checks on Cygwin

Started by Andrew Dunstanabout 2 years ago7 messagescomitters

Jump to latest

Andrew Dunstan

andrew@dunslane.net

about 2 years ago

Skip some permissions checks on Cygwin

These are checks that are already skipped on other Windows systems.

Backpatch to all live branches, as appropriate.

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/f83908798f78c4cafda217ca875602c88ea2ae28

Modified Files
--------------
src/bin/initdb/t/001_initdb.pl | 2 +-
src/bin/pg_basebackup/t/010_pg_basebackup.pl | 2 +-
src/bin/pg_ctl/t/001_start_stop.pl | 3 ++-
src/bin/pg_rewind/t/002_databases.pl | 2 +-
src/bin/pg_verifybackup/t/003_corruption.pl | 3 ++-
5 files changed, 7 insertions(+), 5 deletions(-)

Noah Misch

noah@leadboat.com

about 2 years ago

In reply to: Andrew Dunstan (#1)

Re: pgsql: Skip some permissions checks on Cygwin

On Thu, Jun 13, 2024 at 12:12:29PM +0000, Andrew Dunstan wrote:

Skip some permissions checks on Cygwin

These are checks that are already skipped on other Windows systems.

skip "unix-style permissions not supported on Windows", 2
-	  if ($windows_os);
+	  if ($windows_os || $Config::Config{osname} eq 'cygwin');

-	skip "group access not supported on Windows", 3 if ($windows_os);
+	skip "group access not supported on Windows", 3
+	  if ($windows_os || $Config::Config{osname} eq 'cygwin');

Cygwin does support Unix-style permissions, so I'm not following the rationale
for this change. Can you say more?

Andrew Dunstan

andrew@dunslane.net

about 2 years ago

In reply to: Noah Misch (#2)

Re: pgsql: Skip some permissions checks on Cygwin

On 2024-06-23 Su 9:03 PM, Noah Misch wrote:

On Thu, Jun 13, 2024 at 12:12:29PM +0000, Andrew Dunstan wrote:
Skip some permissions checks on Cygwin
These are checks that are already skipped on other Windows systems.
skip "unix-style permissions not supported on Windows", 2
-	  if ($windows_os);
+	  if ($windows_os || $Config::Config{osname} eq 'cygwin');
-	skip "group access not supported on Windows", 3 if ($windows_os);
+	skip "group access not supported on Windows", 3
+	  if ($windows_os || $Config::Config{osname} eq 'cygwin');
Cygwin does support Unix-style permissions, so I'm not following the rationale
for this change. Can you say more?

It seems to have some difficulties with group permissions in some cases.
I improved the error message a bit, and got a bunch that all look like this:

build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/global/6303
mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/global/pg_control
mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/global/pg_filenode.map
mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/base/5
mode must be 0750, not 0000000000000000000000000000000000000700
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/base/5/112
mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/base/5/113
mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/base/5/1247
mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/base/5/1247_fsm
mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/base/5/1247_vm
mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/base/5/1249
mode must be 0640, not 0600

Apparently there's some issue with the group None. I don't mind digging
further if you think it might be worthwhile, but it seemed to me that
since we didn't mind too much skipping these checks on native Windows we
wouldn't mind on Cygwin either.

Up to now we haven't been covering Cygwin with TAP tests, and I'd like
to change that.

cheers

andrew

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

Noah Misch

noah@leadboat.com

about 2 years ago

In reply to: Andrew Dunstan (#3)

Re: pgsql: Skip some permissions checks on Cygwin

On Mon, Jun 24, 2024 at 10:17:32AM -0400, Andrew Dunstan wrote:

On 2024-06-23 Su 9:03 PM, Noah Misch wrote:
On Thu, Jun 13, 2024 at 12:12:29PM +0000, Andrew Dunstan wrote:
Skip some permissions checks on Cygwin
These are checks that are already skipped on other Windows systems.
skip "unix-style permissions not supported on Windows", 2
-	  if ($windows_os);
+	  if ($windows_os || $Config::Config{osname} eq 'cygwin');
-	skip "group access not supported on Windows", 3 if ($windows_os);
+	skip "group access not supported on Windows", 3
+	  if ($windows_os || $Config::Config{osname} eq 'cygwin');
Cygwin does support Unix-style permissions, so I'm not following the rationale
for this change. Can you say more?
It seems to have some difficulties with group permissions in some cases. I
improved the error message a bit, and got a bunch that all look like this:

build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/global/6303
mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/global/pg_control
mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/global/pg_filenode.map
mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/base/5
mode must be 0750, not 0000000000000000000000000000000000000700
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/base/5/112
mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/base/5/113
mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/base/5/1247
mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/base/5/1247_fsm
mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/base/5/1247_vm
mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/base/5/1249
mode must be 0640, not 0600

Apparently there's some issue with the group None. I don't mind digging
further if you think it might be worthwhile, but it seemed to me that since

I'll guess digging further is not worthwhile. Even if it is a PostgreSQL bug,
the consequences seem limited to unintentionally withholding group access.
There's a remote chance you're using a "noacl" mount, where this result could
fit.

we didn't mind too much skipping these checks on native Windows we wouldn't
mind on Cygwin either.

The reasons for skipping the test are different on native Windows (does not
provide POSIX file modes) vs. Cygwin (has POSIX file modes, but the
combination of PostgreSQL and Cygwin somehow reaches the wrong mode).

Up to now we haven't been covering Cygwin with TAP tests, and I'd like to
change that.

Thanks for doing that.

Andrew Dunstan

andrew@dunslane.net

about 2 years ago

In reply to: Noah Misch (#4)

Re: pgsql: Skip some permissions checks on Cygwin

On 2024-06-24 Mo 4:42 PM, Noah Misch wrote:

On Mon, Jun 24, 2024 at 10:17:32AM -0400, Andrew Dunstan wrote:
On 2024-06-23 Su 9:03 PM, Noah Misch wrote:
On Thu, Jun 13, 2024 at 12:12:29PM +0000, Andrew Dunstan wrote:
Skip some permissions checks on Cygwin
These are checks that are already skipped on other Windows systems.
skip "unix-style permissions not supported on Windows", 2
-	  if ($windows_os);
+	  if ($windows_os || $Config::Config{osname} eq 'cygwin');
-	skip "group access not supported on Windows", 3 if ($windows_os);
+	skip "group access not supported on Windows", 3
+	  if ($windows_os || $Config::Config{osname} eq 'cygwin');
Cygwin does support Unix-style permissions, so I'm not following the rationale
for this change. Can you say more?
It seems to have some difficulties with group permissions in some cases. I
improved the error message a bit, and got a bunch that all look like this:

build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/global/6303
mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/global/pg_control
mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/global/pg_filenode.map
mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/base/5
mode must be 0750, not 0000000000000000000000000000000000000700
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/base/5/112
mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/base/5/113
mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/base/5/1247
mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/base/5/1247_fsm
mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/base/5/1247_vm
mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/base/5/1249
mode must be 0640, not 0600

Apparently there's some issue with the group None. I don't mind digging
further if you think it might be worthwhile, but it seemed to me that since
I'll guess digging further is not worthwhile. Even if it is a PostgreSQL bug,
the consequences seem limited to unintentionally withholding group access.
There's a remote chance you're using a "noacl" mount, where this result could
fit.

mount doesn't show noacl being used

we didn't mind too much skipping these checks on native Windows we wouldn't
mind on Cygwin either.

The reasons for skipping the test are different on native Windows (does not
provide POSIX file modes) vs. Cygwin (has POSIX file modes, but the
combination of PostgreSQL and Cygwin somehow reaches the wrong mode).

True.

I wonder if Marco can shed any light on what's going on here.

cheers

andrew

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

Marco Atzeri

marco.atzeri@gmail.com

about 2 years ago

In reply to: Andrew Dunstan (#5)

Re: pgsql: Skip some permissions checks on Cygwin

On 24/06/2024 22:58, Andrew Dunstan wrote:

On 2024-06-24 Mo 4:42 PM, Noah Misch wrote:
On Mon, Jun 24, 2024 at 10:17:32AM -0400, Andrew Dunstan wrote:
On 2024-06-23 Su 9:03 PM, Noah Misch wrote:
On Thu, Jun 13, 2024 at 12:12:29PM +0000, Andrew Dunstan wrote:
Skip some permissions checks on Cygwin
These are checks that are already skipped on other Windows systems.
       skip "unix-style permissions not supported on Windows", 2
-      if ($windows_os);
+      if ($windows_os || $Config::Config{osname} eq 'cygwin');
-    skip "group access not supported on Windows", 3 if ($windows_os);
+    skip "group access not supported on Windows", 3
+      if ($windows_os || $Config::Config{osname} eq 'cygwin');
Cygwin does support Unix-style permissions, so I'm not following the
rationale
for this change. Can you say more?
It seems to have some difficulties with group permissions in some
cases. I
improved the error message a bit, and got a bunch that all look like
this:

build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/global/6303
mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/global/pg_control
mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/global/pg_filenode.map
mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/base/5
mode must be 0750, not 0000000000000000000000000000000000000700
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/base/5/112
mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/base/5/113
mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/base/5/1247
mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/base/5/1247_fsm
mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/base/5/1247_vm
mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/base/5/1249
mode must be 0640, not 0600

Apparently there's some issue with the group None. I don't mind digging
further if you think it might be worthwhile, but it seemed to me that
since
I'll guess digging further is not worthwhile. Even if it is a
PostgreSQL bug,
the consequences seem limited to unintentionally withholding group
access.
There's a remote chance you're using a "noacl" mount, where this
result could
fit.
mount doesn't show noacl being used

we didn't mind too much skipping these checks on native Windows we
wouldn't
mind on Cygwin either.

The reasons for skipping the test are different on native Windows
(does not
provide POSIX file modes) vs. Cygwin (has POSIX file modes, but the
combination of PostgreSQL and Cygwin somehow reaches the wrong mode).

True.

I wonder if Marco can shed any light on what's going on here.

cheers

andrew

are you referring to a specific version or is an old issue ?

Regards
Marco

Andrew Dunstan

andrew@dunslane.net

about 2 years ago

In reply to: Marco Atzeri (#6)

Re: pgsql: Skip some permissions checks on Cygwin

On 2024-06-24 Mo 6:43 PM, Marco Atzeri wrote:

On 24/06/2024 22:58, Andrew Dunstan wrote:
On 2024-06-24 Mo 4:42 PM, Noah Misch wrote:
On Mon, Jun 24, 2024 at 10:17:32AM -0400, Andrew Dunstan wrote:
On 2024-06-23 Su 9:03 PM, Noah Misch wrote:
On Thu, Jun 13, 2024 at 12:12:29PM +0000, Andrew Dunstan wrote:
Skip some permissions checks on Cygwin
These are checks that are already skipped on other Windows systems.
       skip "unix-style permissions not supported on Windows", 2
-      if ($windows_os);
+      if ($windows_os || $Config::Config{osname} eq 'cygwin');
-    skip "group access not supported on Windows", 3 if 
($windows_os);
+    skip "group access not supported on Windows", 3
+      if ($windows_os || $Config::Config{osname} eq 'cygwin');
Cygwin does support Unix-style permissions, so I'm not following
the rationale
for this change. Can you say more?
It seems to have some difficulties with group permissions in some
cases. I
improved the error message a bit, and got a bunch that all look
like this:

build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/global/6303

mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/global/pg_control

mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/global/pg_filenode.map

mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/base/5

mode must be 0750, not 0000000000000000000000000000000000000700
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/base/5/112

mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/base/5/113

mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/base/5/1247

mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/base/5/1247_fsm

mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/base/5/1247_vm

mode must be 0640, not 0600
build/testrun/initdb/001_initdb/log/regress_log_001_initdb:*main::STDERR/home/Administrator/postgresql/build/testrun/initdb/001_initdb/data/tmp_test_qlhf/data_group/base/5/1249

mode must be 0640, not 0600

Apparently there's some issue with the group None. I don't mind
digging
further if you think it might be worthwhile, but it seemed to me
that since
I'll guess digging further is not worthwhile. Even if it is a
PostgreSQL bug,
the consequences seem limited to unintentionally withholding group
access.
There's a remote chance you're using a "noacl" mount, where this
result could
fit.
mount doesn't show noacl being used

we didn't mind too much skipping these checks on native Windows we
wouldn't
mind on Cygwin either.

The reasons for skipping the test are different on native Windows
(does not
provide POSIX file modes) vs. Cygwin (has POSIX file modes, but the
combination of PostgreSQL and Cygwin somehow reaches the wrong mode).

True.

I wonder if Marco can shed any light on what's going on here.

cheers

andrew
are you referring to a specific version or is an old issue ?

This is probably an old issue. We haven't been running TAP tests on
Cygwin (at least in the buildfarm) so the issue hasn't surfaced before now.

cheers

andrew

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