? Pending

User tests: Successful: Unsuccessful:

avatar jeckodevelopment
jeckodevelopment
11 Feb 2019

Pull Request for Issue #23858

Summary of Changes

As requested by the Copyright owner in #23858 (comment) and following #23866 , #23867
this addresses the GPL compliance issue and allows the Joomla Project to update the file in order to ensure compatibility with PHP 7.3.

Thanks

@wilsonge @nikosdion @brianteeman

avatar jeckodevelopment jeckodevelopment - open - 11 Feb 2019
avatar jeckodevelopment jeckodevelopment - change - 11 Feb 2019
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 11 Feb 2019
Category External Library Libraries
avatar HLeithner
HLeithner - comment - 11 Feb 2019

@jeckodevelopment can you also change the continue statements because the original PR got closed and the branch deleted.

avatar jeckodevelopment jeckodevelopment - change - 11 Feb 2019
Labels Added: ?
avatar jeckodevelopment
jeckodevelopment - comment - 11 Feb 2019

@HLeithner can you please check if all the instances have been covered?

avatar HLeithner
HLeithner - comment - 11 Feb 2019

looks good to me, maybe we get some tests on this so I can merge it.

avatar brianteeman
brianteeman - comment - 11 Feb 2019

Is the code change that simple and limited to fof. Continue is used throughout the code base. Also this is only a warning. The code still works. The code is not deprecated in php

avatar HLeithner
HLeithner - comment - 11 Feb 2019

We started already fixing this warnings in the core.

avatar nikosdion
nikosdion - comment - 11 Feb 2019

This is by no means a complete solution to the license issue :/ I ran git log c1b02959282bbe2cd8de951f1d2b84a62610ce5c..HEAD --compact-summary --pretty=oneline -- libraries/fof/ > ~/Downloads/changes.txt on the latest staging branch with these results:

3a384eeacbc4990d5b6dd8b8b4524cbfb0b13172 Merge branch 'staging' of github.com:joomla/joomla-cms into 3.8-dev
d57a8e51a2082e102e9a2b5ce5526584d5ab6142 [Codestyle] Use @link over @see for URL's and use correct return types (#15302)
 libraries/fof/database/query.php | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
557fb812d512207e882ffa6bffab536533a016e7 [com_fields] Copy paste comment error (#15565)
 libraries/fof/form/header.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
3f0bcd4687cd9325eb08b9918eacdffa1de122da Replace dirname(__FILE__) with __DIR__ (#14269)
 libraries/fof/view/view.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
7e4b439455b70e78c00f89b74a0a01df0dbf1d0d Added a method to get user timezone as DateTimezone instance (#15007)
 libraries/fof/form/field/calendar.php   | 2 +-
 libraries/fof/form/header/fielddate.php | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
c5dfb6c7c0c66fd672c604539cb854d27edd9975 Use the JHtml::_ function instead of calling functions directly. (#12546)
 libraries/fof/form/field/imagelist.php | 2 +-
 libraries/fof/form/field/media.php     | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
e6f73bcf047f5c3eb126e528f80680aba8f3a2aa Fixes #13642 - The issue which causes custom menu items to be lost when a component is updated. (#13857)
 libraries/fof/utils/installscript/installscript.php | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
205d94be2e3c33dcb802b1919f8fa8fc349a28b6 [com_fields] Move custom field types to plugin (#13319)
 libraries/fof/form/field/actions.php    | 4 +++-
 libraries/fof/form/field/components.php | 4 +++-
 libraries/fof/form/field/list.php       | 4 +++-
 libraries/fof/form/field/published.php  | 4 +++-
 4 files changed, 12 insertions(+), 4 deletions(-)
2570285679ddd9174694687d2dca87f1f2581c04 Custom fields (#11833)
 libraries/fof/form/field/actions.php    | 4 +---
 libraries/fof/form/field/components.php | 4 +---
 libraries/fof/form/field/list.php       | 4 +---
 libraries/fof/form/field/published.php  | 4 +---
 4 files changed, 4 insertions(+), 12 deletions(-)
008b78d61252994744950d7724615be3fad5066e Merge branch '3.7.x' into staging
0e2cd3765372d48ceb431a00384d265b83e29ad4 2fa handeling for mcrypt and openssl (#12497)
 libraries/fof/encrypt/aes.php | 35 ++++++++++++++++++++++++-----------
 1 file changed, 24 insertions(+), 11 deletions(-)
b00892f1d10690afeb4f4ecfa7ee3b54ba355077 Update include.php
 libraries/fof/include.php | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
4c0da178dcde554ceeb14478431010e9427f9d01 deprecate fof
 libraries/fof/include.php | 2 --
 1 file changed, 2 deletions(-)
cf95fc887073e1f3ace34eba7b6d997a9ea689e0 deprecate fof
 libraries/fof/include.php | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Therefore the modified files which require a notice are:

  • libraries/fof/database/query.php
  • libraries/fof/encrypt/aes.php
  • libraries/fof/form/field/actions.php
  • libraries/fof/form/field/calendar.php
  • libraries/fof/form/field/components.php
  • libraries/fof/form/field/imagelist.php
  • libraries/fof/form/field/list.php
  • libraries/fof/form/field/media.php
  • libraries/fof/form/field/published.php
  • libraries/fof/form/header.php
  • libraries/fof/form/header/fielddate.php
  • libraries/fof/include.php
  • libraries/fof/utils/installscript/installscript.php
  • libraries/fof/view/view.php

If you are not sure what changes were made you can use the aforementioned command yourself to locate the commits and / or use git blame on each of the files I mentioned.

FYI the commit c1b0295 is the last time I contributed a new version of FOF to Joomla on Friday, 19 August 2016 as a result of merging pull request #11673 where I also explicitly state that FOF 2 is EOL.

FYI 2: You can simply put "This file has been modified by the Joomla! project / OpenSourceMatters Inc. This no longer reflects the original work of its author." or something along those lines as a comment underneath the copyright header.

Thank you for your attention to this. Big kudos to @jeckodevelopment who responded to my contact request so promptly!

avatar HLeithner
HLeithner - comment - 11 Feb 2019

@nikosdion if its ok for you I would like to have a own PR for the other files already changed, because they don't have to do this PR

avatar brianteeman
brianteeman - comment - 11 Feb 2019

@nikosdion et al

Would it make sense to add the comment to all the fof files especially as there wont be any further changes to it from Nik and it will need to be supported by the project for approx 3 more years (J4 release date +2)

avatar nikosdion
nikosdion - comment - 11 Feb 2019

Yes, you can add this to all FOF files saying that the whole FOF package is modified by the Joomla! project / OSM and no longer reflects the original work by the author. I guess that's more fair to everyone as it doesn't prevent anyone from making changes to these files once J3 goes into maintenance mode (I suspect that come PHP 8 you'll have to do loads of changes to ensure compatibility with it).

avatar jeckodevelopment
jeckodevelopment - comment - 11 Feb 2019

So, are any of you going to make a PR to add the note to all the files involved?
Thank you

avatar jeckodevelopment
jeckodevelopment - comment - 11 Feb 2019

@nikosdion should be ok now. Can you please check?

avatar nikosdion
nikosdion - comment - 11 Feb 2019

Yes, I approve these changes. Thank you!

avatar jeckodevelopment
jeckodevelopment - comment - 11 Feb 2019

The changes introduced by a5185fe should be tested
Other commits add only the licensing note.

avatar nikosdion
nikosdion - comment - 11 Feb 2019

Your changes match the ones I made in akeeba/fof@6a8973f They do work.

avatar Joomv
Joomv - comment - 11 Feb 2019

Thank you for doing this @jeckodevelopment

avatar wilsonge wilsonge - change - 11 Feb 2019
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2019-02-11 17:30:44
Closed_By wilsonge
avatar wilsonge wilsonge - close - 11 Feb 2019
avatar wilsonge wilsonge - merge - 11 Feb 2019
avatar wilsonge
wilsonge - comment - 11 Feb 2019

Thanks guys for working on this and apologies @nikosdion for the oversight here

avatar nikosdion
nikosdion - comment - 11 Feb 2019

No problem. To be honest, I only paid attention to this part of the GPL last year when I was reviewing some code I had imported from Joomla! Platform to AWF. I think the misconception stems from what we have in mind as being the difference between 3-clause BSD and GPL licenses. While GPL doesn't have you copy the license text verbatim it still asks you to mark modified files as such. So let's just say that we're all guilty of screwing it up but now we all know better :D

Have a great night and once again thank you so much for your prompt response!

avatar Joomv
Joomv - comment - 11 Feb 2019

I know we don’t talk to each other but can you check we’re correct everywhere please?

Thanks

Rowan
[edited because I thought it was an email]

avatar nikosdion
nikosdion - comment - 11 Feb 2019

Hello Rowan,

I checked FOF because it was pretty straightforward: I knew that this code was not co-copyrighted with OSM and it's a fairly big chunk of files. I ran a git log command which I documented in this GitHub issue for future reference.

If someone would please give me a list of directories and / or files which have non-OSM copyrighted code I could run similar commands to locate the modified files and report them back.

If you want, I can also document the process and the reasoning either as a Markdown file in the build folder or another place you can point out to me to help retain institutional knowledge.

avatar Joomv
Joomv - comment - 11 Feb 2019

I would love that!

Luca please oblige

Thanks

Rowan

[also edited because I thought it was an email - don't let me near this place again]

avatar mbabker
mbabker - comment - 11 Feb 2019

If someone would please give me a list of directories and / or files which have non-OSM copyrighted code I could run similar commands to locate the modified files and report them back.

Off hand libraries/vendor/(everything-but-joomla), libraries/idna_convert, libraries/php-encryption, libraries/phputf8, libraries/phpass, media/editors/codemirror, media/editors/tinymce, media/jui/js, media/jui/less (the bulk of those two being Bootstrap). Maybe media/system has some externals.

avatar dgrammatiko
dgrammatiko - comment - 11 Feb 2019

@mbabker the media folder that you've mentioned has filed with MIT licenses, not GPL, so I don't know if this also applies there

avatar mbabker
mbabker - comment - 11 Feb 2019

I was just shooting paths I knew had external code off, wasn't doing a license check on everything. It's a starting point at least.

avatar Joomv
Joomv - comment - 11 Feb 2019

Be nice or I'll start replying by email again
[really shouldn't be on this platform - it is like Twitter but with markup]
@dgrammatiko [edited for directional purposes]
Lets be NICE!

avatar Quy
Quy - comment - 12 Feb 2019

Please change milestone to 3.9.3.

Add a Comment

Login with GitHub to post a comment