search

pantoniou / libfyaml

Fully feature complete YAML parser and emitter, supporting the latest YAML spec and passing the full YAML testsuite.
MIT License
241 stars 74 forks source link

Replace fy-list with MIT one #64

Open Enjection opened 1 year ago

Enjection commented 1 year ago

Rewrite (partially) fy-list.h from scratch to remove GPL-licensed code and make this library actually MIT.

pantoniou commented 1 year ago

This is interesting; as I mentioned at the open issue I am fine with the end-goal (an MIT licensed list implementation).

This is a good starting point for this discussion.

First of all I'll list the reason why the Linux kernel's list implementation was picked although it has a slightly incompatible license.

  1. At some point in time I'd like to integrate libfyaml in the kernel, so having the same list implementation would make things easier to review.
  2. This list implementation is heavily used so is bug free (at least from non obscure bug cases).
  3. I didn't have any commercial applications in mind during the library's creation.

Looking at the patch this is a straight forward replacement, however I'd like to preserve the original list implementation as an option. A configure time switch can be used to enable the new MIT licensed implementation.

Another point would be to have unit test for the list operations since list operation are the backbone of the data structures of this library, and bugs in the list implementation can be hard to be tracked down.

So for now, can you update with unit tests and I will take a look again about how to keep the compatibility I have in mind implemented.

Enjection commented 1 year ago

I added unit tests, but unsure about right placement. Correct me if I'm wrong. I also changed naming so, it's not so straightforward to add a switch in configure and just replace fy-list with GPL one. I could whether change naming to fit kernel-one or make ifdef + typdefs to use them when the right macro is defined. What do you think?

kdubb commented 1 year ago

@pantoniou It would be great to get this in soon since it's not mentioned anywhere that non MIT licensed code is in use.

Also, I would like to suggest that the MIT licensed version is used by default and if/when you wish to integrate with the kernel you can select the version lifted from the kernel. It makes a lot more sense to default to the advertised license and then allow you to change that if you wish.

yrashk commented 1 year ago

I am also very interested in this!

kdubb commented 3 months ago

@pantoniou Is there a reason not to use this? This is a necessary change to meet the advertised license that should be merged.