-
Notifications
You must be signed in to change notification settings - Fork 34
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
ENT-12140: deps upgrades (3.21) #1491
Conversation
@cf-bottom jenkins please, with fixes for centos-7 and ubuntu-20 |
Alright, I triggered a build: Jenkins: https://ci.cfengine.com/job/pr-pipeline/11138/ Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-11138/ |
76fb2be
to
773bb4d
Compare
ab298cf
to
b0b9acd
Compare
(cherry picked from commit 0f76eb9)
b0c376d
to
aa0f58a
Compare
9fc707b
to
add425e
Compare
yeah, looooooooks good. @cf-bottom jenkins please, throw everything at this PR. Thanks. |
Alright, I triggered a build: Jenkins: https://ci.cfengine.com/job/pr-pipeline/11177/ Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-11177/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
deps-packaging/php/cfbuild-php.spec
Outdated
@@ -34,7 +34,7 @@ fi | |||
--with-iconv \ | |||
--with-zlib=%{prefix} \ | |||
--with-libmbfl=%{prefix} \ | |||
--enable-mbstring \ | |||
--enable-mbstring=shared \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we also doing:
you also need to add extension=mbstring.so to your php.ini
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no. curious about where/how it is used! 👍 I will check. I didn't see any issues/errors...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we have three shared extensions:
cfbuild-php.spec: --with-openssl=shared,%{prefix} \
cfbuild-php.spec: --with-curl=shared,%{prefix} \
cfbuild-php.spec: --enable-mbstring=shared \
Interesting that the others had a trailing ,%{prefix}
and neither curl nor openssl .so files are explicitly added to php.ini.
I will check that the mb_*
functions operate properly on an instance and see if we need to fix things. 👍 The mb_*
functions are only used in a handful of places but a few may be used by many other places.
application/libraries/Cfe_table.php: $data = urlencode((mb_convert_encoding($data, 'UTF-8')));
application/controllers/Hubstatus.php: $arrayData = json_decode(mb_convert_encoding($statusData, 'UTF-8'), true);
tests/mocks/ci_basetestcase.php: $array = json_decode(mb_convert_encoding($data, 'UTF-8'), true);
tests/mocks/core/utf8.php: mb_internal_encoding('UTF-8');
tests/phpunit/models/VitalsModelTest.php: $array = json_decode(mb_convert_encoding($json, 'UTF-8'), true);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, good catch, this sample doesn't work unless I add extension=mbstring.so
to a local php.ini
file.
I wonder about the other two: curl and openssl, will check!
Ah, we handle this in a different way: individual files:
root@localhost:~# php --ini
Configuration File (php.ini) Path: /var/cfengine/httpd/php/lib
Loaded Configuration File: /var/cfengine/httpd/php/lib/php.ini
Scan for additional .ini files in: /var/cfengine/httpd/php/lib
Additional .ini files parsed: /var/cfengine/httpd/php/lib/cfengine-enterprise-api.ini,
/var/cfengine/httpd/php/lib/cfmod.ini,
/var/cfengine/httpd/php/lib/curl.ini,
/var/cfengine/httpd/php/lib/openssl.ini,
/var/cfengine/httpd/php/lib/php.ini
Will fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://www.phpinternalsbook.com/php7/build_system/building_extensions.html#building-php-extensions
not sure what the ,%{prefix}
is all about yet, looking...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
from configure
script output
Extensions:
--with-EXTENSION=shared[,PATH]
NOTE: Not all extensions can be built as 'shared'.
Example: --with-foobar=shared,/usr/local/foobar/
o Builds the foobar extension as shared extension.
o foobar package install prefix is /usr/local/foobar/
So given the result of the latest build the PREFIX added on doesn't do much as all three shared extensions are in the same place. I'll add it as I fix the inclusion now and see how it goes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will check shared extensions functioning
deps-packaging/php/cfbuild-php.spec
Outdated
@@ -34,7 +34,7 @@ fi | |||
--with-iconv \ | |||
--with-zlib=%{prefix} \ | |||
--with-libmbfl=%{prefix} \ | |||
--enable-mbstring \ | |||
--enable-mbstring=shared \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we have three shared extensions:
cfbuild-php.spec: --with-openssl=shared,%{prefix} \
cfbuild-php.spec: --with-curl=shared,%{prefix} \
cfbuild-php.spec: --enable-mbstring=shared \
Interesting that the others had a trailing ,%{prefix}
and neither curl nor openssl .so files are explicitly added to php.ini.
I will check that the mb_*
functions operate properly on an instance and see if we need to fix things. 👍 The mb_*
functions are only used in a handful of places but a few may be used by many other places.
application/libraries/Cfe_table.php: $data = urlencode((mb_convert_encoding($data, 'UTF-8')));
application/controllers/Hubstatus.php: $arrayData = json_decode(mb_convert_encoding($statusData, 'UTF-8'), true);
tests/mocks/ci_basetestcase.php: $array = json_decode(mb_convert_encoding($data, 'UTF-8'), true);
tests/mocks/core/utf8.php: mb_internal_encoding('UTF-8');
tests/phpunit/models/VitalsModelTest.php: $array = json_decode(mb_convert_encoding($json, 'UTF-8'), true);
- Modified php building for buggy old toolchains php/php-src#12774 https://sourceware.org/bugzilla/show_bug.cgi?id=23169 - Patched php configure script for ubuntu-16 old gcc version 5 The check in php/build/php.m4 is not sufficient to guard for gcc 5 on ubuntu 16 Ticket: ENT-12140 Changelog: none.
add425e
to
0205cc5
Compare
@cf-bottom jenkins again please, full deal, I think we are good. 🤞 |
Alright, I triggered a build: Jenkins: https://ci.cfengine.com/job/pr-pipeline/11179/ Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-11179/ |
We didn't run on exotics explicitly before merging. The weekly CI was aborted, https://ci.cfengine.com/view/weekly/job/3.21.x-weekly-pipeline/126/, so didn't check things for us. |
Ticket: ENT-12140
together
cfengine/core#5603
#1491