-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Remove GenerateHtml #13705
Comments
I was concerned with this initially, but the only code in drake is a print statement that gives people html code that, when evaluated, hits their public gojs server. We are not redistributing gojs in any way. I dislike the watermark, and don't see a way forward even if we wanted to pay, so prefer to use a different javascript library; but gojs was the fastest to get up and running with. |
Just to check, is there something more than issue-discussion that we can get as an outcome?
Also, @jwnimmer-tri, for the, uh, less savvy of us, can you post the license you reviewed, and perhaps the paragraphs that indicate issues? (either here in email chain?) EDIT: Ah, from the source, https://unpkg.com/gojs/release/go.js, I see the license: |
We can post in the forum if we feel it's ambiguous: Came across this post: Leading here: MIT is already listed... but not sure how it will transfer to use outside of the Underactuated/Manipulation classes for redistribution of Drake. Ultimately, it's unclear what distribution in 2.1 means, esp. in terms of using |
5min alternative search (y'all've probably already seen 'em): |
A few choice phrases from the license:
From Russ's comment above, I'd say we are "using" the licensed software, so we are bound by the agreement. Which agreement?
That's us.
There is no further definition of "evaluation purposes". My take would be that trying it out as we've done and checking to see if there is any reasonable licensing path we could take does count as "evaluation" so we are currently in compliance. At this point I would say the concept works, GoJS seems functional, but the license is prohibitive. So I think we need to actively pursue a remedy: either attempt to negotiate a suitable license from GoJS or find an open source alternative. My preference would be to stick with open source externals. |
It seems unambiguous that the trial period here lasts 30 days, so regardless of whether "evaluation purposes" were ambiguous, we, and any users of the functionality in Drake, could only use it for 30 days. Moreover, if "evaluation purposes" is ambiguous, that is good reason not to use it . Also it is not very user-friendly to effectively sign up some of our users to a trial/evaluation of commercial software without making that clear. We may need to pull release 0.20.0. |
For 0.20 retcon, I'd settle for updating the release notes (and GH announcement page) to call out the problem and discourage use of that feature (or possibly the whole release?). Once master is fixed, we should be able to tag 0.21 fairly quickly. Fixing master is probably also easy -- just rm the h and cc files + tweak the BUILD. Both of those changes could be a single PR today, and then release tomorrow. |
I am probably fine by that. Even if someone wanted to negotiate license terms, I think we would need to revert ASAP until such terms were agreed, so I do not see any reason to delay. |
I will sign up to help with the next release mechanics tomorrow, but can I ask @RussTedrake or @sherm1 or @DamrongGuoy to PR the removal ASAP today? Hopefully also you can edit the 0.20 release notes to add a "warning" announcement near the top about this. See the 0.19 notes for an example of the warning syntax in rst. |
I don't agree that there is any urgency to remove this if it is a legitimate evaluation attempt. And the evaluation period is indefinite, not 30 days (per the web site). Adding a note that the GoJS use in Drake 0.20 is for evaluation and will likely be removed later does seem sensible. Why the rush to remove it? |
My urgency ("today") was with respect to the 30 day limit claimed upthread -- I have not myself verified any of the claims upthread -- that one, or any other. Personally though, I do not want this defect to linger beyond a couple of days. The software developers are asking to be paid for their work. Ethically, we should either pay them, or not use their software. Using it in a Drake stable release is contrary to the spirit of their sharing, even if its within the letter of the agreements. If we plan to pay them, we can leave the evaluation use in place. But it sounded like we were not planning on doing that, in which case the sooner we remove the dependency, the better off we are. Git remembers what we've prototyped already, in case we decide to pay them later on, or rewrite the feature to use a foss library. |
FWIW From the license:
|
Also FWIW here is where they make it clear there is no time limit:
|
If someone has their own GoJS license, I believe they could add their own script tag to continue using GenerateHtml for now. |
Just to go on record here -- I've been watching this license discussion with interest (and will defer to the team's judgement). If we need to pull it, then so be it. @EricCousineau-TRI, I spent some time looking at alternatives before going with gojs and I agree they can work, and converting to them should be our strategy whether the conversion is from master or from the git history. |
To me, a forum header is not a sufficient grant of rights, especially to agree to on all of users behalf; the grant of rights is the license text and that limits us to 30 days.
To me, downloding code and running the GoJS code inside of our tutorial notebook is pretty clearly on the "the license matters" side of the line. In any case, presuming agreement to a non-foss license on our users behalf without sufficient disclaimers is a problem. If in the future someone wants to do the legwork to explain and document on master why this is okay, and then document in the release notes the new caveats to inform users, that's fine. But until someone retroactively does that legwork, I think we must err on the side of caution and retract the dependency for now. To me, shipping code with licensing problems is tantamount to breaking tests that are part of the nightly build, and requires an immediate mitigation and an updated stable release. |
Now that the problem is mitigated with the two PRs, this issue can remain open to track the work to either:
|
@jwnimmer-tri points out that graphviz has javascript support; the https://codefreezr.github.io/awesome-graphviz/#language-bindings has several libraries; some of them are interactive. |
I've opened #13874 to track the resolution of having a javascript diagram solution for drake. Will leave this issue open to track the cleanup of the GenerateHtml c++ code needed once we settle on that solution. |
Yes. Graphviz can perfectly do this. |
We have #13874 to track the goal of reincarnating this feature. I'll open the PR to remove it now. |
Edit: Original title Assess GoJS license terms
In #13644 we added a new, third-party dependency to Drake: GoJS.
I would have liked to see, as part of that PR review process, a license discussion as to whether or not Drake is allowed to depend on a non-foss library. In general, any new software we depend on should undergo that discussion (ideally, before we invest time working with it).
Having spent only a few seconds reading the license, my answer so far is "no" and we need to remove the dependency. Please discuss the particulars @RussTedrake and @sherm1 and (hopefully) convince me otherwise.
\CC @jamiesnape as #4045 subject matter expert.
The text was updated successfully, but these errors were encountered: