Skip to content

Document PHP extension requirements (sort of)#42

Merged
ozh merged 3 commits intomainfrom
php=mysql-ext
May 16, 2022
Merged

Document PHP extension requirements (sort of)#42
ozh merged 3 commits intomainfrom
php=mysql-ext

Conversation

@dgw
Copy link
Copy Markdown
Member

@dgw dgw commented May 15, 2022

Might have prevented YOURLS/YOURLS#3337

@dgw dgw requested a review from a team as a code owner May 15, 2022 06:36
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages bot commented May 15, 2022

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 44ba207
Status: ✅  Deploy successful!
Preview URL: https://15f0b1b2.yourls-docs.pages.dev

View logs

Copy link
Copy Markdown
Member

@ozh ozh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very default extension and so far there has been only one issue about this. There are a lot of default extensions that YOURLS needs (hash, PDO, json...) and that would be maybe overkill to list them all (and maybe letting noob users think YOURLS requires an awful lot of custom stuff). Thoughts?

@dgw
Copy link
Copy Markdown
Member Author

dgw commented May 15, 2022

Maybe this exact page/bullet-point is the wrong place for such a list, but I'd say that the required extensions should be listed somewhere in the documentation for reference.

@ozh
Copy link
Copy Markdown
Member

ozh commented May 15, 2022

Nothing against it, but I don't know how to generate a list of needed default extensions ...

@dgw
Copy link
Copy Markdown
Member Author

dgw commented May 15, 2022

I took a quick trip to google and found that Composer might be able to do it (composer check-platform-reqs). And it does work on my local checkout, but doesn't appear to show all used extensions. It doesn't even list ext-mbstring, which is suggested in YOURLS' own composer.json—but it does show ext-curl.

Maybe this is a half-baked Composer feature? The result didn't change after composer self-update to 2.3.5.

As an alternative I tried out https://github.com/RogerGee/php-ext-depends, with a better (but still seemingly incomplete) result:

~/github/php-ext-depends$ php depends.php --suffix .php ../YOURLS
Core (builtin)
ctype
curl
date (builtin)
dom
filter
hash
iconv
json
mbstring
openssl
pcre (builtin)
PDO
posix
SPL (builtin)
standard (builtin)
zlib

Still, this should be most of what's required. Amendments can be made if users find something that isn't listed causes an error when missing.

(Also I just noticed that my branch name is wrong. How did I get an =? No idea.)

@ozh
Copy link
Copy Markdown
Member

ozh commented May 15, 2022

This list isn't accurate either. For instance, iconv is not required, see https://github.com/YOURLS/YOURLS/blob/master/includes/functions-formatting.php#L328-L331

I would suggest we

  • add all the extension requirements / suggestions in YOURLS/YOURLS composer.json for reference
  • only list on docs.yourls.org the common requirements as currently
  • and mention on docs.yourls.org that common extensions needed are listed in the composer.json for those dealing with uncommon setups

Emphasis on "for 99.9% of folks we just need standard stuff"

@ozh
Copy link
Copy Markdown
Member

ozh commented May 15, 2022

Regarding the specific case of mysqlnd, I think it's de facto covered by the "ext-pdo": "*", we already have

@dgw
Copy link
Copy Markdown
Member Author

dgw commented May 15, 2022

Ah well, the readme for php-ext-depends specifically says that it doesn't try to determine if use of an extension function is in an optional code path.

I went through the previous list real quick and annotated things as best I could:

If my conclusions are reasonable, I'd happily take a stab at updating composer.json with this info. Later I can change this docs PR to point to said updated file for more details.

@ozh
Copy link
Copy Markdown
Member

ozh commented May 15, 2022

👍

@LeoColomb
Copy link
Copy Markdown
Member

👍 for this complete list!

@dgw
Copy link
Copy Markdown
Member Author

dgw commented May 15, 2022

Updated this to how it might look when pointing to composer.json. Of course the file doesn't have all the extensions listed yet. I'm working on that. Expect a PR to main repo tomorrow. 😸

@dgw dgw changed the title Document PHP MySQL extension requirement Document PHP extension requirements (sort of) May 16, 2022
@dgw
Copy link
Copy Markdown
Member Author

dgw commented May 16, 2022

And there we go: YOURLS/YOURLS#3339

@ozh ozh merged commit 8af2cb3 into main May 16, 2022
@ozh ozh deleted the php=mysql-ext branch May 16, 2022 18:59
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.

3 participants