Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cpan/Win32: import PR 39 #22393

Merged
merged 1 commit into from
Jul 17, 2024
Merged

cpan/Win32: import PR 39 #22393

merged 1 commit into from
Jul 17, 2024

Conversation

tonycoz
Copy link
Contributor

@tonycoz tonycoz commented Jul 11, 2024

perl-libwin32/win32#37 was submitted in June 2022, it fixes a bug that causes tests to fail for Win32.pm for 32-bit builds on windows, including in perl (and in 5.40.)

Jan requested a minor change to that PR which wasn't followed up on, so I submitted perl-libwin32/win32#39 which includes that requested change in August 2023.

This still hasn't been acted on.

So import this change into blead.

perl-libwin32/win32#37 was submitted in June
2022, it fixes a bug that causes tests to fail for Win32.pm for
32-bit builds on windows, including in perl (and in 5.40.)

Jan requested a minor change to that PR which wasn't followed up on, so
I submitted perl-libwin32/win32#39 which
includes that requested change in August 2023.

This still hasn't been acted on.

So import this change into blead.
@jkeenan
Copy link
Contributor

jkeenan commented Jul 11, 2024

perl-libwin32/win32#37 was submitted in June 2022, it fixes a bug that causes tests to fail for Win32.pm for 32-bit builds on windows, including in perl (and in 5.40.)

Jan requested a minor change to that PR which wasn't followed up on, so I submitted perl-libwin32/win32#39 which includes that requested change in August 2023.

This still hasn't been acted on.

So import this change into blead.

I don't have access to, or good knowledge of, Win32, but I'm willing to trust @tonycoz on this. Question: do we have access to any machine that can give us before/after test results once this p.r. is merged?

@sisyphus
Copy link
Contributor

Question: do we have access to any machine that can give us before/after test results once this p.r. is merged?

Good question.
I've been building 32-bit builds (both mingw-w64 and Visual Studio 2022) of all perl releases beginning about perl-5.37.2, and I've not ever struck any Win32.pm test failures at all.
For the older builds, I was on Windows 10; more recently I've switched to Windows 11.

I think we can pretty much take it for granted that this PR is not going to do anything to alter the behaviour of the 32-bit builds on my system - but I tested this PR last night anyway, just to be sure.

Since I seem to be insulated from this problem, there's probably not much more I can do.
I wouldn't be surprised if the smokers and CI are unable to provide more rigorous testing - though I don't really know exactly what it takes reproduce this issue.

@tonycoz
Copy link
Contributor Author

tonycoz commented Jul 15, 2024

I've been building 32-bit builds (both mingw-w64 and Visual Studio 2022) of all perl releases beginning about perl-5.37.2, and I've not ever struck any Win32.pm test failures at all.

This is the problem I see:

../cpan/Win32/t/Privileges.t ..
1..7
# Running under perl version 5.041002 for MSWin32
# Current time local: Mon Jul 15 16:10:53 2024
# Current time GMT:   Mon Jul 15 06:10:53 2024
# Using Test.pm version 1.31
ok 1
ok 2
ok 3
ok 4
ok 5
ok 6
# Failed test 7 in t/Privileges.t at line 54
not ok 7
Failed 1/7 subtests

Test Summary Report
-------------------
../cpan/Win32/t/Privileges.t (Wstat: 0 Tests: 7 Failed: 1)
  Failed test:  7

It doesn't happen in a privileged shell, which I suspect is why it doesn't happen if I add an x86-32 Win32 build to CI.

This was VS2019, but I've see it in VS2022, tried to track it down and found the already open bug (and fix).

Are your builds in an admin shell?

@sisyphus
Copy link
Contributor

Are your builds in an admin shell?

No, and $Config{d_symlink} is defined.
But Win32::IsSymlinkCreationAllowed() returns false; yet all 7 tests pass.

From previous experience, I think there is a way of configuring the capability of creating symlinks on this system (without running with admin privileges).
I'll give it a try if I can rediscover the way to do that.

In any case, it seems that the problem has now been fixed, and proven to be fixed.

@tonycoz
Copy link
Contributor Author

tonycoz commented Jul 15, 2024

From previous experience, I think there is a way of configuring the capability of creating symlinks on this system (without running with admin privileges).
I'll give it a try if I can rediscover the way to do that.

The way you allow normal users to create symlinks on a sufficiently recent Windows is to enable developer mode. (Control Panel, Updates and Security, For developers, Developer mode)

I suspect in your case you have developer mode disabled, so the test correctly detects symlinks are disabled (because it always detects symlinks as disabled on 32-bit), and successfully tests that symlinks are disabled, by failing to create a symlink.

Without the fix, the developer mode test is broken and detects developer mode is disabled when it is enabled, tries to create a symlink to check that symlinks are disabled, but successfully creates one, failing the test.

@sisyphus
Copy link
Contributor

Thanks @tonycoz.
For my particular Windows 11 box, if I want to avoid running in a privileged shell, I can allow symlink creation by doing:
Start -> Settings -> System -> For Developers, then toggle the "Developer Mode" switch to on. (I couldn't find the path to follow if I started in Control Panel.)
That done, I see the same as you.
And I see the same issue on mingw-64 builds, too. Again. it's only the 32-bit builds that are affected.

@tonycoz
Copy link
Contributor Author

tonycoz commented Jul 16, 2024

Does my fix allow the test to pass for you on 32-bit? (git fetch origin pull/22393/head:fix-win32-priv ; git checkout fix-win32-priv ; ... test)

@sisyphus
Copy link
Contributor

Does my fix allow the test to pass for you on 32-bit?

Yes,
(Tested on both mingw-w64 and VS2022 builds.)

@tonycoz tonycoz merged commit 08962c5 into Perl:blead Jul 17, 2024
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants