rclone icon indicating copy to clipboard operation
rclone copied to clipboard

anacrolix/dms

Open anacrolix opened this issue 5 years ago • 10 comments

I recently received https://github.com/anacrolix/dms/issues/73#issuecomment-606762705 to anacrolix/dms, and in following the link to https://github.com/rclone/rclone/blob/66e08e0cc8972000d84a61f262286f5644d9fdee/cmd/serve/dlna/cds.go found the code to be remarkably familiar. I see that this was copied in https://github.com/rclone/rclone/pull/2648. I believe the BSD 3-Clause license allows modifications, but requires a copyright notice whether modifications are made or not, and there does not appear to be any such license notice in rclone. The changes since made look very positive, and it would be great if an attempt was made to upstream those, as there is significant overlap with issues currently reported against anacrolix/dms, and the internals which are currently copied and modified, and still linked into are intended to be used more broadly such as they are in rclone. Thanks!

anacrolix avatar Apr 01 '20 07:04 anacrolix

Thanks for pointing out the licence oops! Will fixup ASAP!

@nicolov I think this was imported by you originally wasn't it?

@sretlawd I think you've made improvements to this - what do you think about upstreaming them?

Thanks

ncw avatar Apr 01 '20 11:04 ncw

Sorry for the license oversight, and thanks a lot for your efforts.

Yes, I did import the code originally to get streaming to work, but haven't made any of the improvements. Can I help out somehow?

nicolov avatar Apr 01 '20 21:04 nicolov

Happy to send up a patch. I don't currently have access to any Smart TVs, and no idea when I'll be going home, so I can't really do much at the moment..

ghost avatar Apr 13 '20 13:04 ghost

@sretlawd I don't really have access either, I expect it should be pretty straightforward to upstream many of your changes, the code hasn't changed much, if at all in many areas and the layout remains the same.

anacrolix avatar Apr 14 '20 00:04 anacrolix

@ncw could you link to any license/copyright that has been included as a result of this issue?

The anacrolix/dms project has recently received a lot of interest and user uptake. It's also now packaged by numerous Linux distros. It would still be great if many of the changes made here were upstreamed so that both projects can benefit.

anacrolix avatar Dec 22 '20 00:12 anacrolix

@anacrolix @nicolov @sretlawd Any updates?

ivandeex avatar Mar 31 '21 10:03 ivandeex

This is not waiting for upstream. I am waiting for this project to review the licensing, and/or upstream or contribute some changes back.

anacrolix avatar Apr 08 '21 23:04 anacrolix

As PR authors do not seem much interested in responding, I made a look into rclone git history.

  • Our use of anacrolix/dms started from PR #2648 by @nicolov that was merged into master on 2019-01-09 and released with rclone v1.46 The original code was vendored by @nicolov under rclone/vendor/github.com/anacrolix/dms without changes. The PR also added a glue code on top of anacrolix/dms under rclone/serve/dlna.
  • Later there was a PR #3045 by @sretlawd that made improvements to the glue code but did not touch vendor code.
  • Later commit https://github.com/rclone/rclone/commit/abb9f89f65bd609d7c0afd8a8f58a21bfae1a4f4 titled "update all vendor dependencies" that most probably was composed by an automatic golang vendoring tool, removed one of vendor files.
  • Later as rclone upgraded to go modules the commit https://github.com/rclone/rclone/commit/2b50d44a2f740e67b5fb2e83f7acab85ea94d256 on 2020-07-21 removed all remnants of anacrolix/dms and any other vendor code from rclone code base.

We continue to maintain some DLNA glue stuff under serve/dlna but we don't have any anacrolix/dms code in the rclone code base since release 1.52.3 even as a vendoring copy. We rather use go.mod as a reference to anacrolix/dms code on github.

Attaching relevant git log (output from git log -p -D -- vendor/github.com/anacrolix/dms/ on master branch) below. git-rclone-anacrolix-dms.log

cc @ncw @anacrolix

ivandeex avatar Apr 09 '21 15:04 ivandeex

@ivandeex my understanding is that this code https://github.com/rclone/rclone/blob/master/cmd/serve/dlna/cds.go - was copied into the repo by @nicolov but came originally from the @anacrolix repo

Here are the current versions for each - you can see the similarities

  • https://github.com/rclone/rclone/blob/master/cmd/serve/dlna/cds.go
  • https://github.com/anacrolix/dms/blob/master/dlna/dms/cds.go

So at minimum we should add the anacrolix copyright statment to the cds.go file.

However I think that anacrolix would like us to upstream the changes which seems like a good idea and I don't know why we didn't do that.

So an ideal resolution for this would be for us to upstream the changes then use the repo unmodified as a go dependency. That means anacrolix/dms gets the fixes from rclone, everyone gets improvements to the library and rclone gets to remove some code from its codebase.

ncw avatar Apr 10 '21 08:04 ncw

That would be ideal, @ncw !

anacrolix avatar Apr 13 '21 08:04 anacrolix

I still see no copyright licence or movement here. I have been remarkably patient on this. Here is a link to the LICENSE for anacrolix/dms: https://github.com/anacrolix/dms/blob/master/LICENSE. The specific point of interest is:

Copyright (c) 2012, Matt Joiner <[email protected]>.
All rights reserved.

Redistribution and use in source and binary forms, with or without
modification, are permitted provided that the following conditions are met:
    * Redistributions of source code must retain the above copyright
      notice, this list of conditions and the following disclaimer.
    * Redistributions in binary form must reproduce the above copyright
      notice, this list of conditions and the following disclaimer in the
      documentation and/or other materials provided with the distribution.

anacrolix avatar Dec 01 '22 22:12 anacrolix

Sorry @anacrolix - this keeps falling off my TODO list.

I propose to put your copyright into the rclone source files, with a note above it something like "this file contains code which was derived from github.com/anacrolix/dms which is under the following license"

Is that OK?

Which source files would you like to see it in?

Alternatively or as well I can add you to the rclone contributors list?

ncw avatar Dec 02 '22 14:12 ncw

I think all the unvendored files (it looks like you stopped vendoring?) from https://github.com/rclone/rclone/pull/2648 that are derived from anacrolix/dms (and any files that were subsequently spun out from those, there might be a few small ones due to refactors). The copyright notice in the root of cmd/serve/dlna, similarly to how you have done https://github.com/rclone/rclone/blob/master/cmd/bisync/LICENSE.cjnaz. Also add to the contributors list, I assume this is https://github.com/rclone/rclone/blob/master/docs/content/authors.md, where nicolov is included (whose only contribution is the mentioned PR).

To nicolov's credit a LICENSE file was included in the vendored code, albeit by an automated process, but that should have also been present in the altered code.

Thanks.

anacrolix avatar Dec 04 '22 01:12 anacrolix

@anacrolix I've had a go at that. Take a look at this commit and see if you are happy with it: 7149cfab7094af1c1ae74fbaa55dccdd88fb16e8

Thanks

ncw avatar Dec 06 '22 14:12 ncw

I think that will do. You can remove the trailing / on https://github.com/anacrolix/dms/.

anacrolix avatar Dec 07 '22 22:12 anacrolix

I've merged that (without the extra /).

Many apologies for not attributing it properly before.

ncw avatar Dec 09 '22 14:12 ncw