Skip to content
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

Adds composer.json package information to the extension #267

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

VangelisP
Copy link

@totten This minor PR adds the functionality to add a default (from template) composer.json file to the extension's root folder

@sluc23
Copy link

sluc23 commented Jan 11, 2023

hi @totten .. do you have any objection on adding composer.json by default when creating the extension with civix?
Definitely it will make our lives easier to include extension in composer stacks..

@colemanw
Copy link
Contributor

My only objection to this is that we've been using civix a lot lately to generate core extensions which go in the same repo as civicrm and therefore don't need composer files. I wonder if it would be better as a command like

> civix generate:composer-json

@civicrm-builder
Copy link
Collaborator

Can one of the admins verify this patch?

@sluc23
Copy link

sluc23 commented Jul 18, 2023

My only objection to this is that we've been using civix a lot lately to generate core extensions which go in the same repo as civicrm and therefore don't need composer files. I wonder if it would be better as a command like

> civix generate:composer-json

hmm why not the opposite?, a param to not generate composer.json by default on extension creation..

civix generate:module --no-composer

@colemanw
Copy link
Contributor

I don't have a strong preference; either way it would need to be documented.

@sluc23
Copy link

sluc23 commented Jul 19, 2023

ok, we can adapt the PR for one way or the other... @totten thoughts?

@totten
Copy link
Owner

totten commented Jul 19, 2023

I've found myself pretty torn:

  • composer provides an administrative mechanism for add-ons on D8/9/10. For folks managing D8+ builds, it's great to have the same download+upgrade mechanism for both Drupal+Civi add-ons.
  • composer provides dependency management for libraries, which works OK on D8/9/10 - but works poorly for D7/WP/J/BD. If we encourage its use for library dependencies, then I worry that we make things worse for D7/WP/J/BD.

Maybe a way to balance those could be:

  1. Yes, create/encourage composer.json on extensions
  2. But, include a linting rule which complains if composer.json:requires or composer.lock:packages has anything risky.

@johntwyman
Copy link

We've used composer for our D7 and now D9 installations, so this functionality is compelling. It will save a huge amount of repository forking, etc., for the sake of one file.

Given colemanw's note about using civix for core extensions, and that D8+ sites only account for ~21% of reported User frameworks on stats.civicrm.org, I'd suggest the better option for now is to make generation of a composer.json file non-default, along the lines of civix generate:module --with-composer-json.

A followup PR could look to add support for developers enabling this by default by way of the civix.ini config file that the tool supports.

Either way, I'm excited and grateful to find this PR.

@mlutfy
Copy link
Contributor

mlutfy commented Aug 20, 2024

composer provides dependency management for libraries, which works OK on D8/9/10 - but works poorly for D7/WP/J/BD. If we encourage its use for library dependencies, then I worry that we make things worse for D7/WP/J/BD.

In my experience, it works fine for D7 and WP. D7/WP typically don't use composer for their core, but for CiviCRM extensions, it usually works fine, even if they bundle libraries. I've also seen quite a few WP setups that used composer (for packaging).

Also works fine for CiviCRM Standalone.

Although in general, if we use extensions that have libraries without composer, then it's generally a pain (ex: the Omnipay extension, and I had to remove/reduce libraries from extensions like my Sparkpost fork and Report Error, to avoid conflicts and the various ways people can install extensions).

Like others have mentioned, using composer for extensions is mostly about saving a lot of time when updating a codebase with many extensions (ex: CiviCRM Spark, which builds with composer, commit to git, git pull on servers).

@mlutfy
Copy link
Contributor

mlutfy commented Aug 21, 2024

More discussion on composer/extensions: https://lab.civicrm.org/infra/gitlab/-/issues/45

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants