TauLabs / TauLabs

taulabs.org
Other
455 stars 393 forks source link

Backport from dRonin #2134

Open kubark42 opened 8 years ago

kubark42 commented 8 years ago

Reviewed and submitted as PRs.

peabody124 commented 8 years ago

For my understanding, most of the no brainers are also included in #2127, right?

kubark42 commented 8 years ago

@peabody124 Yes. The initial list was populated from #2127. The items that we instantly agree on I labeled "no-brainers". I've not been able to go through the entire list yet and review so there are still probably some "no-brainers" that I haven't seen yet.

mlyle commented 8 years ago

@kubark42, @peabody124 -- I'm really concerned because both of you stated that you would respect the copyright and license requirements on our work and all of these PRs just merged without retention of the copyright notices that are in our tree.

I understand that things will not be perfect, but there have been a very large number of dRonin commits merged without this happening.

peabody124 commented 8 years ago

@mlyle reading up on this a bit more (e.g. https://softwarefreedom.org/resources/2012/ManagingCopyrightInformation.html) it is not a rigid requirement to manually copy over things like the doxygen authorship line However, a requirement to “preserve” or “reproduce” a developer’s copyright notice does not necessarily require that the notice be kept in exactly the same place it started; it’s usually acceptable to move notices from individual source files to a central attribution file, for example.

Also a large number of these commits were done without preceding or integrated changes to the authorship line either.

TL;DR: nothing done so far is a GPL violation AFAIK

@kubark42 That being said I think adding those lines is the right thing to do. Once we get all this done we should either at the same time or in an additional pass have a PR that adds the appropriate lines, at least where they existed in the DR repo at the time of the commit.

mlyle commented 8 years ago

@peabody124 All of those files have copyright notices in our tree. Just because you chose to cherry-pick different commits that don't have them makes it OK.

I don't think you have very firm ground to stand on-- I am asking you to maintain the copyright notices where they appear in dRonin code, and where the Tau Labs project itself chooses to put copyright notices-- you've chosen the file-scoped approach.

Once we get all this done we should either at the same time or in an additional pass have a PR that adds the appropriate lines, at least where they existed in the DR repo at the time of the commit.

If they presently exist in our tree-- the copyright notice should be added. Failure to maintain attributions from picking from points in the past which were not the change that added the notice puts the entire project on a questionable legal footing-- including any downstream users.

In fact, I'm somewhat concerned that we @ dRonin could be vulnerable if shoddy practices like this were followed in the past.

kubark42 commented 8 years ago

@peabody124 agreed there's no sense in making drama out of this, we're all friends here. I think you addressed in https://github.com/TauLabs/TauLabs/pull/2127 that we would do an atomic copyright fix once we've sorted through what we do and don't want. This is a sensible approach because it limits the number of commits.

mlyle commented 8 years ago

@kubark42 It is "ok" if you guys are going to take a few days to do it-- as long as you're intending to update all associated files and not try to skip places where the copyright notice was added a few revs later or earlier. I'm concerned with this approach things are likely to get lost, though.

kubark42 commented 8 years ago

@mlyle We always do our best to make sure no copyright accidents happen. When they do, we resolve them as quickly and painlessly as possible. I'm sure you can appreciate this, since dRonin also accidentally placed a copyright in some Google files. Picking a fight on spurious grounds that a 24 hour cycle without copyright commits is "concerning", ignores that we have specifically laid out our plans about how to address those concerns in an earlier comment.

I don't see how this conversation is to anyone's benefit, so let's keep this issue on focus, please.

mlyle commented 8 years ago

@kubark42 peabody124 is arguing above that you guys don't need to do it and alternatively suggesting to attempt to cherry pick through and find places where we were missing the attributions at the time of commit. And in the meantime they're in the tree without attribution. We could send a DMCA notification right now and have the Tau Labs repository taken down. We are showing restraint.

If you're going to mark up all the files that you took changes from dRonin and get that PR pushed through in some reasonable time-- like in the next week-- that's great and I think we have no problem. I still think it's better to do it at the same time to avoid omission or confusion.

peabody124 commented 8 years ago

@mlyle I was pointing out that maintaining file-scoped attribution isn't considered necessary (at least per that source), and certainly adding it when they don't exist in the other tree wouldn't be mandated. I also said it is the right thing to do and we should just do it.

mlyle commented 8 years ago

I'm sure you can appreciate this, since dRonin also accidentally placed a copyright in some Google files.

Totally OK, dude! We changed the file and hold copyright also, and we didn't remove their notices. But it was dumb to cause an additional file-level conflict with a different upstream that would make it hard to pick up additional upstream code in the future. You can always add notices.

If you look over our PR history and our issues, you can see that this is an area (respecting upstream/other copyrights) that is important to us and where we've expended significant effort.

ignores that we have specifically laid out our plans about how to address those concerns in an earlier comment.

Except it didn't actually happen that way-- we thought it was going to come in, in the same PR, and it didn't. In the meantime anyone who pulls the code now will get a copy of all these files without our notice.

@mlyle I was pointing out that maintaining file-scoped attribution isn't considered necessary (at least per that source), and certainly adding it when they don't exist in the other tree wouldn't be mandated. I also said it is the right thing to do and we should just do it.

It exists in our tree currently. Over these files, it was added before or sometimes after the individual commits. Please add it in all cases, and everyone will be happy.

kubark42 commented 8 years ago

I'm sorry our process confused you. Since this is clearly very important to you, the fastest way to resolve it would be to open a PR so we can get to it earlier. Otherwise, we will address it once we're done with our sync'ing operation, which should be soon.

mlyle commented 8 years ago

Since this is clearly very important to you, the fastest way to resolve it would be to open a PR so we can get to it earlier. Otherwise, we will address it once we're done with our sync'ing operation, which should be soon.

No, the burden of maintaining Tau's copyright compliance does not fall on us.

kubark42 commented 8 years ago

Nope, but since you've involved yourself in the process to help maintain our copyright compliance I thought I'd offer you the chance to get the PR in sooner. Look for a PR landing soon!

mlyle commented 8 years ago

Yup, as long as you guys recognize you're out of compliance now.

peabody124 commented 8 years ago

I opened https://github.com/TauLabs/TauLabs/issues/2155 to consider actually moving system-scoped CR information instead of per-file. This might make life easier.

mlyle commented 8 years ago

While we're discussing this, I'd also like to point out #1998 -- I'd appreciate a resolution to this because dRonin is also out of compliance because of this issue. (Just add the appropriate attribution to it?)