V3c/refactor pgo br - #1228
Conversation
…rectory conflicts
…te cached entries based on request options
|
The current PR primarily involves splitting the feat/pgo-v3 branch. The following improvements have been completed:
|
There was a problem hiding this comment.
Pull request overview
This PR refactors parts of the v3 artifact download/extract pipeline and Unix CMake toolchain generation to improve cache correctness (when request options/config change) and fix Linux link ordering issues that can break CMake try_compile probes.
Changes:
- Introduces
CacheMatchInterfaceand wires it intoArtifactDownloader::generateQueue()so download types can invalidate stale cache entries based on current request options/config (e.g.,php-release,git,url). - Adjusts Linux CMake runtime library handling by moving runtime libs into the toolchain as standard libraries appended after objects/archives, avoiding
--as-neededordering pitfalls. - Improves artifact source handling: local-source extraction short-circuit, safer cleanup of legacy symlink source dirs, and switches pkg-config CFLAGS collection to full
--cflags.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/StaticPHP/Util/PkgConfigUtil.php | Switches pkg-config CFLAGS retrieval to full --cflags to retain required -I include paths. |
| src/StaticPHP/Runtime/Executor/UnixCMakeExecutor.php | Moves Linux runtime libs to toolchain standard libraries to fix link ordering (esp. try_compile). |
| src/StaticPHP/Artifact/DownloaderOptions.php | Changes --with-php default behavior to better separate cache-matching from fetch-time defaulting. |
| src/StaticPHP/Artifact/Downloader/Type/Url.php | Adds cache match logic to invalidate cache when URL changes. |
| src/StaticPHP/Artifact/Downloader/Type/PhpRelease.php | Adds cache match logic tied to --with-php semantics (including git). |
| src/StaticPHP/Artifact/Downloader/Type/GitHubRelease.php | Improves stable-release selection by consulting /releases/latest when requested. |
| src/StaticPHP/Artifact/Downloader/Type/Git.php | Adds cache match logic so config changes (url/rev/regex) invalidate cached clones. |
| src/StaticPHP/Artifact/Downloader/Type/CacheMatchInterface.php | New interface for download types to validate cached entries against current request context. |
| src/StaticPHP/Artifact/ArtifactExtractor.php | Skips extraction for local sources and safely handles legacy symlink source dirs. |
| src/StaticPHP/Artifact/ArtifactDownloader.php | Uses cache-match hook in queue generation; improves custom downloader display labels; adjusts logging behavior. |
| src/StaticPHP/Artifact/Artifact.php | Adds origin labels for custom download callbacks; makes source dir resolve to local cached dirname when applicable. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| $pending = array_values(array_filter($this->artifacts, fn ($a) => $this->generateQueue($a) !== [])); | ||
| if ($pending !== []) { | ||
| logger()->info('Downloading' . implode(', ', array_map(fn ($x) => " '{$x->getName()}'", $pending)) . " with concurrency {$this->parallel} ..."); | ||
| } |
There was a problem hiding this comment.
Good catch — fixed in 933d7da by memoizing generateQueue() per download() run instead of recomputing queues (and their hash checks) twice. Queues only depend on each artifact's own cache files, so the memo is safe within a run and cleared at the start of each download().
The pending-artifacts log filter computes each artifact's queue, and downloadWithType() computes it again, doubling the sha1_file/git rev-parse hash checks. Queues only depend on each artifact's own cache files, so memoize them per download() run.
|
StaticPHP Test Bot Detected: Extensions: none | Libraries: none | Targets: none |
What does this PR do?
Checklist before merging
*.phpor*.yml, run them locally to ensure your changes are valid:composer cs-fixcomposer analysecomposer testbin/spc dev:lint-config