POC: manage third party libs via composer #8240

MissAllSunday · 2024-05-31T20:09:46Z

This is a simple POC for allowing SMF to handle their internal third party libraries via composer.

Some key features to take into consideration for this POC

Composer is solely used by SMF, that is, no access to mod authors. Mod authors can still ship their own vendor folder if they provide their respective configuration for it
Modifications to Simplemachines/build-tools will have to be made to further remove hardcoded instances of third party libs, happy to do those changes myself if required.
SMF release process will need to change to accommodate composer commands.

.scrutinizer.yml

Sources/Config.php

live627 · 2024-06-18T02:28:56Z

Sources/Actions/Admin/ACP.php

@@ -1523,8 +1523,7 @@ public static function getFileVersions(array &$versionOptions): array
 		);

 		$ignore_sources = [
-			Config::$sourcedir . '/minify/*',
-			Config::$sourcedir . '/ReCaptcha/*',
+			Config::$vendordir . '/*',


If I'm reading the code right, this line can go because ``$vendordiris outside of$sourcedir`, no?

Yeah, you're right, lines above specify the target which is Sources

live627 · 2024-06-18T02:35:11Z

Sources/Tasks/ExportProfileData.php

-				include_once implode(DIRECTORY_SEPARATOR, [Config::$sourcedir, 'minify', 'src', 'Exception.php']);
+				include_once implode(DIRECTORY_SEPARATOR, [Config::$vendordir, 'minify', 'src', 'Exception.php']);

-				include_once implode(DIRECTORY_SEPARATOR, [Config::$sourcedir, 'minify', 'src', 'Exceptions', 'BasicException.php']);
+				include_once implode(DIRECTORY_SEPARATOR, [Config::$vendordir, 'minify', 'src', 'Exceptions', 'BasicException.php']);

-				include_once implode(DIRECTORY_SEPARATOR, [Config::$sourcedir, 'minify', 'src', 'Exceptions', 'FileImportException.php']);
+				include_once implode(DIRECTORY_SEPARATOR, [Config::$vendordir, 'minify', 'src', 'Exceptions', 'FileImportException.php']);

-				include_once implode(DIRECTORY_SEPARATOR, [Config::$sourcedir, 'minify', 'src', 'Exceptions', 'IOException.php']);
+				include_once implode(DIRECTORY_SEPARATOR, [Config::$vendordir, 'minify', 'src', 'Exceptions', 'IOException.php']);

-				include_once implode(DIRECTORY_SEPARATOR, [Config::$sourcedir, 'minify', 'src', 'Minify.php']);
+				include_once implode(DIRECTORY_SEPARATOR, [Config::$vendordir, 'minify', 'src', 'Minify.php']);

-				include_once implode(DIRECTORY_SEPARATOR, [Config::$sourcedir, 'minify', 'path-converter', 'src', 'Converter.php']);
+				include_once implode(DIRECTORY_SEPARATOR, [Config::$vendordir, 'minify', 'path-converter', 'src', 'Converter.php']);

-				include_once implode(DIRECTORY_SEPARATOR, [Config::$sourcedir, 'minify', 'src', 'CSS.php']);
+				include_once implode(DIRECTORY_SEPARATOR, [Config::$vendordir, 'minify', 'src', 'CSS.php']);

-				include_once implode(DIRECTORY_SEPARATOR, [Config::$sourcedir, 'minify', 'src', 'JS.php']);
+				include_once implode(DIRECTORY_SEPARATOR, [Config::$vendordir, 'minify', 'src', 'JS.php']);


would it not be better to just include the Composer autoload file here?

live627 · 2024-06-18T02:42:56Z

other/update_unicode_data.php

@@ -54,7 +54,7 @@
 }

 // 4. Get some more stuff we need.
-require_once $sourcedir . '/Autoloader.php';
+require_once '/vendor/autoload.php';


why the leading slash? Wouldn't that try to look in the top directory?

Sesquipedalian · 2024-06-24T23:16:11Z

index.php

+$loader = require SMF\Config::$vendordir . '/autoload.php';
+$third_party_mappers = [];
+
+// Ensure $sourcedir is set to something valid.
+if (class_exists(Config::class, false) && isset(Config::$sourcedir)) {
+	$sourcedir = Config::$sourcedir;
+}

-require_once SMF\Config::$sourcedir . '/Autoloader.php';
+if (empty($sourcedir) || !is_dir($sourcedir)) {
+	$sourcedir = __DIR__;
+}
+
+// Do any third-party scripts want in on the fun?
+if (!defined('SMF_INSTALLING') && class_exists(Config::class, false)) {
+	if (!class_exists(IntegrationHook::class, false) && is_file($sourcedir . '/IntegrationHook.php')) {
+		require_once $sourcedir . '/IntegrationHook.php';
+	}
+
+	if (class_exists(IntegrationHook::class, false)) {
+		IntegrationHook::call('integrate_autoload', [&$third_party_mappers]);
+	}
+}
+
+foreach ($third_party_mappers as $prefix => $dirname) {
+	$loader->addPsr4($prefix, $dirname);
+}


I'd prefer to see this code moved back into an autoloader ~~class~~ file rather than having it here in index.php.

Part of the reason for moving the existing autoloading code into a separate class was to avoid polluting the global namespace with variables in index.php. (Note: when backward compatibility support is enabled we end up putting references to class variables from SMF's namespace into the global namespace anyway, but the idea is to eliminate that in a future version.)

remove more calls to code in sources folder

…y autoloads

Co-authored-by: John Rayes <[email protected]>

live627 reviewed Jun 11, 2024

View reviewed changes

.scrutinizer.yml Outdated Show resolved Hide resolved

live627 reviewed Jun 11, 2024

View reviewed changes

Sources/Config.php Outdated Show resolved Hide resolved

live627 reviewed Jun 18, 2024

View reviewed changes

Sesquipedalian requested changes Jun 24, 2024

View reviewed changes

MissAllSunday and others added 14 commits September 24, 2024 10:39

feat: include already used third parties on composer.json

f392732

feat: create new Config::$vendordir var

8d8ab12

feat: remove references to third party in Sources files

59157ac

feat: remove third party code in Sources

c215a08

remove more calls to code in sources folder

fix: revert changes to Sources/Unicode/Currencies.php

59538e7

feat: use composer autoloader for SMF namespace as well as third part…

211582e

…y autoloads

revert exploding $rowData['cur_perms']

042dd7e

remove function call

8bbd6d8

Update Sources/Config.php

8240485

Co-authored-by: John Rayes <[email protected]>

Update .scrutinizer.yml

6f70eaa

Co-authored-by: John Rayes <[email protected]>

feat: move autoload functionality to its own file

50d2364

feat: move autoload functionality to its own file

b41c6b1

feat: use vendor autoload on ExportProfileData.php

31d5afe

feat: update autoload require

f608581

MissAllSunday force-pushed the composer branch from 21e27b0 to f608581 Compare September 24, 2024 17:03

fix: no need to include vendor folder on ignoring list

965d011

MissAllSunday requested review from Sesquipedalian and live627 September 24, 2024 17:32

fix: Revert changes to Currencies.php

162f068

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

POC: manage third party libs via composer #8240

POC: manage third party libs via composer #8240

MissAllSunday commented May 31, 2024

live627 Jun 18, 2024

MissAllSunday Sep 24, 2024

live627 Jun 18, 2024

live627 Jun 18, 2024

Sesquipedalian Jun 24, 2024 •

edited

Loading

POC: manage third party libs via composer #8240

Are you sure you want to change the base?

POC: manage third party libs via composer #8240

Conversation

MissAllSunday commented May 31, 2024

live627 Jun 18, 2024

Choose a reason for hiding this comment

MissAllSunday Sep 24, 2024

Choose a reason for hiding this comment

live627 Jun 18, 2024

Choose a reason for hiding this comment

live627 Jun 18, 2024

Choose a reason for hiding this comment

Sesquipedalian Jun 24, 2024 • edited Loading

Choose a reason for hiding this comment

Sesquipedalian Jun 24, 2024 •

edited

Loading