subject:"Re\: Remove MSVC scripts from the tree"

Re: Remove MSVC scripts from the tree

2023-12-23 Thread Michael Paquier

On Fri, Dec 22, 2023 at 09:07:21AM -0500, Andrew Dunstan wrote:
> I've done it a bit differently, but the same idea. I have tested that what I
> committed passes checks on Unix and works on Windows.

Sounds fine by me.  Thanks for the quick turnaround!
--
Michael


signature.asc
Description: PGP signature

Re: Remove MSVC scripts from the tree

2023-12-22 Thread Andrew Dunstan




On 2023-12-21 Th 18:20, Michael Paquier wrote:

On Thu, Dec 21, 2023 at 03:43:32PM -0500, Andrew Dunstan wrote:

On 2023-12-21 Th 03:01, Michael Paquier wrote:

Andrew, was the original target of pgperlsyncheck committers and
hackers who played with the MSVC scripts but could not run sanity
checks on Windows (see [1])?


yes.

Okay, thanks.  Wouldn't it be better to remove it at the end?  With
the main use case behind its introduction being gone, it is less
attractive to keep maintaining it.  If some people have been using it
in their workflows, I'm OK to keep it but the rest of the tree can be
checked at runtime as well.


I'm actually a bit dubious about win32tzlist.pl. Win32::Registry is not
present in a recent Strawberry Perl installation, and its latest version
says it is obsolete, although it's still included in the cpan bundle
libwin32.

I wonder who has actually run the script any time recently?

Hmm...  I've never run it with meson on Win32.



Turns out I was wrong - Windows sometimes doesn't find files nicely. It 
is present in my Strawberry installation.






In any case, we can probably work around the syncheck issue by making the
module a runtime requirement rather than a compile time requirement, by
using "require" instead of "use".

Interesting.  Another trick would be needed for HKEY_LOCAL_MACHINE,
like what the dummylib but local to win32tzlist.pl.  Roughly among
these lines:
-use Win32::Registry;
+use Config;
+
+require Win32::Registry;
  
  my $tzfile = 'src/bin/initdb/findtimezone.c';
  
+if ($Config{osname} ne 'MSWin32' && $Config{osname} ne 'msys')

+{
+   use vars qw($HKEY_LOCAL_MACHINE);
+}



I've done it a bit differently, but the same idea. I have tested that 
what I committed passes checks on Unix and works on Windows.



cheers


andrew


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

Re: Remove MSVC scripts from the tree

2023-12-21 Thread Michael Paquier

On Thu, Dec 21, 2023 at 03:43:32PM -0500, Andrew Dunstan wrote:
> On 2023-12-21 Th 03:01, Michael Paquier wrote:
>> Andrew, was the original target of pgperlsyncheck committers and
>> hackers who played with the MSVC scripts but could not run sanity
>> checks on Windows (see [1])?
> 
> 
> yes.

Okay, thanks.  Wouldn't it be better to remove it at the end?  With
the main use case behind its introduction being gone, it is less
attractive to keep maintaining it.  If some people have been using it
in their workflows, I'm OK to keep it but the rest of the tree can be
checked at runtime as well.

> I'm actually a bit dubious about win32tzlist.pl. Win32::Registry is not
> present in a recent Strawberry Perl installation, and its latest version
> says it is obsolete, although it's still included in the cpan bundle
> libwin32.
> 
> I wonder who has actually run the script any time recently?

Hmm...  I've never run it with meson on Win32.

> In any case, we can probably work around the syncheck issue by making the
> module a runtime requirement rather than a compile time requirement, by
> using "require" instead of "use".

Interesting.  Another trick would be needed for HKEY_LOCAL_MACHINE,
like what the dummylib but local to win32tzlist.pl.  Roughly among
these lines:
-use Win32::Registry;
+use Config;
+
+require Win32::Registry;
 
 my $tzfile = 'src/bin/initdb/findtimezone.c';
 
+if ($Config{osname} ne 'MSWin32' && $Config{osname} ne 'msys')
+{
+   use vars qw($HKEY_LOCAL_MACHINE);
+}
--
Michael


signature.asc
Description: PGP signature

Re: Remove MSVC scripts from the tree

2023-12-21 Thread Andrew Dunstan

On 2023-12-21 Th 03:01, Michael Paquier wrote:

On Wed, Dec 20, 2023 at 11:39:15PM -0800, Andres Freund wrote:

Can't we teach the tool that it should not validate src/tools/win32tzlist.pl
on !windows? It's obviously windows specific code, and it's special case
enough that there doesn't seem like a need to develop it on !windows.

I am not really excited about keeping a dummy library for the sake of
a script checking if this WIN32-only file is correctly written, and
I've never used pgperlsyncheck, TBH, since it exists in af616ce48347.
Anyway, we could just tweak the list of files returned by
find_perl_files as win32tzlist.pl is valid for perltidy and
perlcritic.

Andrew, was the original target of pgperlsyncheck committers and
hackers who played with the MSVC scripts but could not run sanity
checks on Windows (see [1])?

yes.

There are a few more cases like the
Unicode scripts or some of the stuff in src/tools/ where that can be
useful still these are not touched on a daily basis. The rest of the
pm files are for TAP tests, one for Unicode. I'm OK to tweak the
script, still, if its main purpose is gone..

[1]:
https://www.postgresql.org/message-id/f3c12e2c-618f-cb6f-082b-a2f604dbe...@2ndquadrant.com

I'm actually a bit dubious about win32tzlist.pl. Win32::Registry is not
present in a recent Strawberry Perl installation, and its latest version
says it is obsolete, although it's still included in the cpan bundle
libwin32.

I wonder who has actually run the script any time recently?

In any case, we can probably work around the syncheck issue by making
the module a runtime requirement rather than a compile time requirement,
by using "require" instead of "use".

68 matches

Mail list logo