ingydotnet / inline-c-pm

10 stars 19 forks source link

win32 specific Win32::IPC dependency missing #40

Closed rurban closed 5 years ago

rurban commented 9 years ago

With EUMM it would be trivial to add the required MSWin32 dependency Win32::IPC, but I have no idea how to add it your build system. Is there only Meta, or is there more?

It fails to build on windows smokers which do not have Win32::IPC installed. e.g. http://www.cpantesters.org/cpan/report/d3d8beb6-6dbd-1014-9f5e-99bfe3f2b8ca

wbraswell commented 5 years ago

@sisyphus How can we properly add Win32::IPC as a dependency of Inline::C? If you just tell me what to do, then I will create a pull request etc.

sisyphus commented 5 years ago

I don't know. Does anyone know what the problem actually is?

First thing I do whenever I've just built a new perl on Windows (eg 4 configurations of 5.29.7, as I did yesterday) is to run 'cpan -fi Inline::C'. And that always works fine for me.

I use force because there's a couple of Inline::C tests that fail near the end. Those failures don't bother me, and no-one else experiences them, so I'm quite content to leave them alone and keep using force.

So ... my first thought was Win32::IPC might require force, but that's not the case. I've just manually removed if from a couple of the 5.29.7 builds that I created yesterday and then successfully ran 'cpan -i Win32::IPC'.

Does anyone know the reason that Win32::IPC is not being installed on these Windows smokers when Inlline::C is built ?

Is it a deficiency in Inline::C ? ... or a deficiency in the smoking environment ?

Cheers, Rob

On Wed, Jan 23, 2019 at 5:43 AM William N. Braswell, Jr. < notifications@github.com> wrote:

@sisyphus https://github.com/sisyphus How can we properly add Win32::IPC as a dependency of Inline::C? If you just tell me what to do, then I will create a pull request etc.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ingydotnet/inline-c-pm/issues/40#issuecomment-456514820, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEdvHDINZR61TklJQ-elHCz5TKYAQ1uks5vF1u0gaJpZM4E68uV .

wbraswell commented 5 years ago

Of the 20 failing Win32 tests for Inline::C v0.78, 19 of them are due to the missing Win32::Mutex error: http://www.cpantesters.org/cpan/report/24fc8586-6bf9-1014-9fd4-c2aa8038f308 http://www.cpantesters.org/cpan/report/01033ab2-6bf6-1014-bc48-f1ee15f2f6ad http://www.cpantesters.org/cpan/report/fae85659-6ccc-1014-ac47-5aac8038f308 http://www.cpantesters.org/cpan/report/06e666c7-6c52-1014-86c5-018b64ad720a http://www.cpantesters.org/cpan/report/4e99b992-6d28-1014-85e9-b6b18038f308 http://www.cpantesters.org/cpan/report/3e435fc6-6c15-1014-b528-4657074be7a3 http://www.cpantesters.org/cpan/report/810d4307-6c70-1014-8575-838b271e9b05 http://www.cpantesters.org/cpan/report/1c2c0958-3212-11e8-9cbd-bb4c3e020320 http://www.cpantesters.org/cpan/report/cfc48e66-6c02-1014-a52f-72a58038f308 http://www.cpantesters.org/cpan/report/c54a39a2-bbe5-1014-9183-40e25f496936 http://www.cpantesters.org/cpan/report/1aea1bd9-6c0d-1014-be32-12aa8038f308 http://www.cpantesters.org/cpan/report/e64fe32f-6bfa-1014-bdbe-c0fdc3113a6d http://www.cpantesters.org/cpan/report/3e06b1b4-6c00-1014-b301-7e76bb4651c1 http://www.cpantesters.org/cpan/report/86ff5c91-6c87-1014-b695-2028cfb58d10 http://www.cpantesters.org/cpan/report/eeed342e-6cad-1014-9744-7aa68038f308 http://www.cpantesters.org/cpan/report/c0e2aa0a-6cd6-1014-a88b-3ee2d5a096b1 http://www.cpantesters.org/cpan/report/52d0a274-6bf5-1014-9acf-88d089f6a4c1 http://www.cpantesters.org/cpan/report/6b044406-6c52-1014-aa86-470c99043735 http://www.cpantesters.org/cpan/report/ff0ea72c-6bf9-1014-9005-4cf2c5b556df

Only 1 of the failing Win32 tests is due to something other than missing Win32::Mutex; it appears to have an old incompatible version of Inline installed which is not related to this issue: http://www.cpantesters.org/cpan/report/cbf280e9-6bf9-1014-bc51-f8d3665dec09

For Inline::C v0.78, there are 57 total Win32 tests, of which 37 are passing and (as mentioned) 20 are failing. We have 19/57 which is a 33% failure rate due only to missing Win32::Mutex. This seems to indicate that is is a relatively common occurrence that Win32::Mutex is NOT installed in Windows Perl by default, so we SHOULD be explicitly adding it to the Inline::C dependencies.

@sisyphus Is my reasoning valid here?

sisyphus commented 5 years ago

Your reasoning is quite valid. However, I think we've already explicitly added Win32::Mutex to the Inline::C dependencies for Windows via this entry in the Makefile.PL:

if ( $^O eq 'MSWin32' ) { $WriteMakefileArgs{PREREQ_PM}{'Win32::Mutex'} = '1.09'; }

And that's working fine for me. Every time I install Inline::C on a fresh build of perl it installs Win32::Mutex along with a heap of other dependencies.

How come its working fine for me, but not for smokers ? Is it because the smokers additionally require an entry for Win32::Mutex somewhere in one of the "META" files, as I think Reini might have been suggesting ?

Cheers, Rob

On Wed, Jan 23, 2019 at 1:59 PM William N. Braswell, Jr. < notifications@github.com> wrote:

Of the 20 failing Win32 tests for Inline::C v0.78, 19 of them are due to the missing Win32::Mutex error: http://www.cpantesters.org/cpan/report/24fc8586-6bf9-1014-9fd4-c2aa8038f308 http://www.cpantesters.org/cpan/report/01033ab2-6bf6-1014-bc48-f1ee15f2f6ad http://www.cpantesters.org/cpan/report/fae85659-6ccc-1014-ac47-5aac8038f308 http://www.cpantesters.org/cpan/report/06e666c7-6c52-1014-86c5-018b64ad720a http://www.cpantesters.org/cpan/report/4e99b992-6d28-1014-85e9-b6b18038f308 http://www.cpantesters.org/cpan/report/3e435fc6-6c15-1014-b528-4657074be7a3 http://www.cpantesters.org/cpan/report/810d4307-6c70-1014-8575-838b271e9b05 http://www.cpantesters.org/cpan/report/1c2c0958-3212-11e8-9cbd-bb4c3e020320 http://www.cpantesters.org/cpan/report/cfc48e66-6c02-1014-a52f-72a58038f308 http://www.cpantesters.org/cpan/report/c54a39a2-bbe5-1014-9183-40e25f496936 http://www.cpantesters.org/cpan/report/1aea1bd9-6c0d-1014-be32-12aa8038f308 http://www.cpantesters.org/cpan/report/e64fe32f-6bfa-1014-bdbe-c0fdc3113a6d http://www.cpantesters.org/cpan/report/3e06b1b4-6c00-1014-b301-7e76bb4651c1 http://www.cpantesters.org/cpan/report/86ff5c91-6c87-1014-b695-2028cfb58d10 http://www.cpantesters.org/cpan/report/eeed342e-6cad-1014-9744-7aa68038f308 http://www.cpantesters.org/cpan/report/c0e2aa0a-6cd6-1014-a88b-3ee2d5a096b1 http://www.cpantesters.org/cpan/report/52d0a274-6bf5-1014-9acf-88d089f6a4c1 http://www.cpantesters.org/cpan/report/6b044406-6c52-1014-aa86-470c99043735 http://www.cpantesters.org/cpan/report/ff0ea72c-6bf9-1014-9005-4cf2c5b556df

Only 1 of the failing Win32 tests is due to something other than missing Win32::Mutex; it appears to have an old incompatible version of Inline installed which is not related to this issue: http://www.cpantesters.org/cpan/report/cbf280e9-6bf9-1014-bc51-f8d3665dec09

For Inline::C v0.78, there are 57 total Win32 tests, of which 37 are passing and (as mentioned) 20 are failing. We have 19/57 which is a 33% failure rate due only to missing Win32::Mutex. This seems to indicate that is is a relatively common occurrence that Win32::Mutex is NOT installed in Windows Perl by default, so we SHOULD be explicitly adding it to the Inline::C dependencies.

@sisyphus https://github.com/sisyphus Is my reasoning valid here?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ingydotnet/inline-c-pm/issues/40#issuecomment-456650688, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEdvAElitLWIVI_khiB7muu6YbbdFc2ks5vF9AZgaJpZM4E68uV .

wbraswell commented 5 years ago

@ingydotnet @perlpunk Can you please give us the authoritative answer on where we need to put Inline::C dependencies, specifically Win32::Mutex?

rurban commented 5 years ago

This is a normal EUMM project, hence the deps need to be specified as in every other EUMM project. The PREREQ_PM array and optionally for the YAML and JSON generation the META entries. See perldoc ExtUtils::MakeMaker. Check if meta generation is broken by looking at the YAML and JSON files in the tarball.

Am Mi., 23. Jan. 2019, 06:25 hat William N. Braswell, Jr. < notifications@github.com> geschrieben:

@ingydotnet https://github.com/ingydotnet @perlpunk https://github.com/perlpunk Can you please give us the authoritative answer on where we need to put Inline::C dependencies, specifically Win32::Mutex?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ingydotnet/inline-c-pm/issues/40#issuecomment-456674702, or mute the thread https://github.com/notifications/unsubscribe-auth/AACjUf9DGs-7iOsM1diVhnknx_4QsiJwks5vF_IrgaJpZM4E68uV .

wbraswell commented 5 years ago

Win32::Mutex DOES appear in the following files:

https://github.com/ingydotnet/inline-c-pm/blob/master/Meta#L36-L38 (GitHub only, not on CPAN)

win32:
  requires:
    Win32::Mutex: 1.09

https://st.aticpan.org/source/TINITA/Inline-C-0.78/Makefile.PL (CPAN only, not on GitHub)

if ( $^O eq 'MSWin32' ) {
    $WriteMakefileArgs{PREREQ_PM}{'Win32::Mutex'} = '1.09';
}

Win32::Mutex does NOT appear in the following files:

https://st.aticpan.org/source/TINITA/Inline-C-0.78/META.json (CPAN only, not on GitHub)

https://st.aticpan.org/source/TINITA/Inline-C-0.78/META.yml (CPAN only, not on GitHub)

@rurban @sisyphus Does this mean there is an error in our CPAN release process which has failed to properly generate the "META.json" and "META.yml" files???

rurban commented 5 years ago

Exactly. I will look into that later, I'm away.

perlpunk commented 5 years ago

It looks to me like the META.yaml/META.json files are correct.

Win32::Mutex is only needed on Win32, but AFAIK CPAN::Meta currently has no way to express such dynamic dependencies. We can't put it into the static META files because we don't want this on other operating systems.

The only thing you can specify in META is dynamic_config: 1:

"A boolean flag indicating whether a Build.PL or Makefile.PL (or similar) must be executed to determine prerequisites." https://metacpan.org/pod/CPAN::Meta::Spec#dynamic_config

And the Makefile.PL has the dependency.

Both META.yaml/META.json have dynamic_config: 1, so I wonder why smokers don't adhere to that. https://metacpan.org/source/TINITA/Inline-C-0.78/META.json#L6 https://metacpan.org/source/TINITA/Inline-C-0.78/META.yml#L16

edit: Interesting is, that the dependency is actually recognized. Look into this report: http://www.cpantesters.org/cpan/report/99647050-6c1d-1014-ae7a-c367e3863dc7

Prerequisite modules loaded:

requires:

    Module                  Need     Have    
    ----------------------- -------- --------
    ExtUtils::MakeMaker     7.00     7.34    
    File::Spec              0.8      3.47    
    Inline                  0.79     0.80    
    Parse::RecDescent       1.967009 1.967009
    Pegex                   0.58     0.70    
    perl                    5.010000 5.012002
    Win32::Mutex            1.09     1.09    

And it's also in PERL5LIB: PERL5LIB = C:\strawberry\cpan\build\Win32-IPC-1.11-0/blib/arch;C:\strawberry\cpan\build\Win32-IPC-1.11-0/blib/lib;...

perlpunk commented 5 years ago

So PERL5LIB correctly contains Win32-IPC. But as we can see from the error message the directories reported in PERL5LIB get lost:

Can't locate Win32/Mutex.pm in @INC
 (@INC contains: C:\strawberry\cpan\build\Inline-0.80-0/blib/lib/
 C:\strawberry\cpan\build\Inline-C-0.79_001-0\blib\lib
 C:\strawberry\cpan\build\Inline-C-0.79_001-0\blib\arch
 C:\strawberry\cpan\build\Inline-0.80-0/blib/arch
 C:\strawberry\cpan\build\Inline-0.80-0/blib/lib
 C:/strawberry/perl/site/lib
 C:/strawberry/perl/site/lib
 C:/strawberry/perl/vendor/lib
 C:/strawberry/perl/lib .) at C:/strawberry/perl/lib/if.pm line 13.

So why does PERL5LIB get lost? I suspect this happens here: https://metacpan.org/source/TINITA/Inline-C-0.78/lib/Inline/C.pm#L880 as this is the only place where perl calls itself.

wbraswell commented 5 years ago

@perlpunk Wow great sleuthing! I don't have any way to test this myself, but perhaps @sisyphus or @rurban do?

sisyphus commented 5 years ago

Yes - PERL5LIB is certainly being blasted out of existence, though I can't yet see how that happens. On my Windows box I've moved Win32::Mutex to C:/sislib/lib, which is a directory that's in $ENV{PERL5LIB}. And I then get the same error as the smokers report.

I wondered whether it might be some quirk of using the 'if' module .... but it's not that.

I don't see that perl calling itself is an issue. The following just prints out the same (correct) @INC twice:

C:>perl -le "print for @INC; system 'perl', '-le', 'print for @INC';" C:/sislib/lib C:/_64/blead-5.29.7-p/site/lib C:/_64/blead-5.29.7-p/lib C:/sislib/lib C:/_64/blead-5.29.7-p/site/lib C:/_64/blead-5.29.7-p/lib

I'll keep looking as time permits.

Any thoughts ?

Cheers, Rob

On Fri, Jan 25, 2019 at 5:43 AM Tina Müller (tinita) < notifications@github.com> wrote:

So PERL5LIB correctly contains Win32-IPC. But as we can see from the error message the directories reported in PERL5LIB get lost:

Can't locate Win32/Mutex.pm in @INC (@INC contains: C:\strawberry\cpan\build\Inline-0.80-0/blib/lib/ C:\strawberry\cpan\build\Inline-C-0.79_001-0\blib\lib C:\strawberry\cpan\build\Inline-C-0.79_001-0\blib\arch C:\strawberry\cpan\build\Inline-0.80-0/blib/arch C:\strawberry\cpan\build\Inline-0.80-0/blib/lib C:/strawberry/perl/site/lib C:/strawberry/perl/site/lib C:/strawberry/perl/vendor/lib C:/strawberry/perl/lib .) at C:/strawberry/perl/lib/if.pm line 13.

So why does PERL5LIB get lost? I suspect this happens here: https://metacpan.org/source/TINITA/Inline-C-0.78/lib/Inline/C.pm#L880 as this is the only place where perl calls itself.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ingydotnet/inline-c-pm/issues/40#issuecomment-457310048, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEdvP3LBVcnwo5o_LvXILPETN_HWqFYks5vGf7FgaJpZM4E68uV .

