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

Php 386 [& php-stem with PHP 7.4+ prep?] #2349

Merged
merged 6 commits into from Apr 28, 2020
Merged

Php 386 [& php-stem with PHP 7.4+ prep?] #2349

merged 6 commits into from Apr 28, 2020

Conversation

jvonau
Copy link
Contributor

@jvonau jvonau commented Apr 19, 2020

Fixes Bug

avoids using ignore_errors: yes

Description of changes proposed in this pull request.

better grouping, moves the creation of the stem.ini link to be with the installation of the tar file

Smoke-tested in operating system.

lightly tested on U-20

Mention a team member for further information or comment using @ name

@tim-moody
Copy link
Contributor

ignore_errors has been used 20 to 30 times, so not sure why this one is offensive. I like moving the link creation to the same place where the .so is put in place, but the current logic of trying to have a when that catches all exceptions is less flexible and more fragile. It would not work for the case where we had .so for php-7.4 only for one arch.

@jvonau
Copy link
Contributor Author

jvonau commented Apr 19, 2020

Once the .so files are available for 7.4 the version test I trust would be removed in main.yml and in it's place suitable stanzas to exclude arches that are not supported would be inserted in php-stem.yml to suppress the creation of the symlink.

What we gain is current users who are now excluded because 7.4 would only need to git pull runrole www_base to add the .so file and create the needed link as opposed to having to iiab-install --reinstall with having the split locations of unpacking the tar file in php-stem and creating the link in nginx.

I'll rework this PR to a final layout for right now and future availablility.

@tim-moody
Copy link
Contributor

Once the .so files are available for 7.4 the version test I trust would be removed in main.yml

no. it will be changed to <7.5
but this will still not handle the case where there is an so for x64 and not for aarch64 or some other platform that someone decides to try.

What we gain is current users who are now excluded because 7.4 would only need to git pull runrole www_base to add the .so file and create the needed link as opposed to having to iiab-install --reinstall with having the split locations of unpacking the tar file in php-stem and creating the link in nginx.

yes. I agreed to putting the link with the ini and so.

@jvonau
Copy link
Contributor Author

jvonau commented Apr 19, 2020

Once the .so files are available for 7.4 the version test I trust would be removed in main.yml

no. it will be changed to <7.5
but this will still not handle the case where there is an so for x64 and not for aarch64 or some other platform that someone decides to try.

See the proposed "Uncomment and alter to suit current supported arches"

@tim-moody
Copy link
Contributor

there are still 7.2s out there

@jvonau
Copy link
Contributor Author

jvonau commented Apr 19, 2020

the 3 supported arches?

@tim-moody
Copy link
Contributor

only x64 and armv7l I think

@jvonau
Copy link
Contributor Author

jvonau commented Apr 19, 2020

I went with x64 only as ubuntu-18.yml lists 7.2 and is the only 7.2 available on d.iiab.io

@holta holta added this to the 7.1 milestone Apr 19, 2020
@holta holta changed the title Php 386 Php 386 [& php-stem with PHP 7.4+ prep?] Apr 19, 2020
@holta
Copy link
Member

holta commented Apr 21, 2020

@tim-moody what's your summary recommendation, towards merging this PR or something like it, for IIAB 7.1 in coming days?

Related:

#829 "Is Spanish WikiHow's search fixable? (works on RACHEL site!)"
#2123 "Some IIAB installs contain a partial/old version of PHP at /etc/php/7.0 etc [on Raspbian only...due to WikiHow's php-stem?]"
#2198 "Php stem"
#2345 "Med vars & php-stem error handling"
#2347 "php7.4-stem support for Ubuntu 20.04 LTS? On which CPU architectures if so?"

@holta holta requested a review from tim-moody April 26, 2020 00:43
@holta
Copy link
Member

holta commented Apr 26, 2020

@tim-moody can you review this in the coming 24h if possible?

@tim-moody
Copy link
Contributor

with the current code there is no need to make changes if we get another version of phpx.x-stem, but in order for someone who has an earlier version to upgrade two roles will have to be run. with this PR only one role will have to be run to upgrade, but it will be necessary to change the code each time there is a new version of phpx.x-stem. it's pretty much personal preference. my views were expressed above and I no longer wish to discuss this. please do as you see fit.

@holta
Copy link
Member

holta commented Apr 26, 2020

@georgejhunt got time to look at this quickly today?

There's never a magical answer to code maintainability, but let's pick a way forward here together if possible~

@holta holta requested a review from georgejhunt April 26, 2020 17:45
@holta
Copy link
Member

holta commented Apr 26, 2020

See #2347 (comment) thanks to @tim-moody:

php7.4-stem.aarch64.tar and php7.4-stem.x64.tar uploaded to d.iiab.io

needs testing

@holta
Copy link
Member

holta commented Apr 28, 2020

We're wanting http://d.iiab.io/packages/php7.4-stem.aarch64.tar installed on Ubuntu 20.04 e.g. on RPi (that contains PHP 7.4).

However current master does not work:

2020-04-28 16:06:14,388 p=12847 u=root n=ansible | TASK [nginx : Symlink /etc/php/7.4/fpm/conf.d/20-stem.ini -> /etc/php/7.4/mods-available/stem.ini] *********************************************************************************************************************************************
2020-04-28 16:06:15,050 p=12847 u=root n=ansible | fatal: [127.0.0.1]: FAILED! => {"changed": false, "msg": "src file does not exist, use "force=yes" if you really want to create the link: /etc/php/7.4/mods-available/stem.ini", "path": "/etc/php/7.4/fpm/conf.d/20-stem.ini", "src": "/etc/php/7.4/mods-available/stem.ini"}

I'd really like to solve this today so we can move on and release IIAB 7.1 with search working for WikiHow across OS's if possible, by merging this PR or another compromise PR that moves us forward.

@georgejhunt & All can you possibly help?

@tim-moody
Copy link
Contributor

so merge this and see if you have better luck.
#2345 fixed it in roles/nginx/tasks/install.yml but not in roles/www_base/tasks/main.yml

@holta
Copy link
Member

holta commented Apr 28, 2020

Thanks everyone for the hard work + patience over these past 9 days!

@holta holta merged commit 0723891 into iiab:master Apr 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants