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

Change #getTempPath:buffer: on WinPlatform to use ‘GetTempPathW’ instead of ‘GetTempPath2W’ #16342

Merged
merged 5 commits into from
Apr 2, 2024

Conversation

Rinzwind
Copy link
Contributor

This pull request changes #getTempPath:buffer: on WinPlatform to use ‘GetTempPathW’ instead of ‘GetTempPath2W’ for compatibility with versions of Windows older than ‘Windows 10 Build 20348’ or ‘Windows Server Build 20348’.

See the ‘Requirements’ for ‘GetTempPathW’, the ‘Requirements’ for ‘GetTempPath2W’ and my earlier comment in pull request #13777.

The documentation for ‘GetTempPath2W’ indicates it is equivalent to ‘GetTempPathW’ for non-SYSTEM processes: “When calling this function from a process running as SYSTEM it will return the path C:\Windows\SystemTemp, which is inaccessible to non-SYSTEM processes. For non-SYSTEM processes, GetTempPath2 will behave the same as GetTempPath.” As for how ‘GetTempPath2’ is related to ‘GetTempPath2W’: “The fileapi.h header defines GetTempPath2 as an alias which automatically selects the ANSI or Unicode version of this function based on the definition of the UNICODE preprocessor constant. […] For more information, see Conventions for Function Prototypes.”

@Rinzwind Rinzwind closed this Mar 24, 2024
@Rinzwind Rinzwind reopened this Mar 24, 2024
@Rinzwind Rinzwind closed this Mar 24, 2024
@Rinzwind Rinzwind reopened this Mar 24, 2024
@Rinzwind Rinzwind closed this Mar 24, 2024
@Rinzwind Rinzwind reopened this Mar 24, 2024
@Rinzwind Rinzwind marked this pull request as draft March 24, 2024 16:42
@Rinzwind
Copy link
Contributor Author

I converted this to a draft. I expect this to fix the issue of the Windows test run timing out (issue #15104), but having a build in which that still times out after pull request #16341 is merged would allow checking that the test run’s *.xml and *.fuel files are then archived. It remains to be confirmed whether this actually fixes the timeout issue as for all of the builds so far, ‘pharo-ci-jenkins2-win10’ was used, which doesn’t run the tests at all (issue #15105).

@Rinzwind Rinzwind marked this pull request as ready for review March 25, 2024 22:53
@Rinzwind Rinzwind closed this Mar 25, 2024
@Rinzwind Rinzwind reopened this Mar 25, 2024
@Rinzwind
Copy link
Contributor Author

In build 5, there no longer was a SymbolNotFoundError in the #setUp of various tests in ‘Tests-windows-64-Epicea-Tests-Test.xml’, but a PrimitiveFailed instead. I added a temporary additional commit to get more detailed stack traces to see what the argument to #createDirectory: is: commit f197b1a. I tried opening ‘Tests-windows-64-Epicea-Tests-Test-EpAnnouncementsTest-testMonitorAnnouncesUpdateWhenDisabled.fuel’ in the image of ‘Pharo12.0-PR-64bit-6692750.zip’ but that signaled an FLMethodChanged, and a MessageNotUnderstood after changing #defaultMethodChangedWarningDisabled to answer true.

@Rinzwind Rinzwind closed this Mar 26, 2024
@Rinzwind Rinzwind reopened this Mar 26, 2024
@Rinzwind Rinzwind closed this Mar 26, 2024
@Rinzwind Rinzwind reopened this Mar 26, 2024
@Rinzwind Rinzwind force-pushed the winplatform-gettemppath branch from 1d4f8be to c54c225 Compare March 26, 2024 22:33
@Rinzwind
Copy link
Contributor Author

As can be seen in the ‘Tests-windows-64-Epicea-Tests-Test.xml’ of build 9, the PrimitiveFailed was due to the argument to #createDirectory: referring to a nonexistent drive ‘X’:

<?xml version="1.0" encoding="UTF-8"?>
<testsuite name="Epicea-Tests" tests="55" timestamp="8:45:43.838 pm" seed="377703460"  failures="0" errors="97" time="985.223">                                                            
	<testcase classname="Windows64.Epicea.Tests.EpAnnouncementsTest" name="testMonitorAnnouncesUpdateWhenDisabled" time="20.034">
		<error type="PrimitiveFailed" message="primitive #createDirectory: in WindowsStore failed">
PrimitiveFailed
primitive #createDirectory: in WindowsStore failed
[…]
WindowsStore(DiskStore)>>createDirectory:
	Receiver: a WindowsStore
	Arguments and temporary variables: 
		path: 	Path / 'X:'
[…]

That was in turn due to #testResolveTempPathFromTMP on WindowsResolverTest having been executed before which left its test value for ‘TMP’ in the environment. Fixed in: commit 2ab718d.

The temporary additional commit to get the more detailed stack trace has been removed. This can be merged now.

…nvironmentTest to changes done in commit 2ab718d (“Fixed #setEnv:value:during: on OSEnvironment to restore the […]”).
@Rinzwind
Copy link
Contributor Author

There are some failing tests in build 15. See issue #16119 for the SocketStreamTest failures, and issue #16280 for the ReleaseTest failures. The other test failures are not caused by the changes in this pull request, but show up because the Windows test run no longer times out.

@jecisc
Copy link
Member

jecisc commented Mar 28, 2024

Is this ready to merge? @Rinzwind

@MarcusDenker Do we merge with the failing windows tests? They seems linked to the strict FFI that @guillep enabled. (I think, I'm not sure since I never really used FFI :) )

@Rinzwind
Copy link
Contributor Author

It’s ready to be merged yes.

I opened NewTools pull requests #728 and #729 to fix the ‘Windows64.NewTools.FileBrowser.Tests’ failures.

@jecisc jecisc closed this Mar 28, 2024
@jecisc jecisc reopened this Mar 28, 2024
@Rinzwind
Copy link
Contributor Author

The function signature I used in NewTools pull request #729 was not correct, I opened another pull request to (try to) fix it: NewTools pull request #730.

I’m not sure why using ulong* nil doesn’t work though, the FFI should perhaps be extended to support that.

@Rinzwind Rinzwind closed this Mar 29, 2024
@Rinzwind Rinzwind reopened this Mar 29, 2024
@Rinzwind Rinzwind closed this Mar 29, 2024
@Rinzwind Rinzwind reopened this Mar 29, 2024
@Rinzwind Rinzwind closed this Mar 29, 2024
@Rinzwind Rinzwind reopened this Mar 29, 2024
@Rinzwind Rinzwind closed this Mar 29, 2024
@Rinzwind Rinzwind reopened this Mar 29, 2024
@Rinzwind Rinzwind closed this Mar 29, 2024
@Rinzwind Rinzwind reopened this Mar 29, 2024
@Rinzwind
Copy link
Contributor Author

All three of the failing tests in build 21 are in SocketStreamTest (issue #16119).

@jecisc jecisc merged commit 865e9a6 into pharo-project:Pharo12 Apr 2, 2024
2 of 3 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.

2 participants