cypht-org / cypht

Cypht: Lightweight Open Source webmail aggregator [PHP, JS]
http://cypht.org
GNU Lesser General Public License v2.1
979 stars 154 forks source link

Composer update #1009

Closed jonocodes closed 4 months ago

jonocodes commented 4 months ago

💬 Question

I'm new to composer, and trying to use it here...

Updating

I see calls to 'composer update' in several run scripts, but doesnt that update the lock file? I would think you dont want to update composer.lock at run time. That would make the build less deterministic. Typically I think updating a lock file is an upgrading task in a PR.

But I may be misunderstanding it. I am getting errors when I run 'composer install' without doing the update beforehand.

0.365 Installing dependencies from lock file (including require-dev)
0.368 Verifying lock file contents can be installed on current platform.
0.379 Your lock file does not contain a compatible set of packages. Please run composer update.
0.379 
0.379   Problem 1
0.379     - nikic/php-parser is locked to version v5.0.2 and an update of this package was not requested.
0.379     - nikic/php-parser v5.0.2 requires ext-ctype * -> it is missing from your system. Install or enable PHP's ctype extension.
0.379   Problem 2
0.379     - nikic/php-parser v5.0.2 requires ext-ctype * -> it is missing from your system. Install or enable PHP's ctype extension.
0.379     - sebastian/lines-of-code 1.0.4 requires nikic/php-parser ^4.18 || ^5.0 -> satisfiable by nikic/php-parser[v5.0.2].
0.379     - sebastian/lines-of-code is locked to version 1.0.4 and an update of this package was not requested.
0.379 
0.379 To enable extensions, verify that they are enabled in your .ini files:
0.379     - /etc/php8/php.ini
0.379     - /etc/php8/conf.d/00_curl.ini
0.379     - /etc/php8/conf.d/00_dom.ini
0.379     - /etc/php8/conf.d/00_fileinfo.ini
0.379     - /etc/php8/conf.d/00_iconv.ini
0.379     - /etc/php8/conf.d/00_mbstring.ini
0.379     - /etc/php8/conf.d/00_openssl.ini
0.379     - /etc/php8/conf.d/00_session.ini
0.379     - /etc/php8/conf.d/00_tokenizer.ini
0.379     - /etc/php8/conf.d/00_xml.ini
0.379     - /etc/php8/conf.d/00_xmlwriter.ini
0.379     - /etc/php8/conf.d/00_zip.ini
0.379     - /etc/php8/conf.d/01_phar.ini
0.379 You can also run `php --ini` in a terminal to see which files are used by PHP in CLI mode.
0.379 Alternatively, you can run Composer with `--ignore-platform-req=ext-ctype` to temporarily ignore these required extensions.

Installing suggested packages

I have been seeing these messages at run time:

WARNING: No PHP Redis support found, Redis caching or sessions will not work
WARNING: No PHP Memcached support found, Memcached caching or sessions will not work
WARNING: No PHP gnupg support found, The PGP module set will not work if enabled

Poking around I found there is "suggest" section in composer.json which specifically mentions the packages that may solve these warnings. What is the recommended way to install these? I have tried several methods and failed. Side note: My eventual plan is to install these packages in the docker image.

marclaporte commented 4 months ago

@Shadow243 please advise

Shadow243 commented 4 months ago

💬 Question

I'm new to composer, and trying to use it here...

Updating

I see calls to 'composer update' in several run scripts, but doesnt that update the lock file? I would think you dont want to update composer.lock at run time. That would make the build less deterministic. Typically I think updating a lock file is an upgrading task in a PR.

But I may be misunderstanding it. I am getting errors when I run 'composer install' without doing the update beforehand.

0.365 Installing dependencies from lock file (including require-dev)
0.368 Verifying lock file contents can be installed on current platform.
0.379 Your lock file does not contain a compatible set of packages. Please run composer update.
0.379 
0.379   Problem 1
0.379     - nikic/php-parser is locked to version v5.0.2 and an update of this package was not requested.
0.379     - nikic/php-parser v5.0.2 requires ext-ctype * -> it is missing from your system. Install or enable PHP's ctype extension.
0.379   Problem 2
0.379     - nikic/php-parser v5.0.2 requires ext-ctype * -> it is missing from your system. Install or enable PHP's ctype extension.
0.379     - sebastian/lines-of-code 1.0.4 requires nikic/php-parser ^4.18 || ^5.0 -> satisfiable by nikic/php-parser[v5.0.2].
0.379     - sebastian/lines-of-code is locked to version 1.0.4 and an update of this package was not requested.
0.379 
0.379 To enable extensions, verify that they are enabled in your .ini files:
0.379     - /etc/php8/php.ini
0.379     - /etc/php8/conf.d/00_curl.ini
0.379     - /etc/php8/conf.d/00_dom.ini
0.379     - /etc/php8/conf.d/00_fileinfo.ini
0.379     - /etc/php8/conf.d/00_iconv.ini
0.379     - /etc/php8/conf.d/00_mbstring.ini
0.379     - /etc/php8/conf.d/00_openssl.ini
0.379     - /etc/php8/conf.d/00_session.ini
0.379     - /etc/php8/conf.d/00_tokenizer.ini
0.379     - /etc/php8/conf.d/00_xml.ini
0.379     - /etc/php8/conf.d/00_xmlwriter.ini
0.379     - /etc/php8/conf.d/00_zip.ini
0.379     - /etc/php8/conf.d/01_phar.ini
0.379 You can also run `php --ini` in a terminal to see which files are used by PHP in CLI mode.
0.379 Alternatively, you can run Composer with `--ignore-platform-req=ext-ctype` to temporarily ignore these required extensions.

Installing suggested packages

I have been seeing these messages at run time:

WARNING: No PHP Redis support found, Redis caching or sessions will not work
WARNING: No PHP Memcached support found, Memcached caching or sessions will not work
WARNING: No PHP gnupg support found, The PGP module set will not work if enabled

Poking around I found there is "suggest" section in composer.json which specifically mentions the packages that may solve these warnings. What is the recommended way to install these? I have tried several methods and failed. Side note: My eventual plan is to install these packages in the docker image.

@jonocodes Make sure you have all these extensions enabled. If you need to proceed without enabling extensions (although I do not recommend this), add the flag--ignore-platform-req.

jonocodes commented 4 months ago

Make sure you have all these extensions enabled.

@Shadow243 can you suggest how to enable these extensions in here: https://github.com/cypht-org/cypht-docker/blob/master/image/Dockerfile

I tried using

$ pecl install redis

but it needed C compilers etc.

Then I tried using the repo packages, but there were none for php 7.

$ apk search pecl-redis
php8-pecl-redis-5.3.7-r0
php81-pecl-redis-5.3.7-r0

installing the php8 version did not help.

I also tried this:

$ docker-php-ext-enable redis
error: 'redis' does not exist
kroky commented 4 months ago

@jonocodes please ignore redis warnings - we are not using it in Cypht. Just install the missing ext-* ones. Ctype should be installed by default. Can you check if you have ctype.ini in the etc php mods-available directory?

jonocodes commented 4 months ago

@kroky ctype is installed. I found a workround for the issue, Removed this line from the lock file, and it solved the error: https://github.com/cypht-org/cypht/blob/2cf7a0ae88215e9ebc10935a0910290cec17fc5d/composer.lock#L2091

Regarding redis, it seems like cypht does make use of it: https://github.com/search?q=repo%3Acypht-org%2Fcypht%20redis&type=code

jonocodes commented 4 months ago

I found a way to get it to work: https://github.com/mlocati/docker-php-extension-installer?tab=readme-ov-file#downloading-the-script-on-the-fly-with-curl

kroky commented 4 months ago

Yes, extensions must be properly installed and then composer should not complain.

Regarding Redis - that's a specific use-case of sessions being stored in a Redis db but I am not aware of people actively using it, so I don't think the docker image should set it up by default. It is not a common practice to store sessions in redis anyway.

jonocodes commented 4 months ago

Regarding Redis - that's a specific use-case of sessions being stored in a Redis db but I am not aware of people actively using it, so I don't think the docker image should set it up by default. It is not a common practice to store sessions in redis anyway.

How do you know people at not using it though? With 100k downloads and no telemetry its hard to know these things.

If a feature is exposed in the env vars/config it should be usable. If its not usable then its a bug. Here are a couple options I can think of to deal with this type of scenario:

  1. install the extension so the feature can be used
  2. remove it from the config so users dont try to use it
  3. document that the feature does not work so users dont attempt to use it
  4. document how the user can build a custom image so redis does work

I can tell you from first hand experience that this type of issue has already been a problem. I tried to use sqlite in the provided cypht image and it did not work. There was no documentation and no error messages as to why. I filed a bug for that, but most users wont bother doing that.

kroky commented 4 months ago

Maybe you misunderstood me - I said, I was not aware of people using it and it is not a common practice for php sessions anyways, especially on developer machines. Maybe on some high-throughput websites someone set it up but we never really know. Our use-cases from the cypht active development ecosystem is this - we provide the feature but not actively use it. The warning you see is that you won't be able to use it unless you install that php extension. It is coming from config generation, so just a one-time message when you prepare your site for usage. It is worth keeping it in case you have turned on redis support in config but forgot to install the extension. What I meant by "ignore" was the fact that we don't need Redis setup in the docker image for now. If you want a separate docker image or a redis-powered docker image with some configuration, then sure, go ahead and install that extension. I just think it won't be a common practice purely based on my experience.

jonocodes commented 4 months ago

I agree that the warning is useful and I dont think there is any reason to hide it. It is particularly useful when running cypht by source because the user can simply install the extension without modifying the app and have the feature as they need.

However the docker case is different. The image is essentially the application. Users do not expect to need to modify it - by default they presume it is a fully featured release out of the box. To be clear this means if the user wants to use redis, now they would have to modify the provided production image and run their own custom one. You can think of this roughly as patching source code in a production release - which should be avoided.

Now I can think of a 5th option (though I do not recommend it) that maybe you are more comfortable with: We can provide a run time env var or key off the existing redis ones. If they are set, we can have the image self-install the redis extension at run time.

To be clear, there is nothing specific about Redis here that is important to my point. Its not about caching, or about how many users there are. My goal is to make the docker image as functional as the blessed source image (tar/gz etc).

But maybe there is a reason you specifically dont want to include redis? Perhaps its because of image size or attack surface? If so, those are different issues we could attack separately.

kroky commented 4 months ago

Alright, I think I understand you now. Why don't we come up with a few possible docker images in that case - one full-featured and one just install-and-run? Do you still need help resolving the redis issue?

jonocodes commented 4 months ago

Alright, I think I understand you now. Why don't we come up with a few possible docker images in that case - one full-featured and one just install-and-run?

I dont think I know of any projects that do separate builds with differing features.

Maybe there is a reason you specifically dont want to include redis? Perhaps its because of image size or attack surface? Please elaborate.

The image build is about 645MB. Adding redis, memchached, gnupg and xdebug added less then 10MB to the image.

Do you still need help resolving the redis issue?

No, its working now, using the docker-php-extension-installer. Well at least there is no warning. I have not actually tested the redis integration itself.

kroky commented 4 months ago

I am not a docker expert but I have seen many image types especially for bigger projects, so that shouldn't be a bad practice. Anyway, yes, since cypht is so small in terms of external dependencies and minimalistic approach, having a big image to install just to run it seems not in line with the values of the project. Also, the more software you include, the more exposed for security issues it will be. I understand the desire to include all the prerequisites, so a user can simply point to that image and configure each of the features they want, so I am fine starting with a big image including everything and then stripping it down if we need a lightweight option.

jonocodes commented 4 months ago

ok sounds good. also please point me to "minimalist" values of the project so I can understand it better.

marclaporte commented 4 months ago

also please point me to "minimalist" values of the project so I can understand it better.

https://github.com/cypht-org/cypht/issues/1037