-
Notifications
You must be signed in to change notification settings - Fork 149
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
fix qtype_stack phpunit maxima depedency #857
base: master
Are you sure you want to change the base?
Conversation
This worries me a bit, I doubt having a Thank you anyway. |
Hi @aharjula you are right - this is not the right way to fix this, but i could not yet figure out how moodles CI/CD workflow works and there is an year old issue, which could be fixed by that - so just in case someone else gets his/hers CI/CD Pipeline stuck after adding qtype_stack i would go for this ugly fix than breaking the pipeline regards |
Well, if I understood correctly, you (or the original issues people) are trying to use Moodles core CI/CD scripts (of some kind from somewhere) to test a Moodle install that has additional plugins, and in this case, plugins that have additional requirements, even requirements that go beyond the PHP extensions/libraries that Moodle itself tracks. Moodle has no internal mechanism for configuring requirements as complex as Maxima is for STACK (as far as I know). After all, we would ideally not have Maxima on the actual Moodle server at all, and in many cases, people want to install something other than the default Ubuntu GCL Maxima (the primary reason being better Unicode support). Those scripts do already do some installs themselves, especially those PHP extensions, and the correct way would be to tune those installs to include STACKs requirements if STACK is present in your configuration. You would also want them to match all the other libraries your production configuration has or does not have at the versions you use and not use whatever versions Moodles default tests use. If I understand your (or the original issues) objective correctly, the place where the "fix" should happen is your custom CI/CD config, that type of thing cannot be pushed up to the original individual components as it would push your config to other people in a way that would require trickery to override. Now why this particular solution is something that I don't want to see is due to the following use case:
Basically, while you might "fix" some CI external to STACK, you would break traditional/manual unit tests for STACK on Ubuntu by including a new forced step that could cause trouble. If anything like this would end up in the code base, I would really hope that it would be, by default, disabled and would require something like a particular environment variable or config-option to be enabled. If not disabled by default, it should at least have options for disabling it independent of the current conditions it is being tied to. Having gone through the CI workflows STACK has in github, this would probably not break them (they already seem to call In any case, thank you for documenting this for others that may have chosen this path for setting up their whole Moodle CI/CD processes. |
Hi @aharjula not sure if this is a nicer way to work around the issue, but it will for sure not break your manual phpunit test - this will check for CI Variable to be set: regards |
Indeed it will protect my own manual tests, and that is nice. However, it still makes it difficult for STACKs own CI logic to use non-standard Ubuntu Maxima combined with auto-configuration, we are not doing that now, but this will affect any future developments in that direction. Also, with STACKs own CI scripts, this would trigger reinstallation of the Maxima that was already installed, not a big thing but wasted cycles anyway. I still think this whole issue should be solved at the top-level CI config of that Moodle to which you have added STACK. And the solution should focus more on actually matching the versions in use in production. There must be a way to add things to that Moodle CI/CD script execution environment. In any case, someone else decides where this will go. To me, it still is the wrong place to solve this, but someone else may think that this is a simple thing we can do to help someone and consider the possible trouble this might cause small enough. |
Hi @aharjula
or
this would solve the two issues about wasting cycles and would allow to use custom maxima version, i agree with this not being a nice fix but rather a quirk. I agree that there should be a moodle ci/cd way of resolving dependencies for plugins, but that is not available right now. |
It seems like moodle-tool_imageoptimize has similar external depedencies - the have a check_package_command_for_testing function: Handling of missing external dependencies is done here: |
fixes: #729