Windows MSYS2 MinGW64 environment development

[another_commander]

Thanks for testing it, will commit the changes to master in a while.

That looks like it'll work, but just wondering why you used SETENVVAR(pathEnvVar instead of just SETENVVAR("PATH" like the others?

Because I am using pathEnvVar in three different places (SDL_getenv, envVarString length calculation and SETENVVAR). It just makes sense to store the string literal in a variable and use that.

[another_commander]

I am noticing some strange behaviour in certain cases and I am not sure what's going on. On one test system the executable with the earlier modification would refuse to launch, but as soon as I modified the name of the game folder it run fine (!)(lol). Looking at what is happening inside Task Manager I see Oolite launching and immediately going suspended, then shutting down. Furthermore, the modified executable does not seem to launch at all from within SpecialK anymore on that same system, which is something totally not desirable. I'd like Oolite for Windows to be compatible with that app, as well as other injected apps like Reshade etc. All this could be of course related to AV on the test system or some other localized problem, but this was not happening earlier so...

Can you please try one more thing? If the size of envVarString is the actual issue at hand, can you please change its declaration in the original main.m from this

Code: Select all

char envVarString[2 * MAX_PATH_LEN];

to this

Code: Select all

char envVarString[16 * MAX_PATH_LEN];

This should give it more than enough room to hold the entire final path plus some change. On my test systems this allows SpecialK injection and no shenanigans with specific folder names triggering a suspension of the game on startup.

Edit: Now I have two systems failing to start with the latest modification. Can't push the change at this stage, please let me know if changing the 2 * to 16 * above works.

Edit 2: Looks like this has something to do with allocating memory on the stack (original working implementation) vs the heap (modified implementation using malloc, with startup issues on some PCs).

[mcarans]

I am noticing some strange behaviour in certain cases and I am not sure what's going on. On one test system the executable with the earlier modification would refuse to launch, but as soon as I modified the name of the game folder it run fine (!)(lol). Looking at what is happening inside Task Manager I see Oolite launching and immediately going suspended, then shutting down. Furthermore, the modified executable does not seem to launch at all from within SpecialK anymore on that same system, which is something totally not desirable. I'd like Oolite for Windows to be compatible with that app, as well as other injected apps like Reshade etc. All this could be of course related to AV on the test system or some other localized problem, but this was not happening earlier so...

Can you please try one more thing? If the size of envVarString is the actual issue at hand, can you please change its declaration in the original main.m from this

Code: Select all

char envVarString[2 * MAX_PATH_LEN];

to this

Code: Select all

char envVarString[16 * MAX_PATH_LEN];

This should give it more than enough room to hold the entire final path plus some change. On my test systems this allows SpecialK injection and no shenanigans with specific folder names triggering a suspension of the game on startup.

Edit: Now I have two systems failing to start with the latest modification. Can't push the change at this stage, please let me know if changing the 2 * to 16 * above works.

Edit 2: Looks like this has something to do with allocating memory on the stack (original working implementation) vs the heap (modified implementation using malloc, with startup issues on some PCs).

Sorry just to clarify - I didn't test, I just inspected what you sent and was going to test after you answered my question in case you changed it. I will see if I can test today, but I currently have an RSI which is limiting my computer usage somewhat (although I have to work).

[mcarans]

It occurred to me that an extra 1 may need to be added to the length for the null char at the end of the C string:

Code: Select all

malloc(systemPathLen + currentWorkingdirLen + strlen(pathEnvVar) + 2*sizeof(char));

A fixed limit might cause problems for someone since Windows 11 max path length is apparently 32,767 chars although 4096 does indeed seem like plenty.

Anyway, hopefully I can test later.

[mcarans]

I am noticing some strange behaviour in certain cases and I am not sure what's going on. On one test system the executable with the earlier modification would refuse to launch, but as soon as I modified the name of the game folder it run fine (!)(lol). Looking at what is happening inside Task Manager I see Oolite launching and immediately going suspended, then shutting down. Furthermore, the modified executable does not seem to launch at all from within SpecialK anymore on that same system, which is something totally not desirable. I'd like Oolite for Windows to be compatible with that app, as well as other injected apps like Reshade etc. All this could be of course related to AV on the test system or some other localized problem, but this was not happening earlier so...

Oolite crashed for me with the code you provided, so I took into the account the null terminators as well as the ; and then it worked ie.

