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

Issues with code drop #1

Closed
pfalcon opened this issue Nov 15, 2014 · 6 comments
Closed

Issues with code drop #1

pfalcon opened this issue Nov 15, 2014 · 6 comments

Comments

@pfalcon
Copy link

pfalcon commented Nov 15, 2014

There're couple of issues with the code import:

  1. Files have mix of proper (LF) and improper (CRLF) line-endings. This makes it hard to work with the sources under Unix. Yes, that's how it was in original Tensilica stuff, but everyone knows that vendor code sucks, and real open source makes it better, even by fixing broken line-endings.
  2. (Single) commit message claims it's code from Tensilica/Cadence RC-2010.1 release. But that's clearly not true. Their code did not contain autoconf stuff and claims of copyright by "The ESP8266 Community".
@tommie
Copy link
Owner

tommie commented Apr 11, 2015

Sorry I haven't responded.

  1. I wanted the files to be traceable by having the same checksums, and I also don't want to modify the files unless we really have to, to preserve that traceability. Is this causing issues?
  2. The message says (very briefly) that the code contains RC-2010.1 files, not necessarily that all files in there are from RC-2010.1. I didn't really plan on anyone else using this. The additional copyright was added to cover non-upstream files.

@pfalcon
Copy link
Author

pfalcon commented Apr 11, 2015

Sorry I haven't responded.

I can't believe you did ;-). I surely don't remember each unanswered ticket but recently reviewed upstreams of my project (https://github.com/pfalcon/esp-open-sdk), saw bunch more unanswered tickets and thought "d'oh" ;-). So, thanks for response!

I also don't want to modify the files unless we really have to, to preserve that traceability. Is this causing issues?

Well, on Linux many editors show ^M's for line-endings and it doesn't look too well. Then of course these files aren't really intended for editing... So, I expressed my opinion, I'd convert line-ending to "canonical" form, which then would be converted to local system's conventions on git clone (as far as I know). You give good reasons for not doing that, and well, you're the maintainer, so you're to decide what to do.

The message says (very briefly) that the code contains RC-2010.1 files, not necessarily that all files in there are from RC-2010.1.

Well in point 1, you argue of importance to maintain original file checksum, and the same time don't pay much attention to maintain clear separation of the origin of the files ;-). I'd do 2 separate commits: one verbatim import of vendor sources (but I'd clean up line-endings per above), and a separate commit with my additions and my copyright on those additions (and "The ESP8266 Community" is a bit vague for copyright owner).

So, well, I don't know, at the time of the original report, when I set up esp-open-sdk, I was keen to do it very clean, paying thorough attention to origin and copyright of any upstream files, in case there will be any unfriendly actions from the ESP8266 vendor, so I actually was ready to setup my own hal project which wouldn't be liable to (or for actions of) a mythical "The ESP8266 Community". We know that it all went pretty well, so I don't want to raise older concerns too much. If you don't think it's worth time/effort to re-do anything now, feel free to close this ticket. But then I reserve right to make my own hal project if any issues arise in the future, and don't want such case to be seen as ignoring and forking community efforts - only as means to protect myself and users of my downstream projects from accusations of copyright violations, etc. (And I did notify the existing hal project of such concerns beforehand.)

@tommie
Copy link
Owner

tommie commented Apr 11, 2015

I can't believe you did ;-).

TIL GitHub doesn't set Watch automatically on repos you create unless you have the right default setting for notifications. :\

I'd do 2 separate commits: one verbatim import of vendor sources (but I'd clean up line-endings per above), and a separate commit with my additions and my copyright on those additions (and "The ESP8266 Community" is a bit vague for copyright owner).

Yes, that was sloppy, I agree.

But then I reserve right to make my own hal project if any issues arise in the future, and don't want such case to be seen as ignoring and forking community efforts - only as means to protect myself and users of my downstream projects from accusations of copyright violations, etc. (And I did notify the existing hal project of such concerns beforehand.)

I don't have any objections. I verified the files imported had a FOSS license. Apart from that, I just didn't care much about licensing, aside from making sure there was an explicit license given for everything.

I could change the copyright owner to a physical person if you'd prefer. I guess splitting the initial commit is a bit hard now if others are using the project. We could have a detached "vendor" branch, though. That would also allow line-ending normalization.

@pfalcon
Copy link
Author

pfalcon commented Apr 13, 2015

TIL GitHub doesn't set Watch automatically on repos you create unless you have the right default setting for notifications. :\

Well, for me it exactly sets myself, a creator, as a default watcher for each newly created project.

I could change the copyright owner to a physical person if you'd prefer. I guess splitting the initial commit is a bit hard now if others are using the project. We could have a detached "vendor" branch, though. That would also allow line-ending normalization.

I appreciate your paying attention to and taking care of these issues. Rebasing the repo as would be required to split the commit is apparently not to good now. Maintaining 2 branches is probably not ideal either, but OTOH, there unlikely would be much if at all "maintenance", maybe another code drop if we get newer release. Then, doing it "right" in newer branch may be not too bad an exercise, if you have some time for it. Changing copyright would help too - there's certainly a community, but a good community consists of individual parties (persons or more or less official organizations), has transparency, traceability, proper attribution to individual members, etc.

Thanks again!

@tommie tommie closed this as completed in ecdc989 Apr 19, 2015
@tommie
Copy link
Owner

tommie commented Apr 19, 2015

I've created a vendor branch, reimported the files from sources, fixed line endings and made a merge commit that overwrites the previous files. Hopefully this will not cause too many problems for forks, but it's a one-off either way.

Also made the copyright more specific.

@pfalcon
Copy link
Author

pfalcon commented Apr 26, 2015

Thanks! I pulled these changes to esp-open-sdk.

As a bit of out-of-band, some other info: I have some (preliminary) info on additional peripheral address ranges. As https://github.com/esp8266/esp8266-wiki/wiki is closed for editing, I pushed them to my fork. Specific changes are: https://github.com/pfalcon/esp8266-wiki/wiki/Memory-Map/_compare/3debc5aa8c69a525101490cd446b2aed98e750ec...1f28d4e19927bbaa9ff60f5dab87d2d950cf1b7d . Feel free to merge them upstream or just FYI.

Another piece is: https://github.com/pfalcon/ScratchABit . Motivation for writing that was exactly to allow community-wide hacking in "right way" (doing open-source with open-source) on ESP8266 and other similar projects where vendors can't get it right, like http://www.bunniestudios.com/blog/?p=4297 . It can be used with https://github.com/themadinventor/ida-xtensa right away (well, with a trivial patch, themadinventor/ida-xtensa#3)

@nekromant , @jcmvbkbc : You may be interested in the above too ^^^

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

No branches or pull requests

2 participants