[3.0]: Config quirks

[sbulen]

Basic Information

1. In Windows, Settings.php is being reloaded constantly, mainly due to it's not being found in a get_included_files() check. This is due to the different DIRECTORY_SEPARATORs between Windows & linux.
2. eval returns null, which can result in null being assigned to a setting under some circumstances. E.g., attempting an upgrade, received the following error:

Steps to reproduce

1. Install 2.1.4
2. Upgrade to 3.0

Expected result

A 3.0 forum

Actual result

WSOD error

Version/Git revision

4.0 Alpha 2 - current GH

Database Engine

All

Database Version

8.4.0

PHP Version

8.3.8

Logs

Fatal error: Uncaught TypeError: Cannot assign null to property SMF\Config::$languagesdir of type string in D:\wamp64\www\84van30\Sources\Config.php:915 Stack trace: #0 D:\wamp64\www\84van30\Sources\Config.php(886): SMF\Config::set(Array) #1 D:\wamp64\www\84van30\upgrade.php(188): SMF\Config::load() #2 {main} thrown in D:\wamp64\www\84van30\Sources\Config.php on line 915

Additional Information

No response

[sbulen]

I'm using my WAMP...

I think at least part of the problem is directory separators... It keeps thinking things aren't loaded...

[sbulen]

I believe the rest of the issue is that eval() is used in Config.php against the definition default value strings.
But... eval() RETURNS NULL unless there is an explicit return in the eval'd code...

SMF/Sources/Config.php

Line 915 in 7788cb5

self::${$var} = eval($default . ';');

Using double quotes, ", will likely work better.

[sbulen]

Lots of options on the DIRECTORY_SEPARATOR question...

Option 1: Bite the bullet & use DIRECTORY SEPARATOR consistently throughout; never hardcode '/' again... Ensure folder settings are OS-appropriate.

Proper solution. Least fun. Like having a paladin on the team.

Option 2: At runtime, normalize important paths to unix '/', but the OS-sensitive functions, e.g., get_included_files(), won't work. We'd have to write our own wrappers around them. Note that Windows honors '/' in paths, but uses '\' in return calls like get_included_files(). Least effort, feels almost clean. Most consistent with past SMF behavior.

Option 3: At runtime, normalize important paths to DIRECTORY_SEPARATOR so they work better with OS-sensitive functions. Feels a bit more hackish, though to be honest, likely cleaner than option 2.

Implications here with repair_settings.php also. Likely other utilities.

[sbulen]

I've confirmed the following works to fix the eval issue:
self::${$var} = eval('return ' . "$default;");

[jdarwood007]

In Windows, Settings.php is being reloaded constantly, mainly due to it's not being found in a get_included_files() check. This is due to the different DIRECTORY_SEPARATORs between Windows & linux.

Should we stop using the hard-coded / and replace all instances with DIRECTORY_SEPARATOR?

[Sesquipedalian]

Should we stop using the hard-coded / and replace all instances with DIRECTORY_SEPARATOR?

Yes, we should.

[sbulen]

So it's confirmed we have a paladin on the team.

Now, or after the installer/upgrader rewrite gets merged?

[Sesquipedalian]

After.

[live627]

I'm in favor of option 3, above, and was under the impression that that's always been the way it should be done, probably combined with option 2 because it needed normalised, at least when I rewrote the hooks management system

…

On Thu, Jul 18, 2024 at 6:13 PM Jon Stovell ***@***.***> wrote: After. — Reply to this email directly, view it on GitHub <#8293 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADJNNYQ5UMXX2WWM3GS62DZNBR4LAVCNFSM6AAAAABK7YVNQWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMZXHAZTMMJXHA> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>