Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate files for multiple versions #503

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Generate files for multiple versions #503

wants to merge 1 commit into from

Conversation

shish
Copy link
Collaborator

@shish shish commented Dec 5, 2024

tl;dr:

replace

$function_list = read_docs();
generate_files($function_list, "../generated/");

with

foreach(["8.1", "8.2", "8.3", "8.4"] as $version) {
    exec("cd docs && git checkout $version");
    $function_list = read_docs();
    generate_files($function_list, "../generated/$version/");
}

generate a bunch of stubs like generated/misc.php:

<?php
if(PHP_VERSION == "8.1") require_once __DIR__ . "/8.1/misc.php";
if(PHP_VERSION == "8.2") require_once __DIR__ . "/8.2/misc.php";
if(PHP_VERSION == "8.3") require_once __DIR__ . "/8.3/misc.php";
if(PHP_VERSION == "8.4") require_once __DIR__ . "/8.4/misc.php";

This also automates generation of "deprecated" safe functions (ie, instead of hard-coding deprecated/apache.php, any functions which needed safe wrappers in 8.1 but no longer need safe wrappers in 8.2 will have no-op stubs generated instead)

Currently the Exceptions are shared between all versions, which I think is a good idea? (Thoughts, anybody?)

This isn't even remotely tested, just a proof of concept to show what this kind of approach might look like

@shish shish changed the title regenerate Generate separete wrappers for multiple PHP versions Dec 5, 2024
@shish shish marked this pull request as draft December 5, 2024 19:18
@shish shish mentioned this pull request Jan 7, 2025
9 tasks
@jhdxr
Copy link

jhdxr commented Jan 22, 2025

The CI looks unhappy because of redeclaration, probaly caused by files pending for clean up?

I think we may split this huge PR into several small PR (one for generating scripts, and one for each PHP version), which makes the PR easy to read & review.

@shish shish changed the title Generate separete wrappers for multiple PHP versions Generate files for multiple versions Jan 23, 2025
@shish shish force-pushed the pr503 branch 17 times, most recently from 174426b to 3375fb4 Compare January 27, 2025 19:05
@shish
Copy link
Collaborator Author

shish commented Jan 27, 2025

I am unsure why the generator script is giving different outputs on my laptop and in CI testing D:

@shish shish force-pushed the pr503 branch 4 times, most recently from c127a0d to cf0393d Compare January 28, 2025 08:05
tl;dr:

replace

```php
$function_list = read_docs();
generate_files($function_list, "../generated/");
```

with

```php
foreach(["8.1", "8.2", "8.3", "8.4"] as $version) {
    exec("cd docs && git checkout $version");
    $function_list = read_docs();
    generate_files($function_list, "../generated/$version/");
}
```

generate a bunch of stubs like generated/misc.php:

```php
<?php
if(PHP_VERSION == "8.1") require_once __DIR__ . "/8.1/misc.php";
if(PHP_VERSION == "8.2") require_once __DIR__ . "/8.2/misc.php";
if(PHP_VERSION == "8.3") require_once __DIR__ . "/8.3/misc.php";
if(PHP_VERSION == "8.4") require_once __DIR__ . "/8.4/misc.php";
```

This also automates generation of "deprecated" `safe` functions (ie, instead of hard-coding `deprecated/apache.php`, any functions which needed safe wrappers in 8.1 but no longer need safe wrappers in 8.2 will have no-op stubs generated instead)

Currently the `Exceptions` are shared between all versions, which I _think_ is a good idea? (Thoughts, anybody?)

This isn't even remotely tested, just a proof of concept to show what this kind of approach might look like
@shish
Copy link
Collaborator Author

shish commented Jan 28, 2025

Looks like CI was only checking out the most recent revision, and the git checkout command failed silently - both should be fixed now...

@shish shish marked this pull request as ready for review January 29, 2025 11:26
@shish
Copy link
Collaborator Author

shish commented Jan 29, 2025

  • It passes automated tests 🥳
  • Automated tests cover very little 😢
  • I have done zero actual real testing 👀 (That is the next thing I'm going to do after writing this comment)
  • I don't even know how to test the Rector stuff, I've never used Rector before 😕 If you know rector please help
  • Yes, the commit is huge, but you can treat the stuff in generated/ as a black box (or just glance over one or two files to get a sense for how it works), the main changes are in generator/
  • No, I don't see any way of meaningfully splitting it into smaller commits without breaking it (eg sure I could have one commit which changes the generator files and one which changes generated files, but then that first commit will fail tests because it's out of sync)

If you want to test this, you can add a dependency on "thecodingmachine/safe": "dev-pr503" in your composer.json (like this - https://github.com/shish/shimmie2/pull/1379/files) - if people could try that and let me know if it works for your projects, that'd be great <3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants