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

Support php-version: preinstalled #872

Open
1 task done
SystemKeeper opened this issue Oct 12, 2024 · 5 comments
Open
1 task done

Support php-version: preinstalled #872

SystemKeeper opened this issue Oct 12, 2024 · 5 comments
Assignees
Labels
awaiting-release Added/Fixed and tested, awaiting release enhancement New feature or request

Comments

@SystemKeeper
Copy link

Describe the feature
Most runners (be it GitHub-Hosted or Self-Hosted) have php versions pre-installed. For some workflows the performance is more important than the exact php version. So when targeting ubuntu-24.04 it makes sense to use PHP 8.3, while on ubuntu-22.04 it makes sense to use PHP 8.1. Therefore I was thinking if adding something like php-version: preinstalled or php-version: highest-preinstalled would make sense and setup-php would automatically choose the (highest) version that's already inside the image?

Version

  • I have checked releases, and the feature is missing in the latest patch version of v2.

Underlying issue
Reduce bandwidth consumption and time to setup.

Describe alternatives
Manually adjust the workflows.

Additional context

Are you willing to submit a PR?
Generally yes, although no idea where to start.

@SystemKeeper SystemKeeper added the enhancement New feature or request label Oct 12, 2024
@shivammathur
Copy link
Owner

Added in da72908

@shivammathur shivammathur added the awaiting-release Added/Fixed and tested, awaiting release label Dec 23, 2024
@SystemKeeper
Copy link
Author

Awesome ❤️

Quick question, the linked commit says pre-installed in the docs, but checks for pre in the scripts?

@shivammathur
Copy link
Owner

It should accept pre, pre-installed and preinstalled.

@SystemKeeper
Copy link
Author

But the check is only for "pre":

if [ "$version" = "pre" ]; then

Or am I missing something?

@shivammathur
Copy link
Owner

It only passes first 3 characters to the scripts.

setup-php/src/utils.ts

Lines 61 to 74 in 775fa76

switch (true) {
case /^(latest|lowest|highest|nightly|\d+\.x)$/.test(version):
return JSON.parse((await fetch.fetch(await getManifestURL()))['data'])[
version
];
default:
switch (true) {
case version.length > 1:
return version.slice(0, 3);
default:
return version + '.0';
}
}
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-release Added/Fixed and tested, awaiting release enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants