Verify bundled sources using CI - Opcache JIT IR - #20179
Conversation
626ac97 to
c3094b6
Compare
c3094b6 to
c1085de
Compare
mvorisek
changed the title
| uses: actions/checkout@v5 | ||
| with: | ||
| repository: dstogov/ir | ||
| ref: 5a81104e650ebd7ac24eb63d4dff67db723a5278 |
There was a problem hiding this comment.
Do you propose to change this file every time when IR is updated?
Is it possible to parse GIT commit? or introduce some special file in the IR directory instead?
There was a problem hiding this comment.
Yes. It can be replaced using simple regex or yq utility possibly.
yq command:
yq 'with(.jobs[].steps[] | select(.with.repository == "dstogov/ir") | .with.ref = "xyz"; .)' verify-bundled-files.yml
|
Can this PR be merged please, I would really like to get this approved so I can use that approach for more sources. |
|
-1. I don't support this. |
|
Might I know what are you worried about? In #19802 I listed some sources which are currently integrated/bundled but not checked by any code nor many are documented how the bundling was done. The motivation of this PR is to address these negatives by documenting all integrated/bundled external sources by repeatable code/CI. The CI does not consume CI resources when the source files are not changed. |
This adds troubles and future limitations. (e.g. different PHP branches might be based on different IR branches or old PHP branches might get only critical IR fixes). |
|
The CI files are versioned /w sources together, there should be no limitations. The changed files (to either run or skip CI) are computed against base branch - see my note #20179 (comment) - which should be set for stable branches once branched. If any source in any branch is hotfixed, the CI code should be adjusted with a proper, well named patch. It takes a few minutes more but the documentation is highly improved and future upgrades are much easier. Also this has been proven by big ecosystems like Debian or Alpine (or even Composer lock file), all packages are downloaded using repeatable recipes. |
|
@dstogov can you please give this PR an approval as I hope
concerns are fully addressable. I offer a helping hand on this topic if any issue will arise and I belive this PR can be seen as an overall improvement to the external sources integration process. Tagging also @arnaud-lb as he liked the original feature request. |
|
I fully support the idea, but this may be too early to apply it to IR as it's unclear yet how it should be pulled. For instance there is a single IR branch that receives bug fixes and new features, but we may want to pull only bug fixes into php-src branches. Maybe you could start with other sources, until we figure how to pull IR? BTW, I think we should implement the "download and replace" logic in standalone scripts, outside of workflow files, so that these scripts could be used to update source as well. |
mvorisek
changed the title
f6b098a to
d3e6151
Compare
Co-authored-by: Arnaud Le Blanc <365207+arnaud-lb@users.noreply.github.com>
45d1d3b to
7b24846
Compare
|
I'm merging this now, as Dmitry has said that he will no longer update IR himself. New workflow to update IR:
From now on we will update IR only on master (maybe 8.6 too if we find bugs during RC). |
* PHP-8.6: Verify bundled sources using CI - Opcache JIT IR (#20179)
|
Thank you @mvorisek! |
* upstream/master: (99 commits) NEWS Fix property hook escape analysis causing misoptimization Fix __isset escape analysis causing misoptimization Fix memory leak when closing a statement on a killed connection Reset field_count for OK packet (php#23890) Fix phpGH-23986: Clear the realpath cache in the child after pcntl_fork() (php#23987) ext/standard: Remove redundant if-branch in rot13 (php#23795) ext/pdo: Throw a ValueError from bindColumn() for an unknown column (php#23835) Zend: rename zend_object* parameter to "this_ptr" for zend_call_* functions (php#23989) ext/pdo: Release driver options after bindParam and bindColumn tests: Raise test stack for stream error depth limit under MSan (php#23985) Zend: Remove zend_atomic.[ch] abstraction (php#23927) fibers: fix phpGH-23921 (Fibers start with error_reporting = 0 when the error_reporting INI directive is not set) ext/pdo: Keep statement class when ATTR_STATEMENT_CLASS is rejected Verify bundled sources using CI - Opcache JIT IR (php#20179) zend_portability: Simplify definition of `ZEND_NORETURN` (php#23908) Fix phpGH-23758: PDO_Firebird returns null for empty BLOBs (php#23763) Remove redundant parentheses in session tests (php#23609) Update IR (php#23861) Fix OSS-Fuzz #552682112: assertion failure wrt zp_arg_must_be_sent_by_ref() (php#23760) ... # Conflicts: # ext/openssl/xp_ssl.c
4 participants
initial PR of #19802
The new CI asserts all bundled files are up-to-date and the CI code also provides trusted documentation of how the bundled deps were added.
On push/PR the CI is run only if the bundled files were changed.