Scope/Isolate PHP packages bundled with EDOT to prevent clash with app

[SergeyKleyman]

Closes #275

[macroscopeapp]

🔴 Critical ComponentTests/PhpDepsComponentTest.php:268

verifyVendorDevAndProdOnlyPackages() passes strings (VendorDir::getFullPath() and $prodVendorDir) to InstallPhpDeps::verifyDevProdOnlyPackages(), but that method expects array $packageNameToVersionMap. This causes a TypeError at runtime. Additionally, verifyDevProdOnlyPackages() uses an undefined variable $vendorDir instead of its parameter $packageNameToVersionMap, which will throw an "Undefined variable" error when reached.

Also found in 1 other location(s)

tools/build/GenerateComposerFiles.php:402

In verifyDerivedComposerFile, line 402 uses compact('depsGroup', 'dbgFilePath') but the variable $dbgFilePath does not exist in that scope. The method parameters are $dbgBaseFilePath and $dbgDerivedFilePath. This will cause an "Undefined variable $dbgFilePath" error.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file tests/ElasticOTelTests/ComponentTests/PhpDepsComponentTest.php around line 268:

`verifyVendorDevAndProdOnlyPackages()` passes strings (`VendorDir::getFullPath()` and `$prodVendorDir`) to `InstallPhpDeps::verifyDevProdOnlyPackages()`, but that method expects `array $packageNameToVersionMap`. This causes a `TypeError` at runtime. Additionally, `verifyDevProdOnlyPackages()` uses an undefined variable `$vendorDir` instead of its parameter `$packageNameToVersionMap`, which will throw an "Undefined variable" error when reached.

Also found in 1 other location(s):
- tools/build/GenerateComposerFiles.php:402 -- In `verifyDerivedComposerFile`, line 402 uses `compact('depsGroup', 'dbgFilePath')` but the variable `$dbgFilePath` does not exist in that scope. The method parameters are `$dbgBaseFilePath` and `$dbgDerivedFilePath`. This will cause an "Undefined variable $dbgFilePath" error.

[macroscopeapp]

🟡 Medium tools/ToolsLoggingClassTrait.php:105

logThrowable writes the stack trace header and entries unconditionally using ToolsLog::writeLineRaw, even when the requested log level isn't enabled. This produces orphaned output: the throwable message is correctly suppressed (line 118 uses ToolsLog::withLevel which checks $level), but the stack trace lines 119-123 still write to stderr. The check at line 107 also incorrectly hardcodes LogLevel::critical instead of using $level, so e.g. a warning-level throwable won't log at all even when warnings are enabled.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file tools/ToolsLoggingClassTrait.php around line 105:

`logThrowable` writes the stack trace header and entries unconditionally using `ToolsLog::writeLineRaw`, even when the requested log level isn't enabled. This produces orphaned output: the throwable message is correctly suppressed (line 118 uses `ToolsLog::withLevel` which checks `$level`), but the stack trace lines 119-123 still write to stderr. The check at line 107 also incorrectly hardcodes `LogLevel::critical` instead of using `$level`, so e.g. a warning-level throwable won't log at all even when warnings are enabled.

[macroscopeapp]

🟢 Low test/StaticCheckProd.php:60

SplFileInfo::getRealPath() returns string|false, but at lines 61 and 63 its result is passed directly to ToolsUtil::deleteDirectory() and ToolsUtil::deleteFile() without checking for false. With declare(strict_types=1) enabled, if getRealPath() returns false (e.g., for a broken symlink), PHP throws a TypeError and crashes the static check. Consider using getPathname() instead, which does not return false.

-            if ($itemInTestsDir->isDir()) {
-                ToolsUtil::deleteDirectory($itemInTestsDir->getRealPath());
            } else {
-                ToolsUtil::deleteFile($itemInTestsDir->getRealPath());
+            if ($itemInTestsDir->isDir()) {
+                ToolsUtil::deleteDirectory($itemInTestsDir->getPathname());
+            } else {
+                ToolsUtil::deleteFile($itemInTestsDir->getPathname());
             }

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file tools/test/StaticCheckProd.php around lines 60-64:

`SplFileInfo::getRealPath()` returns `string|false`, but at lines 61 and 63 its result is passed directly to `ToolsUtil::deleteDirectory()` and `ToolsUtil::deleteFile()` without checking for `false`. With `declare(strict_types=1)` enabled, if `getRealPath()` returns `false` (e.g., for a broken symlink), PHP throws a `TypeError` and crashes the static check. Consider using `getPathname()` instead, which does not return `false`.

[macroscopeapp]

🟢 Low tools/ToolsUtil.php:306

When $allowOverwrite is false on Linux, moveFile passes an empty string instead of -n to mv, so existing files are silently overwritten. mv defaults to overwriting; -n is required to prevent it. Consider passing -n when $allowOverwrite is false to match the parameter's documented intent.

-        $allowOverwriteOpt = ($allowOverwrite ? (self::isCurrentOsWindows() ? '/y' : '-f') : '');
+        $allowOverwriteOpt = ($allowOverwrite ? (self::isCurrentOsWindows() ? '/y' : '-f') : (self::isCurrentOsWindows() ? '' : '-n'));

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file tools/ToolsUtil.php around line 306:

When `$allowOverwrite` is `false` on Linux, `moveFile` passes an empty string instead of `-n` to `mv`, so existing files are silently overwritten. `mv` defaults to overwriting; `-n` is required to prevent it. Consider passing `-n` when `$allowOverwrite` is `false` to match the parameter's documented intent.

[macroscopeapp]

🟢 Low tools/ToolsUtil.php:417

deleteDirectoryContents on Linux uses rm -rf "$dirPath"/*, which does not match hidden files (dotfiles). When called via ensureEmptyDirectory for directories containing .gitkeep or .composer caches, those files remain and the directory is not actually emptied. Consider using rm -rf "$dirPath"/.[!.]* "$dirPath"/* or iterating with DirectoryIterator to remove all entries including hidden ones.

        } else {
-            self::execShellCommand("rm -rf \"$dirPath\"/*");
+            self::execShellCommand("rm -rf \"$dirPath\"/.[!.]* \"$dirPath\"/* 2>/dev/null || true");
        }

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file tools/ToolsUtil.php around lines 417-426:

`deleteDirectoryContents` on Linux uses `rm -rf "$dirPath"/*`, which does not match hidden files (dotfiles). When called via `ensureEmptyDirectory` for directories containing `.gitkeep` or `.composer` caches, those files remain and the directory is not actually emptied. Consider using `rm -rf "$dirPath"/.[!.]* "$dirPath"/*` or iterating with `DirectoryIterator` to remove all entries including hidden ones.

[macroscopeapp]

🟠 High build/GenerateComposerFiles.php:84

On line 84, verifyGeneratedFiles calls self::verifyComposerLock($depsGroup, ...) but this method does not exist. The class only defines verifyBaseComposerLock and verifyDerivedComposerLock, so PHP will throw a fatal error "Call to undefined method" when verifyGeneratedFiles is invoked. Consider calling verifyDerivedComposerLock instead, or add the missing method if different behavior is needed.

-                    self::verifyComposerLock($depsGroup, self::buildFullPath($repoRootDir, self::buildJsonFileName($depsGroup->name)));

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file tools/build/GenerateComposerFiles.php around line 84:

On line 84, `verifyGeneratedFiles` calls `self::verifyComposerLock($depsGroup, ...)` but this method does not exist. The class only defines `verifyBaseComposerLock` and `verifyDerivedComposerLock`, so PHP will throw a fatal error "Call to undefined method" when `verifyGeneratedFiles` is invoked. Consider calling `verifyDerivedComposerLock` instead, or add the missing method if different behavior is needed.

[macroscopeapp]

🟡 Medium build/generate_semconv.sh:13

BUILD_DIR resolves the relative path build/semconv from the current working directory, not from $src_repo_root_dir. When the script is run from any directory other than the repository root, this creates the build directory in the wrong location, causing subsequent Docker volume mounts and file operations to fail or target unintended paths. Consider prefixing with $src_repo_root_dir to match the pattern used for other paths in this change.

Also found in 1 other location(s)

tools/build/scope_PHP_deps.sh:135

The path ./tools/set_PHP_ini_option.sh at line 135 is relative but the working directory is set to / (via -w / on line 130). This means the script will try to execute /tools/set_PHP_ini_option.sh which doesn't exist inside the container. The correct path should be /src_repo_root_dir/tools/set_PHP_ini_option.sh based on the volume mount -v "${src_repo_root_dir}/:/src_repo_root_dir/:ro". This will cause the docker command to fail.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file tools/build/generate_semconv.sh around line 13:

`BUILD_DIR` resolves the relative path `build/semconv` from the current working directory, not from `$src_repo_root_dir`. When the script is run from any directory other than the repository root, this creates the build directory in the wrong location, causing subsequent Docker volume mounts and file operations to fail or target unintended paths. Consider prefixing with `$src_repo_root_dir` to match the pattern used for other paths in this change.

Also found in 1 other location(s):
- tools/build/scope_PHP_deps.sh:135 -- The path `./tools/set_PHP_ini_option.sh` at line 135 is relative but the working directory is set to `/` (via `-w /` on line 130). This means the script will try to execute `/tools/set_PHP_ini_option.sh` which doesn't exist inside the container. The correct path should be `/src_repo_root_dir/tools/set_PHP_ini_option.sh` based on the volume mount `-v "${src_repo_root_dir}/:/src_repo_root_dir/:ro"`. This will cause the docker command to fail.

[macroscopeapp]

🔴 Critical build/build_php_deps.sh:210

Line 211 ends the sh -c command string without a line continuation backslash, so the && on line 212 causes a shell syntax error when the Docker container executes. The container will exit immediately with a parsing failure, breaking the build. Add the missing backslash at the end of line 211.

Suggested change

	apk update && apk add bash rsync \
	&& php ./tools/build//verify_generated_composer_lock_files.php
	&& /docker_host_repo_root/tools/copy_repo_exclude_generated.sh /docker_host_repo_root /tmp/repo \
	apk update && apk add bash rsync \
	&& php ./tools/build//verify_generated_composer_lock_files.php \
	&& /docker_host_repo_root/tools/copy_repo_exclude_generated.sh /docker_host_repo_root /tmp/repo \

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file tools/build/build_php_deps.sh around lines 210-212:

Line 211 ends the `sh -c` command string without a line continuation backslash, so the `&&` on line 212 causes a shell syntax error when the Docker container executes. The container will exit immediately with a parsing failure, breaking the build. Add the missing backslash at the end of line 211.

[macroscopeapp]

🔴 Critical build/GenerateComposerFiles.php:112

In generateDerivedFromBase, the variables $baseJsonFile and $baseLockFile are only defined inside the if (ToolsLog::isLevelEnabled($logLevelToShowDiff)) block, but are used unconditionally on lines 140-141 when calling verifyDerivedComposerJson and verifyDerivedComposerLock. When debug logging is disabled, these variables are undefined, causing a PHP crash.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file tools/build/GenerateComposerFiles.php around line 112:

In `generateDerivedFromBase`, the variables `$baseJsonFile` and `$baseLockFile` are only defined inside the `if (ToolsLog::isLevelEnabled($logLevelToShowDiff))` block, but are used unconditionally on lines 140-141 when calling `verifyDerivedComposerJson` and `verifyDerivedComposerLock`. When debug logging is disabled, these variables are undefined, causing a PHP crash.

[macroscopeapp]

🟡 Medium build/ComposerUtil.php:155

In execRemove, when the command line exceeds MAX_COMMAND_LINE_LENGTH, the code executes $cmdPartsAll which includes the package that caused the overflow. This means commands can exceed the limit. The batch should be executed before adding the overflowing package, then start a new batch with that package.

        foreach ($packagesToRemove as $packageName) {
+            $cmdPartsAll = ListUtil::concat($cmdParts1stPart, $cmdParts2ndPart);
+            if (!ArrayUtil::isEmpty($cmdParts2ndPart) && strlen(ToolsUtil::buildShellCommand($cmdPartsAll)) >= self::MAX_COMMAND_LINE_LENGTH) {
+                self::execCommand($cmdPartsAll, $envVars);
+                $cmdParts2ndPart = [];
+            }
             $cmdParts2ndPart[] = $packageName;
-            $cmdPartsAll = ListUtil::concat($cmdParts1stPart, $cmdParts2ndPart);
-            if (strlen(ToolsUtil::buildShellCommand($cmdPartsAll)) < self::MAX_COMMAND_LINE_LENGTH) {
-                continue;
-            }
-            self::execCommand($cmdPartsAll, $envVars);
-            $cmdParts2ndPart = [];
         }

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file tools/build/ComposerUtil.php around lines 155-163:

In `execRemove`, when the command line exceeds `MAX_COMMAND_LINE_LENGTH`, the code executes `$cmdPartsAll` which includes the package that caused the overflow. This means commands can exceed the limit. The batch should be executed *before* adding the overflowing package, then start a new batch with that package.