Code: Select all

    // Prepend system PATH env variable with our own executable's path
    char pathEnvVar[] = "PATH";
    char *systemPath = SDL_getenv(pathEnvVar);
    size_t currentWorkingdirLen = strlen(currentWorkingDir);
    size_t systemPathLen = strlen(systemPath);
    // the max possible length of the string below is systemPath plus the path
    // we have determined for us, plus one char for the ";" and one char for the null terminator
    char *finalPath = malloc(systemPathLen + currentWorkingdirLen + 2*sizeof(char));
    // the max possible length of the string below is systemPath plus the path
    // we have determined for us, plus the string "PATH", plus one char for the
    // "=" of the final string that will be passed on to SDL_putenv and one char for the null terminator
    char *envVarString = malloc(systemPathLen + currentWorkingdirLen + strlen(pathEnvVar) + 2*sizeof(char));
    strcpy(finalPath, currentWorkingDir);
    strcat(finalPath, ";");
    strcat(finalPath, systemPath);

    #define SETENVVAR(var, value) do {\
		    sprintf(envVarString, "%s=%s", (var), (value));\
		    SDL_putenv (envVarString);\
		    } while (0);

    SETENVVAR("GNUSTEP_PATH_HANDLING", "windows");
    SETENVVAR(pathEnvVar, finalPath);
    SETENVVAR("GNUSTEP_SYSTEM_ROOT", currentWorkingDir);
    SETENVVAR("GNUSTEP_LOCAL_ROOT", currentWorkingDir);
    SETENVVAR("GNUSTEP_NETWORK_ROOT", currentWorkingDir);
    SETENVVAR("GNUSTEP_USERS_ROOT", currentWorkingDir);
    SETENVVAR("HOMEPATH", currentWorkingDir);
       	
    SetCurrentDirectory(currentWorkingDir);

    free(envVarString);
    free(finalPath);

Let me know if that works for you.

[another_commander]

Yes! Works for me too on one of the two systems that previously failed. I will have to check on the second system in a few hours and if all goes well (which I expect it to), the fix should be committed in about twelve hours or so. Good catch with the null characters.

Edit: All good on second system, fix committed. Thanks for the help in squashing this one.

[mcarans]

Yes! Works for me too on one of the two systems that previously failed. I will have to check on the second system in a few hours and if all goes well (which I expect it to), the fix should be committed in about twelve hours or so. Good catch with the null characters.

Edit: All good on second system, fix committed. Thanks for the help in squashing this one.

You're welcome. I think I missed one though - this needs one extra char for the ";" as well:

Code: Select all

char *envVarString = malloc(systemPathLen + currentWorkingdirLen + strlen(pathEnvVar) + 2*sizeof(char));

becomes:

Code: Select all

// the max possible length of the string below is systemPath plus the path we have
// determined for us, plus one char for the ";", plus the string "PATH", plus one char for the
// "=" of the final string that will be passed on to SDL_putenv and one char for the null terminator
char *envVarString = malloc(systemPathLen + currentWorkingdirLen + strlen(pathEnvVar) + 3*sizeof(char));

[another_commander]

OK committed the latest fix.

[mcarans]

OK committed the latest fix.

The line in Oolite at which gdnc is started is this:
https://github.com/OoliteProject/oolite ... ger.m#L599

In GNUStep base, I think it is this cookie code that causes gdnc.exe to launch:
https://github.com/gnustep/libs-base/bl ... ion.m#L231

Does the request to download a list of OXZ need cookies?

Code: Select all

 if ([this->_request HTTPShouldHandleCookies] == YES)

(https://github.com/gnustep/libs-base/bl ... C6-L227C59)

I tried setting HTTPShouldHandleCookies as NO to avoid calling the cookie stuff and that seems to avoid launching gdnc.exe.

ie.
https://github.com/OoliteProject/oolite ... 23-L582C33
and
https://github.com/OoliteProject/oolite ... er.m#L1667

Code: Select all

    
    NSMutableURLRequest *request = [NSMutableURLRequest requestWithURL:[NSURL URLWithString:url]];
    [request setHTTPShouldHandleCookies:NO];

I've checked it in to https://github.com/mcarans/oolite/tree/modern_build which you can git pull and test if you like.

[another_commander]

Try it and see, I don't think we need any cookies here.

[another_commander]

Right now the PC I am on has no access to github and the wiki where most OXPs are stored so can't test. Will do so in a few hours. Can you download OXZs using the Expansion Manager in the game now?

[another_commander]

Code: Select all

    
    NSMutableURLRequest *request = [NSMutableURLRequest requestWithURL:[NSURL URLWithString:url]];
    [request setHTTPShouldHandleCookies:NO];

I've checked it in to https://github.com/mcarans/oolite/tree/modern_build which you can git pull and test if you like.

I tested the quoted code with the standard codebase and dev environment and was able to download an OXZ from the wiki without issue. If this works for you and allows you to download OXZs successfully without requiring gdnc.exe, we can push the change to master.

Edit: Tested also modern_build and it downloaded the same OXZ successfully as well. No gdnc.exe was required. Looks like this is solved.

[mcarans]

Code: Select all

    
    NSMutableURLRequest *request = [NSMutableURLRequest requestWithURL:[NSURL URLWithString:url]];
    [request setHTTPShouldHandleCookies:NO];

I've checked it in to https://github.com/mcarans/oolite/tree/modern_build which you can git pull and test if you like.

I tested the quoted code with the standard codebase and dev environment and was able to download an OXZ from the wiki without issue. If this works for you and allows you to download OXZs successfully without requiring gdnc.exe, we can push the change to master.

Edit: Tested also modern_build and it downloaded the same OXZ successfully as well. No gdnc.exe was required. Looks like this is solved.

I didn't see any issues in the modern_build either with this change. I'll try the standard build later.

As far as I recall, there are currently no further bugs with the modern_build that have been found so far. Please let me know if I've missed anything.

The only thing you had mentioned is the file size of libicudt77.dll. That comes from https://github.com/msys2/MSYS2-packages ... u/PKGBUILD. If it's a big problem, that package can be built with specific locales to reduce file size. The locale specification is here: https://github.com/unicode-org/icu/blob ... le-slicing

If all is well, my next idea is to make a GH Action release of the packages (EDIT: see https://github.com/mcarans/oolite_mingw ... /tag/0.0.1) and in a separate repo, to see if it's possible to make a GH Action that downloads those release packages (latest version), installs them in a fresh MSYS MinGW64, then builds the nsis Oolite package, enabling the automation of a versioned Oolite release with pre-built versioned packages.

[mcarans]

I tested the quoted code with the standard codebase and dev environment and was able to download an OXZ from the wiki without issue. If this works for you and allows you to download OXZs successfully without requiring gdnc.exe, we can push the change to master.

EDIT:
For me, these extern OpenGL and #ifdef changes do not affect the standard build.
https://github.com/OoliteProject/oolite ... ed139a8225

Also while you did say that the build should have symbols, I specifically use -ggdb3 to get maximum GDB debugging info (https://gcc.gnu.org/onlinedocs/gcc/Debu ... tions.html), so maybe this would be useful in the standard build too?
https://github.com/OoliteProject/oolite ... d44a4272c0

I have tested the above changes as well as the no cookies change for OXZ and they are all fine on the standard build.

However, this folder creation change for the modern_build does break the standard build (in spite of the fact that I recall seeing "withIntermediateDirectories:YES" elsewhere in the code without any Windows specific #ifdef to exclude using it):
https://github.com/OoliteProject/oolite ... 94ff92b890

[another_commander]

For me, these extern OpenGL and #ifdef changes do not affect the standard build.
https://github.com/OoliteProject/oolite ... ed139a8225

I guess those are changes that are needed for the modern build to compile, correct? I am concerned about the second one with the interface redefinition. I am worried that it may be breaking the auto-opening of the system assigned app for viewing log files once OXP verification is complete. Have you tested that part? In any case, I'd rather have an #if OO_MODERN_BUILD macro in both cases there just to ensure that we don't unintentionally break something in the standard builds. If we find out later that there is no breakage the macro can always be removed.

Also while you did say that the build should have symbols, I specifically use -ggdb3 to get maximum GDB debugging info (https://gcc.gnu.org/onlinedocs/gcc/Debu ... tions.html), so maybe this would be useful in the standard build too?
https://github.com/OoliteProject/oolite ... d44a4272c0

The default debug info is enough for standard debugging tasks (setting breakpoints, peeking variables, getting backtraces etc.). If we need more detailed debug data we can build an exe specifically for our test case and test as needed.

I have tested the above changes as well as the no cookies change for OXZ and they are all fine on the standard build.

This will go into master later today then.

However, this folder creation change for the modern_build does break the standard build (in spite of the fact that I recall seeing "withIntermediateDirectories:YES" elsewhere in the code without any Windows specific #ifdef to exclude using it):
https://github.com/OoliteProject/oolite ... 94ff92b890

Again, I think an OO_MODERN_BUILD macro could be of help here. I generally don't think there are many other places in the code where we would need it, so it is not a big deal using it.