Thu May 24 18:12:51 2012: Request 76368 was acted upon.
Transaction: Correspondence added by j...@activestate.com
       Queue: Win32
     Subject: RE: [rt.cpan.org #76368] rewrote Win32 
   Broken in: 0.44
    Severity: Wishlist
       Owner: Nobody
  Requestors: bul...@hotmail.com
      Status: new
 Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=76368 >


On Sat, 07 Apr 2012, patcat via RT wrote:

Sorry for the long delay; I've been traveling for a month and am just
catching up with some things.
 
>  Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=76368 >
> 
> The Win32 library's XS code looked very bloated to me. So I rewrote it.

Thank you for your effort! I would like to split up your big patch into
several smaller ones, each with just a single purpose. Ideally you
would create a serious of commits on github and just issue a pull
request against:

https://github.com/jandubois/win32

However, for some parts of your patch I need some convincing that they
are correct/necessary:

+0.45   [2012-??-??]
+       - massive refactoring, to reduce code bloat, DLL is 24KB vs 33KB in 
+         0.44, using 32bit Perl 5.10 on VC 2003, with default -01

I'm not concerned about "bloat" when the total size is just 33KB.
Reducing the size is fine, but should not be done at the cost of making
the code harder to maintain. Which part of your patch is responsible for
the size reduction?

+       - removed the CRT

I don't understand the point of this. Perl itself already loads in a
CRT, so why wouldn't we use it? Having code that is part of the CRT
repeated in the module seems like bloat to me. :)

+       - mentioned the GetLastError is always invalid bug

ok

+       - mentioned 32KB Unicode path support is untested

It can be mentioned, but there are a lot of issues with Unicode filename
support in Perl for Windows.

The general status of Unicode support in the Win32::* modules is quite
a mess. Maybe we should discuss this in a separate thread on the
mailing list.

+       - mentioned what does return a valid last error

ok

+       - added delay loading of version.dll and ole32.dll on VC and GCC Perl

ok

+       - compiles with PERL_CORE now

Compiling modules with PERL_CORE is just wrong. Only code that goes into
perl5xx.dll should ever be compiled with PERL_CORE. I understand that
some modules would define PERL_CORE anyways because using it gave you a
speed advantage. This speed advantage no longer exists in 5.14 and
later. So I don't want to support the misuse of PERL_CORE in Win32,
given that it would only benefit unsupported legacy releases of Perl.

+       - fixed GetCurrentThreadId tests that were broken on DOS Windows

ok. I don't particularly care to keep Win9X compatibility in future
Win32 releases (because it will stand in the way of improving
Unicode support), but am fine with small changes to keep it working
on Win9X for now.

+       - fixed handle leak in Spawn

thanks!

I'm also happy to apply your improvements to the regression tests. Let
me know if you want to provide them in a separate patch, or if I should
just extract them from current one!

Cheers,
-Jan



Reply via email to