Add http-parser license and authors #251
Conversation
Motivation: Quote: > The above copyright notice and this permission notice shall be included in > all copies or substantial portions of the Software. Just linking is not enough. Modifications: Added the http-parser license. Added the AUTHORS file. Not required by the license, but the right thing to do. Result: Proper license inclusion for http-parser.
|
Can one of the admins verify this patch? |
|
@helje5 dont we actually already doing this here? : https://github.com/apple/swift-nio/blob/master/NOTICE.txt#L47 and here: |
|
I think including the authors is really not needed. |
|
@swift-nio-bot please test |
|
@swift-nio-bot test this please |
|
Well, the NOTICE is IMO definitely not an inclusion as required by the license. But yes, the one in the source itself is fine. As I said, you don't have to include the AUTHORS, but I think it is not considered nice to leave it out. In the end: What do you loose by not including those files (and maybe even the README from upstream). 3 files which are not included in the build and 4KB of data? 2ms when cloning the repo? |
|
@normanmaurer I thought that what we had so far contains everything that's needed but IANAL. I think we should delegate the question if we're currently missing anything to the lawyers. |
Yes, it looks like you have everything you need (LICENSE also embedded directly in the source, I missed that). |
Apple lawyers have already poured over every inch of this codebase and subsequently approved it. I don't think it's a good idea for us developers to make any changes to licensing after that. |
|
@normanmaurer ping, what should we do here? |
|
Can one of the admins verify this patch? |
1 similar comment
|
Can one of the admins verify this patch? |
|
IMHO we should just close it. |
|
IMO you should just do the right thing. Would you like to see your own CONTRIBUTORS.txt being dropped when someone copies your repro? Most certainly not! If you close this it sends a very clear message and would make me very unhappy. 😔 |
|
@helje5 if someone takes NIO and integrates it into their project I would expect them to drop our contributors file but mention that they use NIO. That’s what we did too. We could add to the contributors file that it of course contains contributions from countless other people. |
|
If this is your opinion, go ahead and close this, it is your project. I cannot imagine a single reason for not including the contributor (and license) files alongside a straight copy of another project. They are isolated in the copied directory, they do not add bulk, they do not slow down compilation time, they just show that you properly appreciate the work of OpenSource contributors. Or the opposite. 🤷♀️ |
|
@helje5 fair enough but how much should we add? Should also add the freebsd authors, Swift, llvm, ... |
|
You don't embed and ship FreeBSD, Swift nor LLVM. You don't even pull any of the stuff in in some automatic way. The other stuff the user gets herself and with it all the acknowledgements the projects desire or require. E.g. Swift.org is actually a good example for a project which is rather strict. http_parser (like NIO) has made very clear by having a contributors.txt, that this aspect is important to them. Otherwise it wouldn't even exist. So if you use that software (for free, produced by contributors like me), please just include those 1, 2 or 3 text files. It is a no-brainer. That we even have to have that discussion is making me quite sad. |
|
@helje5 I don't think an AUTHORS file is meant to be used like that. From what I can tell, NodeJS itself does it just as we do (except that we're still lacking an automatic update script which we should fix): https://github.com/nodejs/node/blob/master/tools/update-authors.sh So they wouldn't mention http_parser and on top of that they pull in all of this and don't mention it in their AUTHORS file. IMHO the authors file is meant for authors that worked on the project you navigated to. And pretty much everybody has dependencies and it'd be a long list if you mention them all. We mention (in NOTICE) that we pull in FreeBSD's SHA1 implementation, http_parser and a few other things, and I guess that makes it obvious that their authors have contributed transitively to NIO too, no? Do you have a good example for a project that pulls all transitive authors in automatically? |
I think all this is pretty clear and obvious, but all I ask for is preserving license/author files within the cloned upstream directory. (If you copied the SHA1 sources, I would also place the related documents alongside it, yes). The more credits you give, they happier I would personally be :-) But I really wouldn't expect you to do unreasonable amounts of extra work, but just preserving the upstream setup and not (unnecessarily!) drop anything along the copying.
AUTHORS files exist like forever. They are the counterpart to old-school ChangeLog files and are here for exactly this scenario. You pull the sources out of their some repository and put them somewhere else. Nowadays this is a little less common because w/ GitHub the upstream repositories are more stable, direct forking is available and pro-active removals like that are clearly visible in forks (which is why no one would ever dare to do that). Yet sometimes people still directly pull in small projects like http_parser or Expat, this is what ChangeLog/AUTHORs are good for. BTW: For this particular package the cleanest approach would have been: fork the upstream, enhance w/ the SPM setup. Then import that package into NIO via SPM. Anyways, I'm tired of the discussion - I find it pretty ridiculous, but as mentioned: your project, your choice. I still hope you do the right thing, but I'm not overly convinced ;-) |
|
@helje5 ah, sorry I think I misunderstood you. I'd be up for copying their |
|
@helje5 sorry, I re-read your PR and it does also does add their AUTHORS into the directory that contains the 3rd party dependency. For some reason I never saw that and assumed that you argued for adding all the transitive authors to our CONTRIBUTORS file which I don't think is great. If you modify the download & patch script to pull those files in, I'd be happy taking this change. @normanmaurer / @Lukasa / @tomerd what do you think? |
|
No objection from me. |
|
@weissi Yes, this PR is only about adding the two text files to the I'll have a look at your script. Though what about my suggestion of a separate SPM repo, as a clone of upstream? It makes updating the thing more standard. If you want, I'd prepare that for you. |
Motivation: When the sources are updated from upstream, we also need to update the license/authors files. Modifications: Curl the files, output to current directory. Result: Related files are updated alongside the sources. Note: I couldn't fully test, because neither me nor brew have `gsed`.
|
I modified the script, but couldn't really test it. Where do you get |
|
@helje5 we can't use the upstream repo as we need to rename the symbols (
|
Motivation: If the user misses gsed, let him know how to get it. Modifications: Added an echo pointing the user to brew install gnu-sed Result: User will have a way to run this script.
Motivation: Would be nice if the script would actually work. Modifications: Put the script calls in the proper sequence. Result: The update script can be called and it actually worx.
|
OK, I fixed the script, it now seems to work fine (also: there is a new version available which adds a new HTTP method SOURCE - surprise, surprise ...) So if you want, I think you should be able to merge this in. |
|
@swift-nio-bot test this please |
|
@swift-nio-bot test this please |
6 participants
Motivation:
Quote (LICENSE-MIT):
Just linking is not enough, I think. You have the better lawyers, but it would be the proper thing to do in any case.
Modifications:
Added the http-parser license.
Added the AUTHORS file. Not required by the license, but again the right thing to do.
Result:
Proper license inclusion for http-parser.