wbraswell commented 5 years ago

@bulk88 Any thoughts on why PERL5LIB is getting lost here???

sisyphus commented 5 years ago

PERL5LIB gets clobbered in Inline.pm's create_config_file() at line 810:

local $ENV{PERL5LIB} if defined $ENV{PERL5LIB};

At least, if I comment out that line, everything goes ok and the test suite passes - except for t/27inline_maker.t, which always fail for me, irrespective.

Might we safely replace that line with:

unless($^O =~ /MSWin32/I) { local $ENV{PERL5LIB} if defined $ENV{PERL5LIB}; }

What might such a change break ? Who understands the point of localizing PERL5LIB ?

I notice that PERL5OPT is also localized. Might that bight us at some stage ?

Cheers, Rob

On Fri, Jan 25, 2019 at 1:31 PM William N. Braswell, Jr. < notifications@github.com> wrote:

@bulk88 https://github.com/bulk88 Any thoughts on why PERL5LIB is getting lost here???

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ingydotnet/inline-c-pm/issues/40#issuecomment-457433408, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEdvPmNgULRenQRJU8X3AtpxuRJHf_Iks5vGmxbgaJpZM4E68uV .

perlpunk commented 5 years ago

@sisyphus interesting. Maybe this code is somehow broken? https://metacpan.org/source/INGY/Inline-0.80/lib/Inline.pm#L822

my @_inc = map { "-I$_" }
($inline,
 grep {(-d File::Spec->catdir($_,"Inline") or
 -d File::Spec->catdir($_,"auto","Inline") or 
 -e File::Spec->catdir($_,"Parse/RecDescent.pm"))} @INC);
system $perl, @_inc, "-MInline=_CONFIG_", "-e1", "$dir"
...

As far as I can see this should actually replace the need for using PERL5LIB, but maybe Mutex.pm ends up somewhere else? Does it work if you add "Win32/Mutex.pm" to that grep?

Edit: but I also don't see why one would delete PERL5LIB and then use only specific directories from @INC. @ingydotnet any idea?

sisyphus commented 5 years ago

Aaah, yes ... adding or -e File::Spec->catdir($_,"Win32/Mutex.pm") to the grep conditions has the desired effect .... and also feels like the right thing to do ;-)

Pushed to git in commit 3b4ea7b92fdf23035000e09e829f22cc934b55c0

Seems a bit odd to be taking care of Inline::C issues inside the Inline module, but the relationship has always been a bit like that (for some definition of "bit like that"). Please feel free to revert/modify if what I've done is incorrect.

Nice work, Tina. Kudos to Will for making things happen.

Cheers, Rob.

On Fri, Jan 25, 2019 at 7:48 PM Tina Müller (tinita) < notifications@github.com> wrote:

@sisyphus https://github.com/sisyphus interesting. Maybe this code is somehow broken? https://metacpan.org/source/INGY/Inline-0.80/lib/Inline.pm#L822

my @inc = map { "-I$" } ($inline, grep {(-d File::Spec->catdir($,"Inline") or -d File::Spec->catdir($,"auto","Inline") or -e File::Spec->catdir($_,"Parse/RecDescent.pm"))} @INC); system $perl, @_inc, "-MInline=CONFIG", "-e1", "$dir" ...

As far as I can see this should actually replace the need for using PERL5LIB, but maybe Mutex.pm ends up somewhere else? Does it work if you add "Win32/Mutex.pm" to that grep?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ingydotnet/inline-c-pm/issues/40#issuecomment-457499494, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEdvNbkobI8Bk2qqSB1O3kuZOBeysNyks5vGsTXgaJpZM4E68uV .

mohawk2 commented 5 years ago

I expect Inline doesn't rely on PERL5LIB in case the build stage gets run later, with different environment. However, there is no earthly reason why it's doing the grep it's doing: it should capture the whole of @INC. @sisyphus, disagree?

wbraswell commented 5 years ago

@sisyphus @perlpunk great job with this fix!

Out of curiosity, why do we have the strategy of making case-by-case exceptions such as "Parse/RecDescent.pm" or "Win32/Mutex.pm", instead of just NOT clobbering PERL5LIB in the first place?

sisyphus commented 5 years ago

Do you mean that we could simply populate @_inc with:

my @inc = map { "-I$" } @INC;

That seems to be working fine for me, both inside and outside of Test::Harness. (At least it's fine with Inline::C - I haven't yet tried building Inline itself with that change.)

It assumes that everything that needs to be in @INC is already there - and that seems to be the case. I suspect the original grepping was done because there were some directories that were not in @INC, but needed to be included. (That now seems to NOT be the case. Is perl doing something differently now or was it always like that ?)

I'll test it over a range of perl versions (windows only) and get back.

Cheers, Rob

On Fri, Jan 25, 2019 at 11:52 PM mohawk2 notifications@github.com wrote:

I expect Inline doesn't rely on PERL5LIB in case the build stage gets run later, with different environment. However, there is no earthly reason why it's doing the grep it's doing: it should capture the whole of @INC. @sisyphus https://github.com/sisyphus, disagree?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ingydotnet/inline-c-pm/issues/40#issuecomment-457563727, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEdvLc7PMMm7Sf9lG7KdyZkX7CPuBt1ks5vGv4UgaJpZM4E68uV .

wbraswell commented 5 years ago

@sisyphus Yes I believe that is approximately what I am saying, although I do not know if this is actually correct or not...

sisyphus commented 5 years ago

That post of mine was written in response to mohawk2's. Looks like I managed to get it stuck in the wrong place in the queue ;-)

Anyway, the simplified method of assigning to @_inc with:

my @inc = map { "-I$" } @INC;

hasn't broken any Inline-80 or Inline-C-0.79_001 tests for me - tested on Windows for perls 5.10.0 through to 5.28.0.

So I don't mind if someone wants to commit that to git, or just leave us go down the path of adding Win32::Mutex to the grep list.

It would be nice to get rid of the localizing of PERL5LIB and that probably wouldn't break any tests either. But I can't help feeling that there's a valid case for localizing PERL5LIB under some usage case, even though I can't properly grasp just how that plays out.

So ... we need to get a new version of Inline released before we can get the Win32 smokers to start testing Inline::C.

Cheers, Rob

On Sat, Jan 26, 2019 at 1:19 PM William N. Braswell, Jr. < notifications@github.com> wrote:

@sisyphus https://github.com/sisyphus Yes I believe that is approximately what I am saying, although I do not know if this is actually correct or not...

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ingydotnet/inline-c-pm/issues/40#issuecomment-457792507, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEdvPOiql7n6W1FNN5L0v6t6XP3trquks5vG7sBgaJpZM4E68uV .

wbraswell commented 5 years ago

@sisyphus Okay sorry I got confused with the mixed up messages. :-P I vote for releasing the File::Spec->catdir($_,"Win32/Mutex.pm") code now on CPAN to see if it solves all the Win32 failures w/out causing any other unexpected failures. If all goes well, we could then try another CPAN testers release with the experimental generic my @_inc = map { "-I$_" } @inc; code, which could in theory cause unforeseen issues because we are disabling all the grep behavior w/out really understanding why. (Either way, we are already due for another CPAN dev release to test the latest fix for the still-pending include dir issue https://github.com/ingydotnet/inline-c-pm/issues/72)

perlpunk commented 5 years ago

Yes, this was also my question. Why don't we keep @INC completely and perform a grep? But just releasing the Win32::Mutex fix would help us now going forward with Inline::C, I agree.

@ingydotnet do you want me to do a release of Inline? Or who should & can do one? I would need a commit bit and CPAN comaint. I see there have been two dev releases in 2018 without a following regular release. We need a regular release so that smokers of Inline::C install it. Any reasons not to do one?

perlpunk commented 5 years ago

I just released 0.79_002. I guess it will take some more time until we can get an Inline.pm release out.

wbraswell commented 5 years ago

@sisyphus Do you still have a CPAN commit bit for Inline? If so, can you please make a CPAN dev release of Inline v0.80_03 w/ your latest commit https://github.com/ingydotnet/inline-pm/commit/3b4ea7b92fdf23035000e09e829f22cc934b55c0 ? (I assume it is preferred to make a CPAN dev release first to make sure the "Win32::Mutex" code does not accidentally break anything, then we can make a full CPAN release in a day or two if all is well.)

sisyphus commented 5 years ago

Seems that I have commit bits for both Inline and Inline::C, but I'm not going to get involved in making releases.

Cheers, Rob

On Sun, Jan 27, 2019 at 5:37 AM William N. Braswell, Jr. < notifications@github.com> wrote:

@sisyphus https://github.com/sisyphus Do you still have a CPAN commit bit for Inline? If so, can you please make a CPAN dev release of Inline v0.80_03 w/ your latest commit ingydotnet/inline-pm@3b4ea7b https://github.com/ingydotnet/inline-pm/commit/3b4ea7b92fdf23035000e09e829f22cc934b55c0 ?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ingydotnet/inline-c-pm/issues/40#issuecomment-457854604, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEdvGrEWqreKxWVOzoU9baU3dtr7cx8ks5vHKBNgaJpZM4E68uV .

wbraswell commented 5 years ago

Inline v0.81 has been released, which includes this commit: https://github.com/ingydotnet/inline-pm/commit/3b4ea7b92fdf23035000e09e829f22cc934b55c0

@sisyphus says "Inline-0.81 looking fine here - and enabling Win32::Mutex to be found via PER5LIB on Windows." https://github.com/ingydotnet/inline-c-pm/issues/72#issuecomment-460187963

Does this mean we can close this issue as being correctly resolved? How do we know for sure if the issue is actually solved?

sisyphus commented 5 years ago

The only indication that it's solved will be the absence of the error. However, the fix is tested only when someone relies on PERL5LIB to locate Win32::Mutex - and I'm certainly not interested in creating a test script that emulates those conditions.

I did wonder whether similar problems might arise if the Pegex modules were being located via PERL5LIB, but my testing on Windows 10 indicates there's no such problem.

Probably should wait 'til at least the next stable release of Inline::C has been tested by the affected smoker(s) before closing.

Cheers, Rob

On Tue, Feb 19, 2019 at 3:57 PM William N. Braswell, Jr. < notifications@github.com> wrote:

Inline v0.81 has been released, which includes this commit: ingydotnet/inline-pm@3b4ea7b https://github.com/ingydotnet/inline-pm/commit/3b4ea7b92fdf23035000e09e829f22cc934b55c0

@sisyphus https://github.com/sisyphus says "Inline-0.81 looking fine here - and enabling Win32::Mutex to be found via PER5LIB on Windows."

72 (comment)

https://github.com/ingydotnet/inline-c-pm/issues/72#issuecomment-460187963

Does this mean we can close this issue as being correctly resolved? How do we know for sure if the issue is actually solved?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ingydotnet/inline-c-pm/issues/40#issuecomment-464983004, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEdvHZxa16b1Jk_6xlDtdi74avcXNZ0ks5vO4RAgaJpZM4E68uV .

wbraswell commented 5 years ago

@sisyphus Okay sounds like a good plan! :-)

mohawk2 commented 5 years ago

Can this be closed?

wbraswell commented 5 years ago

I think the plan is to keep this issue open until Inline::C 0.79 is fully released on CPAN and we have waited a sufficient period of time for CPAN testers to report errors.

perlpunk commented 5 years ago

Ok, so I think it can be closed :)

wbraswell commented 5 years ago

Yes I agree this issue can be closed for now, so far all Win32 tests are passing for Inline 0.80: http://matrix.cpantesters.org/?dist=Inline-C%200.80;os=mswin32;reports=1