Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tart License Violation #47

Closed
fkorotkov opened this issue Jan 16, 2024 · 11 comments
Closed

Tart License Violation #47

fkorotkov opened this issue Jan 16, 2024 · 11 comments

Comments

@fkorotkov
Copy link

Hey team and @Marcocanc, it was hard to not recognize some of Tart's code in #46. For example, LayerV2Downloader seems like a copy/paste of Tart's DiskV2#pull:

https://github.com/cirruslabs/tart/blob/f4bc02d1751dd5ad447619515e7d73a9efc2d7e6/Sources/tart/OCI/Layerizer/DiskV2.swift#L39-L121

Here is a visual diff that shows similarities. All the variable names are the same, structure is the same, just some comments removed and different kind of error handling:

Tart-Cillicon-Diff

On behalf of Tart authors we are glad you found Tart code useful but unfortunately such usage is agains Tart's License.

@Marcocanc
Copy link
Member

Marcocanc commented Jan 16, 2024

Hi @fkorotkov , thanks for signaling. Will change the code accordingly. Let me know if I should remove it immediately or if it's fine to keep it as is until the parts that are too similar are changed. Kudos for your work on Tart by the way!

@edigaryev
Copy link

edigaryev commented Jan 16, 2024

I would like to add that this is not all, here's a piece of code that was blatantly copied without even changing anything in the code itself. You won't find this a similar code on GitHub (except for in Tart forks and in your project).

Difference between the original Sources/tart/MACAddressResolver/MACAddress.swift#L3-L21 on and the copied code Cilicon/LeaseParser.swift#L63C1-L81:

image

@Marcocanc
Copy link
Member

@edigaryev thanks for reporting this. This code was actually not used. It's no secret that some of the code in Cilicon is heavily inspired by Tart. It's using its image format after all.
This specific piece of code was pasted into the file as a reference on which the functional implementation above it was inspired. Unfortunately we forgot to delete this piece of code which was never used. I have pushed a commit that removes it.

@fkorotkov
Copy link
Author

I guess LayerV1Downloader even has the same comments. Compare this in LayerV1Downloader and this in Tart's DiskV1.

It seems the issue dates back to 2.0.0 version of Cilicon and affects both 2.0.0 and 2.1.0 versions. To not break things for your users I guess we can ignore the past and let's make sure future versions of Cilicon are compliant. Please consider using Tart similar to how Tartlet is using it. Your users will benefit from upstream support and you won't need to keep up-to-date Tart compatibility.

@Marcocanc
Copy link
Member

Thanks for understanding @fkorotkov!
A completely new version of Cilicon is actually currently in the works which will rewrite a lot of its code (including those parts inspired/adopted from tart). I absolutely considered having Cilicon communicate with tart directly, but decided against it in favor of more flexibility for future features (better status monitoring, different ways to display the VM screen).
Thanks again for building tart, and especially for maintaining the images!

@fkorotkov
Copy link
Author

Can we keep the issue open until the issue is actually fixed?

@Marcocanc
Copy link
Member

Of course! Just to manage expectations:
As you can imagine, given the small NoL of the code most changes will consist of formatting, structure, variable naming and comments. Although I'm not a expert in this, I would argue that licensing cannot fully apply on snippets of code that consist mostly of calls to APIs of the platform SDK. In case you disagree I'd be happy to set up a call to discuss how we can make this work.

@Marcocanc Marcocanc reopened this Jan 16, 2024
@fkorotkov
Copy link
Author

I can refer you a lawyer that we consult with about such issues if you have doubts that licensing is applied on such snippets.

@Marcocanc
Copy link
Member

Marcocanc commented Jan 16, 2024

Please let me know if the current version is ok with you. Just to explain my previous point, I meant that (in my non-expert opinion) a certain methodology, especially when almost entirely relying on platform SDKs, is probably not covered by copyright/licensing. For example if a random engineer, who was to implement the same functionality based on the requirements (without seeing the code), would be likely to end up writing the exact or very similar code by coincidence.
I did not mean to say that licensing does not apply to identical blocks of code in your code-base.
Of course we want to make sure we're not violating your license, so just to be sure I will pass this on to our IP lawyer tomorrow.

@fkorotkov
Copy link
Author

Do you mean renames in ef87736 by the "current version"? Unfortunately I don't see how it's fixing the issue rather than just tying hide it.

I'd really like you to take this issue seriously. It's great that you have an access to an IP layer. Please consult with them about your "inspiration"/"adaption" technique of coping significant code snippets and what's the licensing implication of it.

@Marcocanc
Copy link
Member

I have removed the V2 downloader for now which should fix the license issues and will write one from scratch soon. Sorry for the inconvenience!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants