Hmm... Sorry, I don't think I can review this one.

I think we have a guidelines about fonts
https://fedoraproject.org/wiki/Packaging:FontsPolicy

And don't think all the bundled fonts are "free" (e.g. Verdana which is a Microsoft one), if some are not-free, you even have to drop them from the source archive.

Perhaps you can package this library without any font.

The only ref to the fonts directory is in loadFont function, and if not found, the 'FontName' will be considered as a system font (GD will search in the default system path)

Default value may need to be fixed (+ __construct)
    public $FontName = "GeosansLight.ttf";

Of course, this may require some additional tweaking in the application which rely on this library (to only use system available fonts)

Created attachment 1257156 [details]
system-fonts.patch

Here is a simple patch which allow to remove all bundled fonts and switch to a default system provided one (dejavu-sans-fonts need to be a dependency).

Examples (from README.md) work.

Hello Remi!

Thanks so much for catching that, and for writing that patch for me. That was a huge help!

I noticed that one of the fonts was available in fedora, so I added a symlink to it as well. Here are updated files:

Spec URL: https://bowlofeggs.fedorapeople.org/php-szymach-c-pchart.spec
SRPM URL: https://bowlofeggs.fedorapeople.org/php-szymach-c-pchart-2.0.3-2.fc26.src.rpm

SRPM URL is broken ;)

BTW, the symlink is not needed and won't work, and from the patch the search ignore the bundled fonts directory.

The patch need to be adapted to identity bundled font name and use either:
- system font name if available in GD default font path
- system font patch

Created attachment 1260013 [details]
system-fonts.patch

New version, instead of always use DejaVu, use a substitution map.

FYI, see bug #1429157

I forgot to tell, second example (in readme) use Silkscreen, so is a good test for this patch.

Ah, I think the build changed to fc27 on my dev box and I just copy/pasted the SRPM URL from last time (which had been fc26). Sorry about that!

I've got a new build with your new patch and the symlink dropped:


Spec URL: https://bowlofeggs.fedorapeople.org/php-szymach-c-pchart.spec
SRPM URL: https://bowlofeggs.fedorapeople.org/php-szymach-c-pchart-2.0.3-3.fc27.src.rpm

Created attachment 1260267 [details]
phpci.log

     Note: phpCompatInfo version 5.0.4 DB version 1.18.0 built Feb 24 2017
     14:37:58 CET static analyze results

Created attachment 1260268 [details]
review.txt

Generated by fedora-review 0.6.1 (f03e4e7) last change: 2016-05-02
Command line :/usr/bin/fedora-review -b 1425275
Buildroot used: fedora-rawhide-x86_64
Active plugins: Generic, PHP, Shell-api

[x]: Package complies to the Packaging Guidelines


== APPROVED ==

Sorry I was too fast.

We have 2 licensing issues:

* easy fix: bundled ttf should be removed from the .src.rpm (as we are not sure they are all free, and even mostly sure some are not).

=> pull a git snapshot (makesrc.sh in lot of php packages) + clean the fonts dir + create a tarball used in your spec.


* upstream license

See 

It was previously stated that this package is on [MIT](https://opensource.org/licenses/MIT) license, which did not meet the requirements
set by the original author. It is now under the [GNU GPL v3](http://www.gnu.org/licenses/gpl-3.0.html)
license, so if you wish to use it in a commercial project, you need to pay an [appropriate fee](http://www.pchart.net/license).

This restriction in the GPLv3 about commercial use seems to make it non-free.

Can you please try to clarify this, perhaps asking on legal list ?

I started a discussion on the legal mailing list:

https://lists.fedoraproject.org/archives/list/legal@lists.fedoraproject.org/thread/4MDAWLRSVR3QIIYFJLVKMEELS5OPBIMI/

It sounds like I have two options:

0) Try to convince pChart upstream and c-pchart to both reword their license as a pure GPL license (no restrictions on commercial use, so long as they follow the GPL). Since c-pchart is not actively maintained, this could be tricky.

1) Try to convince Ampache to stop using this package, since it is not truly Free Software.

I will probably start both options in parallel. Should we mark this as fedora-review- for now, or should we keep it open to see what happens with the above options? I'm fine with either way.

(In reply to Randy Barlow from comment #13)

> 0) Try to convince pChart upstream and c-pchart to both reword their license
> as a pure GPL license (no restrictions on commercial use, so long as they
> follow the GPL). Since c-pchart is not actively maintained, this could be
> tricky.

+1

> 1) Try to convince Ampache to stop using this package, since it is not truly
> Free Software.

+1

Other option: make use of this library optional in ampache. (but this, of course,  have to be discussed with upstream).

The IMHO ampache have a big issue bundling this lib.

> I will probably start both options in parallel. Should we mark this as
> fedora-review- for now, or should we keep it open to see what happens with
> the above options? I'm fine with either way.

I'm also fine with both way.

I wrote upstream to ask them to clarify their license. It was difficult to find out how to contact them as their forum doesn't send registration confirmation e-mails (and hasn't had a post in years). I found an e-mail address on their website. So far I haven't heard back. If I don't hear anything within a week, I will begin talking with Ampache about it.

Another possibility might be to find out if this dependency can be easily removed from Ampache downstream, depending on what it is used for and how important that functionality is.

I have now also filed an issue with c-pchart describing the issue and asking if they know how to contact upstream:

https://github.com/szymach/c-pchart/issues/35

I have been unable to get in touch with upstream pchart after ~2.5 weeks. Downstream c-pchart says they don't know how to get in touch either, though they suggested trying Linked In so I did.

I have now filed an issue with Ampache (the real reason I want to package this library, since Ampache depends on it) informing them of the problem and suggesting some possible workarounds:

https://github.com/ampache/ampache/issues/1515

What should we do with this ticket? Should we close it for now, and reopen it if we ever do get upstream pchart and downstream c-pchart to both change their licensing statements?

> What should we do with this ticket? Should we close it for now, and reopen
> it if we ever do get upstream pchart and downstream c-pchart to both change
> their licensing statements?

Yes, close it or set "notready" in flags

Agree with Remi in Comment 18.

I spent some time messing with my Ampache installation this morning and I was able to get it to run without c-pchart. In fact, I was unable to find anything in the web ui that makes graphs in the first place (perhaps it is a plugin I don't have enabled), so I am not even able to find functionality that I use that is lost.

Thus, I think I can proceed with packaging Ampache with the caveat that I will have to patch out its code that uses this library.