As mentioned in #2667786: Why does the D8 version depend on Composer Manager, the D8 version of this module contains the mailchimp-api-php library within the module package.
To allow for an easier way to update this PHP library, and since using Composer to install dependencies is now widely supported for Drupal 8, it would be recommended to include the library via Composer, instead of having it bundled with the module.

Update 2017-11-28

Comment #9 suggests finding a solution, which allows using Composer for the ones that want + shipping and loading the library locally as a fallback for the others

Comments

svendecabooter created an issue. See original summary.

svendecabooter’s picture

Status: Active » Needs review
StatusFileSize
new68.63 KB

Attached is a patch that loads the Mailchimp library via Composer, and updates the README.txt to add instructions on how to install via Composer.

agoradesign’s picture

Yes please! Doing this not via Composer is an absolute no-go imho! I was quite shocked, when I saw that the library is directly embedded in the module, additionally because it's outdated (version 1.0.3, latest one is 1.0.6) and that's not fine...

renrhaf’s picture

+1 for installing the API via composer !

stdio’s picture

Status: Needs review » Reviewed & tested by the community
greg boggs’s picture

The module originally used composer to manage its dependency. While there are 4 of you who are comfortable with using Composer to manage PHP Dependencies, and we love it ourselves, we were literally flooded with people who did not understand it.

Drupal core currently bundles it's dependencies in the tar.gz download, but not in the source code. Once that feature, or another like it is available in contrib, we'll remove the library from the module. Until then, we are keeping it bundled to stay friendly to new users who don't know how to program. You are welcome, of course, to use this patch if you'd like to use a custom version of the PHP library for your site. But, we maintain both the library and the Drupal module. So, we are keeping the versions in sync as we make releases.

greg boggs’s picture

Status: Reviewed & tested by the community » Postponed
agoradesign’s picture

That's true, there are still some people uncomfortable with that. But they have to keep up with the technology, nearly all big CMS and PHP frameworks already depend on or at least support Composer, or are on the way to do that. There will soon be no other way. As of Drupal, there'll also be soon no way to get around this, as more and more modules are using external libraries installed via Composer.

But it's your decision to postpone and we'll have to accept this. It would be great, if you'd at least update the shipped library, so that we can solve #2852755-4: Proxy/timeout http client configuration

agoradesign’s picture

Issue summary: View changes
Status: Postponed » Needs work

Hi Greg,

I hope you won't mind, if I re-open the discussion here because I want to raise discussion, if there's a way to allow both: using Composer + having some kind of local fallback.

I've never tried this, but theoretically there should be an easy way: couldn't we check for existence of the Mailchimp class, before we do the include calls on top of the module file?

I'd love to try, but unfortunately I'm lacking the time atm... that's we I want to re-open the issue because maybe someone else has time and interest to try. If that ain't that easy, we still could ask one of our Composer gurus in the community: webflo, bojan, mglaman

matt b’s picture

Agree with @agoradesign - there needs to be a fallback. Otherwise you will cut off users who cannot user composer, either due to how their hosting is set up, their own skills/confidence/capabality, or due to lack of time to constantly rebuilt and redeploy their drupal 8 site.

https://www.drupal.org/forum/support/upgrading-drupal/2017-01-20/drupal-...

megachriz’s picture

There's an alternative for installing libraries without using Composer: https://www.drupal.org/project/ludwig

The mailchimp module could provide a ludwig.json file instead of bundling the library with the module to support users who don't or can't use Composer. If we would go this route, the following actions are needed:

  1. Remove the library from this project.
  2. Add a ludwig.json file which lists the thinkshout/mailchimp-api-php library. In a ludwig.json file all dependencies of a library should also be defined, except libraries that are already a dependency of Drupal core (see #3005037-3: Important questions to answer before adding Ludwig support to module). The libraries dependency "guzzlehttp/guzzle" is already required by Drupal core, so does not need to be listed.
  3. On the project page, document that the library can be installed using Ludwig.
  4. In the README file, document that the library can be installed using Ludwig.
agoradesign’s picture

+1 for that. I'd be more strict with that. In the year 2018/2019, PHP devs and webmasters should be able to get along with Composer. This is best practice for a too long time already in the PHP community.

However, I see already the issue queue flooded with Composer related problems and complaints. At least bumping the major version to 2.x should be something to think about because it would be quite a major BC break

rwanth’s picture

I would like to reopen the discussion here, since it has been stagnant for a long time.

As we approach 2020, using composer for dependency management is pretty standard across the Drupal community. We should not need to include a static version of the library within the module, especially since it overrides the version that's already being required by composer.json.

The only action that needs to be taken is stripping the library from the module. Autoloaders handle bringing in the library without issue.

If users who do not or cannot use composer are a major concern, we can tag and provide a release branch (with separate instructions) that includes the library.

A ludwig.json file should also be a straightforward addition:

{
  "require": {
    "thinkshout/mailchimp-api-php": {
      "version": "v2.0.0",
      "url": "https://github.com/thinkshout/mailchimp-api-php/archive/v2.0.0.zip"
    }
  }
}

I think this is important to address now, since several PRs have recently been merged into the library and the latest tagged release if from February 2018. It would be much easier to make use of these updates with this change in place.

matt b’s picture

composer does not work for me, please do not remove the manual fallback.

rwanth’s picture

@Matt B, I think the real problem is that it isn't a fallback, it is an override. The library is already listed as a dependency module's composer.json, but it is being ignored by the manual included files.

Would the ludwig and/or release branch options allow you to continue making use of the module without composer?

matt b’s picture

I'd be happy if I can manually manage the library dependency without having to use Composer.

aprice42’s picture

Assigned: Unassigned » aprice42
agoradesign’s picture

what about the following idea:

  1. first, we take care of fixing code deprecations and do all necessary stuff to get D9 compatibility (https://dev.acquia.com/drupal9/deprecation_status/errors?project=mailchi...)
  2. second, we clean up the codebase and fix CS errors, if there are any
  3. having a clean codebase now, let's open a new branch supporting the new semantic versioning naming conventions: https://www.drupal.org/node/1015226 -> a new branch is needed for this anyway
  4. In the new branch, we then remove the shipped library and require it by Composer instead.

Everyone's happy then. The module would be state of the art and old school at the same time. And the best thing ist, that it'll be possible for quite a long time to still maintain both branches, because the only differences are the changed composer.json and the removed Mailchimp API files

matt b’s picture

"In the new branch, we then remove the shipped library and require it by Composer instead."

Not happy as previously stated. This means the module is only open to those who can use composer. After many years of trying (and I use composer with no issues on other projects), composer and drupal still does not work.

megachriz’s picture

@Matt B

This means the module is only open to those who can use composer.

That's not entirely true. Projects can provide an alternative installation method besides Composer, by adding a ludwig.json file. See #11 and further comments.

megachriz’s picture

Status: Needs work » Needs review
StatusFileSize
new178.67 KB

Here's a patch that removes the library files, removes references to the library files and adds a ludwig.json file. I checked the patch in #2 first to see if there was anything else to change.

I tested if I could install the library with Ludwig and that test was successful: I could see a list of audiences on /admin/config/services/mailchimp/lists after installing the mailchimp_lists module. I do had to clear caches after placing the library manually at modules/mailchimp/lib/thinkshout-mailchimp-api-php/v2.0.0.

Let's see how the testbot responds.

agoradesign’s picture

and as I've written: you can cherry pick any commit to the other branch too. The branches will only differ by the composer.json and the fact that the library is missing in the new branch.... of course before branching we may need a few lines, altering the library info, so that the module can lookup for the library both in the site-wide library directory and inside the module directory

And if you like it or not, using Drupal without Composer will get harder and harder anyway and soon ain't be possible anymore, with or without changing this here.

and imho there's even the possibility to support both variants within the same branch.. if the module is looking up first in the global libraries directory, and as a fallback to its shipped library, anyone can use and update the API via Composer, all others fall back to the shipped one (but in a future new branch, I really would recommend to remove the shipped library entirely.. if you then keep the logic to lookup there, anyone can copy the files by hand there

Status: Needs review » Needs work

The last submitted patch, 21: mailchimp-composer-and-ludwig-install-2845950-21.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

megachriz’s picture

Status: Needs work » Needs review
StatusFileSize
new180.71 KB
new2.15 KB

This adds an autoloader for autoloading test classes. The autoload class is ported from #2410885: Support for staging.

megachriz’s picture

StatusFileSize
new180.7 KB
new640 bytes

Great, tests are passing!

This patch fixes coding standard issues in Autoload.php (conversions to short array syntax). Note: other CS issues are not fixed because they are unrelated to this patch.

firfin’s picture

Patch works for me, thanks @megachriz
I did not test the manual/ludwig way, so I am hesitant to change the status to RTBC.
Ideally this will be tested by @Matt_B as he is the most active supporter for a manual option...

viappidu’s picture

StatusFileSize
new182.57 KB
new1.74 KB

Applies to latest dev

Webbeh’s picture

Assigned: aprice42 » Unassigned
matt b’s picture

It seems I'm not that active, sorry! I'd go ahead, I'm not going to install ludwig in a rush as I've managed to work around all other composer dependencies.

firfin’s picture

Will try and make time to test #27 somewhere this week. And then put it on RTBC ASAP.

vladimiraus’s picture

Status: Needs review » Reviewed & tested by the community

Works on the current dev branch.

gcb’s picture

Assigned: Unassigned » wxactly

  • wxactly committed 973f7b3 on 2.x authored by viappidu
    Issue #2845950 by MegaChriz, viappidu, svendecabooter: Install Mailchimp...
wxactly’s picture

StatusFileSize
new181.08 KB

Re-rolling this patch for the 2.x branch, and updating the README documentation for composer/Ludwig

wxactly’s picture

Hey all - I love the direction of continuing to provide some non-composer support by implementing Ludwig, however... After experimenting a bit, I don't think we can commit to this route for the module. Proper Ludwig support would require us to maintain a full dependency tree within the Mailchimp module for the Mailchimp library and all of its dependencies with specific versions. That's nine separate packages:

ralouphie/getallheaders (3.0.3)
psr/http-message (1.0.1)
guzzlehttp/psr7 (1.7.0)
guzzlehttp/promises (1.4.0)
symfony/polyfill-php72 (v1.20.0)
symfony/polyfill-intl-normalizer (v1.20.0)
symfony/polyfill-intl-idn (v1.20.0)
guzzlehttp/guzzle (6.5.5)
thinkshout/mailchimp-api-php (2.0.0)

It's just not going to be feasible to keep this dependency list up to date, and that would lead to too much risk due to future critical/security bugs, incompatible package updates, etc.

The good news is, we are still moving forward with removing the Mailchimp library from this module starting in 2.x! That means composer will be required to install this module starting in 2.x. If you cannot use composer, we recommend staying on the 8.x-1.x line.

wxactly’s picture

Version: 8.x-1.x-dev » 2.x-dev
Status: Reviewed & tested by the community » Fixed
devad’s picture

Re: @wxactly

The dependency list provided in #35 is not relevant for Ludwig support because all listed dependencies except
thinkshout/mailchimp-api-php (2.0.0) are handled automatically by Drupal 9 Core.

thinkshout/mailchimp-api-php is a standard psr4 library and Ludwig has support for it.

I have tried 2.0.0-rc1 + D9.1.0 with ludwig.json file from #13 and it works nicely.

Here is the Ludwig report:

thinkshout/mailchimp-api-php PHP library for v3 of the MailChimp API
src	        psr-4	v2.0.0	mailchimp	   Installed

thinkshout/mailchimp-api-php PHP library for v3 of the MailChimp API
src/http	psr-4	v2.0.0	mailchimp	   Installed

Ludwig 8.x-1.4 has brought semi-automated missing libraries download and unpack (one button click), so it has never been easier to manage libraries with Ludwig.

If maintainers have decided to go towards pure composer support for v.2.0.0 - that's understandable modern approach.

I just wanted to bring up this review that 2.0.0-rc1 version does not have any problem to continue to support ludwig integration. It is a simple one-library ludwig.json file support. So, maybe... just maybe... maintainers can undo the Ludwig integration drop for now, and make it part of official 2.0.0 release again.

And make all Drupal 9 users who are not yet familiar with Composer happy. :)

gcb’s picture

@wxactly and I are convinced by your suggestion, @devad . Can you test it out for us and confirm it works as expected? https://git.drupalcode.org/project/mailchimp/commit/9996fb0

devad’s picture

Thank you for swift reply @gcb.

I have tested latest Mailchimp .dev + latest Ludwig 8.x-1.4 with both D8 and D9 and everything works as expected.

If Mailchimp is installed without composer and without Ludwig module the admin/config/services/mailchimp visit results in error message: "Failed to load Mailchimp PHP library. Please refer to the installation requirements." as expected.

With Ludwig module installed and library downloaded and unpacked with Ludwig Reports > Packages "Download and unpack all missing packages" button - the missing library error message is gone and Mailchimp works nicely.

The cooperation with the Key module or Mailchimp E-Commerce module work nice as well, since these modules don't have additional library dependencies outside of Drupal 8/9 core.

If you want you can add a link to official Ludwig "Installation and usage" documentation inside Mailchimp's readme.md file and/or to project's homepage - for those who need to use Mailchimp without Composer (with Ludwig). Here is the link:

https://www.drupal.org/docs/contributed-modules/ludwig/installation-and-...

  • gcb committed ad5f479 on 2.x
    Issue #2845950 add Ludwig howto link to README.
    
gcb’s picture

Done, thanks for the testing!

devad’s picture

This info from readme.mb file is outdated:

3. Ludwig generates a listing of libraries required by those modules. The
     Packages page at admin/reports/packages provides a download link for each
     missing library along with the paths where they should be placed.

The new info should be:

3. Visit packages status at Reports > Packages (admin/reports/packages) and use "Download and unpack all missing libraries" button.

4. Rebuild the cache. Done!

  • gcb committed 27c2a89 on 2.x authored by devad
    Issue #2845950 by devad: Update Ludwig instructions
    

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.