-
Notifications
You must be signed in to change notification settings - Fork 164
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
Composer update #1009
Comments
@Shadow243 please advise |
@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 |
@Shadow243 can you suggest how to enable these extensions in here: I tried using
but it needed C compilers etc. Then I tried using the repo packages, but there were none for php 7.
installing the php8 version did not help. I also tried this:
|
@jonocodes please ignore redis warnings - we are not using it in Cypht. Just install the missing ext-* ones. |
@kroky ctype is installed. I found a workround for the issue, Removed this line from the lock file, and it solved the error: Line 2091 in 2cf7a0a
Regarding redis, it seems like cypht does make use of it: |
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. |
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:
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. |
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. |
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: 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. |
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? |
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.
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. |
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. |
ok sounds good. |
|
💬 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.
Installing suggested packages
I have been seeing these messages at run time:
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.
The text was updated successfully, but these errors were encountered: