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
[deb] Support for php7 and mariadb #681
Conversation
As discussed on https://sourceforge.net/p/vufind/mailman/message/35002108/ This is the main change i would like to make in time to 3.0 release. The packaging process needs other changes that i'm planning to discuss and make (if you're willing) but they're not so much important as this one. This will make vufind compatible with php7 and mariadb de facto, because as long as vufind itself already support them its deb package don't. I would like to point out that this changes won't break the previous behaviour, it will just introduce new alternative dependencies based on virtual packages that can either provide php5 or php7, i did not test them yet, i'm planning to virtualize a debian sid to do so but it's very unlikelly that something is wrong and if it is, it would just break the php7 use case. How are you guys on the release schedule? I'm planning to make the test at about this friday, but i understand that i may be pushing you guys to close to the deadline and if that's the case i can hurry up and test it tomorrow or wednesday.
|
This makes sense at a glance, though of course if you can do more tests that would be appreciated. My plan is to build and test the final 3.0 release package on Friday so that I can deploy the files to SourceForge and let them replicate over the weekend to ensure high availability of content on the official Monday release date. If you are able to test before Friday, that would be helpful, but if you cannot, I will try building the package from this branch as part of my own planned testing. Thanks for your help with this! |
|
Thanks Demian, I will test it before friday then. By the way, the build is doing flawlessly (at least for as much as i can see), I checked the generated .deb and it contains the desired dependencies too. edit- I didn't mention mariadb support because i believe this one doesn't need tests, also i have a development vufind installation that runs on mariadb. |
|
Thanks, that's great to hear. Regarding mariadb, I can confirm that VuFind works with the system since newer versions of Fedora include mariadb instead of mysql, and VuFind installs correctly there... so the only untested question is whether the package will install the correct dependencies if it encounters a Debian-based distro that has only mariadb and not mysql. |
|
@demiankatz sorry, i forgot to mention how the dependencies will work in this case. The package is requesting either mysql-server or virtual-mysql-server-core, which means any of these two is enough. The virtual-mysql-server-core package is a virtual package, the purpose of a virtual package is to group packages that satisfies equal needs, what happens is that when one installs mysql-server OR mariadb-server, the package virtual-mysql-server-core is installed as well as a dependency, so anybody with mariadb-server installed won't need to install mysql-server as well. I believe the only way to cause a problem would be if some debian based distro did a very strange and somewhat erroneous package of mariadb, causing it to not comply with the virtual package, but i don't think this is the case, and if it was, it would be a distro problem and probably fixed as soon as reported.
The only difference would be that mysql-server wouldn't be installed, the other dependencies would not be affected, especially since php-mysql is mariadb compatible. If you find it better, I can test the new package with php7 and mariadb instead of php7 and mysql. |
|
Oh, one more thing. I forgot to ask before but do you want vufind to ask for mariadb or php7 by default and accept php5 or mysql as an alternative? Note that this would be the inverse of how it works now. The system requirements would be the same, the difference would be that if someone has neither mysql, mariadb, php5 and php7 installed, the package would ask for php7 and mariadb installation if possible and if not possible ask for php5 and mysql. |
|
Thanks for the clarification. That all makes sense to me. If you are aware of a distro that has php7 and mariadb instead of php5 and mysql, that would be a great test case for the package. However, if no such creature exists, please don't go out of your way forcing a test. I'm willing to trust that the package will do the right thing in most cases! As far as the defaults, I think your current behavior is reasonable. Right now, mysql and php5 are more common than mariadb and php7 (at least in Ubuntu space, which is what VuFind's documentation points people toward). We can revisit these defaults as time passes to see if we want to reverse anything. |
|
One small question: I see that your alternative for the php5-xsl package is php-xml. Is this intentional, and are these fully equivalent if so? |
|
Yes, it happens that there's no php7-xsl package, instead the xsl functionality was introduced with php7-xml, so they're fully equivalent. edit- And the virtual package php-xml provides php7-xml. PS. I'm installing the sandbox right now. |
|
Great, thanks for the confirmation! |
|
One other question: I notice that the PR introduces some trailing whitespace on several lines. Is this intentional or just an artifact of your editor? I'm planning on backporting your changes to the release-2.5 branch, so I want to be sure I bring everything over if it is meaningful, but otherwise I would prefer to minimize the number of changed lines to reduce confusion. |
|
Oh, they aren't intentional, i really don't know what happened, my editor already highlight trailing whitespaces, I was with the impression that i was removing them, but somehow i managed to introduce them. Whats the better approach to fix this? Is it better if i make another commit into my branch or you remove them when commiting? |
|
I can take care of it tomorrow when committing -- not a problem. Thanks for the confirmation! |
This is needed because currently the virtual package php5 depends on libapache2-mod-php5 but the virtual package php uses php7-fpm by default. So by making the libapache2-mod-php package need explicit we won't run into cases where the user installed php7-fpm or the virtual package php choose php7-fpm instead of the apache's one. Note: Installing vufind without libapache2-mod-php or libapache2-mod-php5 fails with an apache errors as it doesn't recognize vufind.conf, which uses php keywords. Please also note that the first package in the precedence order [libapache2-mod-php5] is already a dependency from php5, so this won't break previous functionality.
|
Hi Demian, i just tested the installation on a mariadb with php7 server and i believe we are good to go now. I had to make one last commit to ensure php7 support, i wrote the reasons to do so on the commit message. Please feel free to ask any questions regarding the PR, i will answer then ASAP as i know you're planning to release on this friday. |
|
Thanks again for your help with this -- I've incorporated it into the release-2.5 branch and will merge it forward from there momentarily. |
|
Ubuntu 16.04 just came out and uses PHP 7 by default, so that gave me a perfect opportunity to test the new package. Unfortunately, there is a problem: mbstring is no longer bundled by default, requiring a separate php-mbstring package. However, there is no PHP 5 equivalent. Due to the limited expressiveness of the DEB "Depends" statement, I'm not sure how to deal with this. Any chance you're around and have any suggestions? I'll try hacking around, but this seems like a hard problem to solve elegantly. |
|
Thanks Demian, it's always nice to contribute to vufind, the community is great. When you decide to default to php7 and/or mariadb, i would be glad to help on the packaging side again :) By the way, the Ubuntu 16.04 LTS released this week already uses php7 by default (also, it doesn't even ship php5) and next year Debian should release its new Stable release, defaulting to php7 too. So you should consider these when choosing to default to php7. |
|
@samueloph I think we may have been writing comments simultaneously. Did you see my problem regarding php-mbstring? |
|
@demiankatz yeah, same time, i'll give it a look |
|
@samueloph Thanks! Essentially I think the problem here is that we need parentheses but they are not supported... because ideally we need something like php5 | (php, php-mbstring). I saw a forum post that proposed creating virtual packages to handle this sort of situation, but I can't seem to figure out a way to embed multiple virtual packages in the control file (I imagine it's not legal). |
|
@demiankatz Actually we can create a virtual package in the control file, i'm already studying the fix. Give me 30 minutes max. edit- I was in a hurry and just noticed now that you thought multiple virtual packages as a dependency were a problem, not the creation of such. |
|
Fantastic, thank you! If I can help in any way, please let me know. |
|
The fix is simpler than I first thought, we can use the following property: So Should fix, do you want me to commit or you do it on your side? Also, I can't believe they've made such changes on the php7 packages without warning about it (or i can't seen to find it), i will compare php5 and php7 packages to see if they dropped anything else. |
|
I'll take care of it. Thanks so much! |
|
Just for clarification, I've got a list of packages that now need to be added to the Depends list if they were used before:
php-xml and php-mbstring are already at vufind's Depends, is there a need to add any other @demiankatz ? |
|
@samueloph, no, I don't believe we need any of those other dependencies. (One ILS driver requires SOAP, but we haven't made that a hard dependency in the past). I think we're okay now. I've tested that everything works right on both Ubuntu 15.10 and 16.04 using the latest .deb! Thanks again for all of your help. |
TO-DO
As discussed on https://sourceforge.net/p/vufind/mailman/message/35002108/
This is the main change i would like to make in time to 3.0 release. The packaging process needs other changes that i'm planning to discuss and make (if you're willing) but they're not so much important as this one. This will make vufind compatible with php7 and mariadb de facto, because as long as vufind itself already support them its deb package don't.
I would like to point out that this changes won't break the previous behaviour, it will just introduce new alternative dependencies based on virtual packages that can either provide php5 or php7, i did not test them yet, i'm planning to virtualize a debian sid to do so but it's very unlikelly that something is wrong and if it is, it would just break the php7 use case.
How are you guys on the release schedule? I'm planning to make the test at about this friday, but i understand that i may be pushing you guys to close to the deadline and if that's the case i can hurry up and test it tomorrow or wednesday.
PS.: If i'm not wrong, only members of the project can label PRs, so i'm leaving this one without it.