The version of iCalcreator required by this module is not PHP 7 compatible, because it uses class constructors with the same name as the class.

Not sure how to resolve this, since we can't upgrade to a newer version of the library. Or can we?

CommentFileSizeAuthor
#14 date_ical-library_update-2707373-14-D7.patch1.37 KBliam morland
#6 iCalcreator-php-7-2707373-6.patch3.04 KBjoelstein
#2 iCalcreator-php-7-2707373-2.patch2.79 KBjoelstein

Comments

joelstein created an issue. See original summary.

joelstein’s picture

joelstein commented
Status: Active » Needs review
StatusFileSize
new iCalcreator-php-7-2707373-2.patch2.79 KB

At the very least, here's a patch you can apply to the iCalcreator library for PHP 7 support.

coredumperror’s picture

coredumperror commented

We can't upgrade to a newer version of the library without needing to also make significant changes to the Feeds import code. If anyone wants to put in the work to figure out what changes need to be made, mostly having to do with the way date_ical's code accesses member attributes that have been renamed/protected in the new version, I'd be happy to accept the patch. But my team isn't going to be using PHP 7, so my boss won't give me any cycles to work on this myself.

joelstein’s picture

joelstein commented

Understood. Perhaps in the meantime you could add something to the README file pointing to this patch?

coredumperror’s picture

coredumperror commented

Good idea.

joelstein’s picture

joelstein commented
StatusFileSize
new iCalcreator-php-7-2707373-6.patch3.04 KB

Here's an updated patch with one constructor I missed.

geek-merlin’s picture

geek-merlin
Primary language German
Location Freiburg, Germany
commented
Status: Needs review » Postponed (maintainer needs more info)

Current ICalCreator 2.24 has this fixed.
https://github.com/iCalcreator/iCalcreator/tree/v2.24

Anyone tested if this is interoperable with us?

EDIT: Ups, #3 says: NO.

bkosborne’s picture

bkosborne
Primary language English
Location New Jersey, USA
commented
Status: Postponed (maintainer needs more info) » Active

I don't think it makes sense to mark this as postponed. The maintainer invited others to contribute fixes to make the module completely compatible with the latest version of iCalcreator - we should leave this issue active to help others find it.

For those that want to use the patch from #6 and are using a make file for their project, just add this to your make file:

; iCalcreator
libraries[iCalcreator][directory_name] = "iCalcreator"
libraries[iCalcreator][destination] = "libraries"
libraries[iCalcreator][download][type] = "git"
libraries[iCalcreator][download][url] = "https://github.com/iCalcreator/iCalcreator.git"
libraries[iCalcreator][download][revision] = "e3dbec2cb3bb91a8bde989e467567ae8831a4026"
libraries[iCalcreator][patch][] = "https://www.drupal.org/files/issues/iCalcreator-php-7-2707373-6.patch"

It will overwrite the similar definition from that this module's own make file provides.

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented

I have submitted the patch in #6 as pull request on iCalcreator:

https://github.com/iCalcreator/iCalcreator/pull/68

Update: The author of iCalcreator has declined the pull request and apparently does not plan to make iCalcreator 2.20.x compatible with PHP 7.

joseph.olstad’s picture

joseph.olstad
Primary language French
commented

hmm, the upstream people didn't respond to our request for 2.20.3
Maybe we should fork iCalcreator and make a release, then update our documentation?

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented

That makes sense to me.

Update: I have tagged the existing pull request:

https://github.com/lkmorlan/iCalcreator/releases/tag/v2.20.3

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented

My fork of iCalcreator 2.20.x now supports PHP 7.4 in version 2.20.4.

https://github.com/lkmorlan/iCalcreator/releases/tag/v2.20.4

joseph.olstad’s picture

joseph.olstad
Primary language French
commented

Thanks for the fork! Can you or someone please provide a patch to update the documentation /README.txt ?

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented
Status: Active » Needs review
StatusFileSize
new date_ical-library_update-2707373-14-D7.patch1.37 KB
damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs review » Reviewed & tested by the community

LGTM.

solideogloria’s picture

solideogloria commented

I also reviewed the patch and the fork's commits for PHP 7.4 compatibility. +1 LGTM.

joseph.olstad’s picture

joseph.olstad
Primary language French
commented

ok I will commit patch 14 and make a release very soon thanks for the reviews and the patch

joseph.olstad’s picture

joseph.olstad
Primary language French
commented

Thanks everyone and especially @Liam Morland for his initiative of forking the iCalcreator library.

joseph.olstad’s picture

joseph.olstad
Primary language French
commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented

Thanks a lot for forking this @Liam Morland

Anybody using composer to manage the library can do this:

    "repositories": [
        {
           "type": "vcs",
            "url": "https://github.com/lkmorlan/iCalcreator"
        }
    ]
    ...
    "require": {
        "kigkonsult/icalcreator": "2.20.4",

And with a plugin like https://github.com/mnsami/composer-custom-directory-installer you can install it to the libraries directory, or sort out the vendor autoloading.