-
Notifications
You must be signed in to change notification settings - Fork 56
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
create composer.json on generate:module
?
#234
Comments
Composer installers plugin suggest to use See: https://github.com/composer/installers?tab=readme-ov-file#current-supported-package-types. Related: |
👍 for this |
Yes, this would be fantastic and solve quite a few issues. Cross-ref: https://lab.civicrm.org/dev/core/-/issues/5581, eileenmcnaughton/nz.co.fuzion.omnipaymultiprocessor#261 (comment) |
Regarding the issue @nganivet is mentioning that also requires that civicrm installs the extensions composer.json requirements. I am not sure whether that is the scope of this issue. What do you think @sluc23 ? The omnipay extension loads a couple of dependencies through composer.json. This means that the code also contains the vendor directory within the extension itself. The omnipay loads a subdependency which is already present in civicrm but with a different version (depending on php version, cms and civicrm version). |
The scope of this issue is just to create a composer.json so that we can more easily install extensions with composer. See: https://civicrm.org/blog/bradleyt/composer-installers-and-civicrm The Omnipay problem is a different problem, outside the scope of civix. Most people do not install extensions with composer. |
@jaapjansma @mlufty There are more and more extensions that have dependencies, including to other civicrm extensions, external packages and third party libraries. I think we should also address usage with the composer merge plugin and/or other mechanism to deal with these dependencies as part of pushing extension authors to include composer.json in their releases. See https://www.drupal.org/docs/8/modules/webform/webform-frequently-asked-questions/how-to-use-composer-to-install-libraries-for-the-webform-module, this would elegantly resolve the issue referenced above. |
@mlutfy since we already have a <php_compatibility> key in the info.xml, it likely is in scope that civix translates this to a require: { "php": ">=x.y" } in the generated composer.json ... unless I am missing something? |
@nganivet I agree. Ideally we should use composer merge. |
In order to move extensions forwards to be more composer-friendly in the Drupal 8/9/10 ecosystem, I think it would be a good idea to start requiring extensions to ship with its own
composer.json
file with the basic minimal metadata.In Drupal 8+, we start using composer to add Drupal components and we can include here Civi extensions. But because there's not (yet) a clear convention about how an extension must be shipped to be easily required in an external composer.json file, I'm proposing to start requiring (mandatory?) this..
Now a days many extensions already include
composer.json
file to define their required dependencies.. but usually those files do not have the basic attributes needed to be part of anothercomposer.json
(like name, description, and type) as composer "packages", and usually they need to be downloaded as "artifacts" or git repos, but no composer packagesSo my proposal is to create on
civix generate:module
command the basiccomposer.json
with this general format:Most of the values can be copied from the original
info.xml
file, and free to be edited by the developer, but the one that shouldn't be changed is"type": "civicrm-extension"
. With that attribute an external composer.json file, can require this package and depending on the type execute different actions, like placing extensions in specific pathI think this issue goes over the scope of
civix
and needs some more global definitions and agreements in the Civi extensions ecosystem, by i think if we start creataing it incivix
the adoption curve will be quicker.Thoughts?
The text was updated successfully, but these errors were encountered: