Closed (fixed)
Project:
Mailchimp
Version:
2.x-dev
Component:
General
Priority:
Normal
Category:
Feature request
Assigned:
Reporter:
Created:
23 Jan 2017 at 14:01 UTC
Updated:
7 Jan 2021 at 00:59 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
svendecabooterAttached is a patch that loads the Mailchimp library via Composer, and updates the README.txt to add instructions on how to install via Composer.
Comment #3
agoradesign commentedYes 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...
Comment #4
renrhaf+1 for installing the API via composer !
Comment #5
stdio commentedComment #6
greg boggsThe 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.
Comment #7
greg boggsComment #8
agoradesign commentedThat'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
Comment #9
agoradesign commentedHi 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
Comment #10
matt bAgree 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-...
Comment #11
megachrizThere'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:
Comment #12
agoradesign commented+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
Comment #13
rwanthI 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:
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.
Comment #14
matt bcomposer does not work for me, please do not remove the manual fallback.
Comment #15
rwanth@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?
Comment #16
matt bI'd be happy if I can manually manage the library dependency without having to use Composer.
Comment #17
aprice42 commentedComment #18
agoradesign commentedwhat about the following idea:
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
Comment #19
matt b"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.
Comment #20
megachriz@Matt B
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.
Comment #21
megachrizHere'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.
Comment #22
agoradesign commentedand 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
Comment #24
megachrizThis adds an autoloader for autoloading test classes. The autoload class is ported from #2410885: Support for staging.
Comment #25
megachrizGreat, 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.
Comment #26
firfin commentedPatch 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...
Comment #27
viappidu commentedApplies to latest dev
Comment #28
WebbehComment #29
matt bIt 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.
Comment #30
firfin commentedWill try and make time to test #27 somewhere this week. And then put it on RTBC ASAP.
Comment #31
vladimirausWorks on the current dev branch.
Comment #32
gcbComment #34
wxactly commentedRe-rolling this patch for the 2.x branch, and updating the README documentation for composer/Ludwig
Comment #35
wxactly commentedHey 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.
Comment #36
wxactly commentedComment #37
devad commentedRe: @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-phpis 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:
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. :)
Comment #38
gcb@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
Comment #39
devad commentedThank 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/mailchimpvisit 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.mdfile 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-...
Comment #41
gcbDone, thanks for the testing!
Comment #42
devad commentedThis info from readme.mb file is outdated:
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!