#2208 closed defect (fixed)
excluded from Debian because of no-source-code-included files
Reported by: | zooko | Owned by: | warner |
---|---|---|---|
Priority: | critical | Milestone: | 1.10.1 |
Component: | packaging | Version: | 1.10.0 |
Keywords: | Cc: | ||
Launchpad Bug: |
Description
According to the Tails developers (https://labs.riseup.net/code/issues/6227#note-23) Tahoe-LAFS v1.10 is about or already has been excluded from the upcoming new Debian release because of the presence of some code with no accompanying source: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=735940
Change History (39)
comment:1 Changed at 2014-03-25T13:06:54Z by zooko
comment:2 Changed at 2014-03-26T03:27:57Z by zooko
Let's start by writing a patch that removes the minified js files and adds the source js files. (Because that's easy, and it might help.)
The other possible things to do here I don't necessarily fully understand:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=735940#40
comment:3 follow-up: ↓ 5 Changed at 2014-03-29T19:17:21Z by daira
I suggest the following:
- Rename src/allmydata/web/static/d3-2.4.6.min.js to d3.min.js and src/allmydata/web/static/jquery-1.6.1.min.js to jquery.min.js.
- Replace d3-2.4.6.time.min.js with https://raw.github.com/jiahuang/d3-timeline/master/src/d3-timeline.js (not minified).
- Upgrade d3 to version 3.3.13 and jquery to 1.7.2. (This is to ensure that we're compatible with those versions.)
- Make the Debian package depend on libjs-d3 >= 3.3.13 and libjs-jquery >= 1.7.2.
- Patch the Debian package to remove src/allmydata/web/static/d3.min.js and src/allmydata/web/static/jquery.min.js.
- Patch the Debian installation script to symlink d3.min.js to /usr/share/javascript/d3/d3.min.js and jquery.min.js to /usr/share/javascript/jquery/jquery.min.js.
comment:4 Changed at 2014-03-29T19:42:22Z by daira
Note that steps 1-3 inclusive also fix #2122.
comment:5 in reply to: ↑ 3 ; follow-up: ↓ 7 Changed at 2014-03-30T03:41:22Z by zooko
Shouldn't we take the ".min" out of the filename if the content is not minified?
comment:6 follow-up: ↓ 8 Changed at 2014-04-01T02:27:13Z by anarcat
i would say yes: not-minified code shouldn't have the '.min' infix.
as for the steps, they make sense! however, i wonder what step 5 means, if those files are removed upstream already...
comment:7 in reply to: ↑ 5 Changed at 2014-04-03T00:18:13Z by daira
Replying to zooko:
Shouldn't we take the ".min" out of the filename if the content is not minified?
The files provided by the Debian libjs-d3 and libjs-query packages are minified.
comment:8 in reply to: ↑ 6 Changed at 2014-04-03T00:37:39Z by daira
Replying to anarcat:
i would say yes: not-minified code shouldn't have the '.min' infix.
Indeed not (and d3-timeline.js won't).
as for the steps, they make sense! however, i wonder what step 5 means, if those files are removed upstream already...
They wouldn't be removed upstream.
comment:9 Changed at 2014-04-07T23:01:30Z by daira
It appears (from smoke-testing) that d3-timeline.js is not the correct replacement for d3-2.4.6.time.min.js. I don't know where to find pre-minified source for the latter. (It is not part of d3 3.3.13.)
comment:10 Changed at 2014-04-08T03:57:35Z by warner
Right, it's from d3 2.4.6 (they must have merged it into the core codebase between 2.4.6 and the current 3.4.4).
I copied it from https://github.com/mbostock/d3.git (the official d3.js repo), tag "v2.4.6", the file named d3.time.min.js . So d3.time.js in the same repo+tag is the unminified source.
I'm cool with switching to non-minified versions. I'd suggest going all the way to the current release (v3.4.4) instead of stopping at 3.3.13 unless.. ohh, debian currently has 3.3.13? Or some specific debian target version has 3.3.13?
Anyways, as long as we switch to a sufficiently-new version of d3, we can get rid of the separate d3.time file.
comment:11 follow-up: ↓ 12 Changed at 2014-04-08T14:46:27Z by daira
Yes, the problem is that the libjs-d3 package in Debian provides only provides version 3.3.13 of d3 (see https://packages.debian.org/jessie/libjs-d3), and that is prior to d3.time having been merged in. So that package does not contain d3.time at all.
Therefore, although we might decide to upgrade to more recent d3 in upstream Tahoe-LAFS anyway, that wouldn't solve this ticket.
comment:12 in reply to: ↑ 11 Changed at 2014-04-08T14:56:54Z by daira
Replying to daira:
Therefore, although we might decide to upgrade to more recent d3 in upstream Tahoe-LAFS anyway, that wouldn't solve this ticket.
Or more precisely, it wouldn't solve this ticket if it is a requirement to use a Debian-packaged version of d3. If Debian is happy with us including unminified source, there's no problem. I will ask on the Debian ticket.
comment:13 follow-up: ↓ 14 Changed at 2014-04-08T18:54:06Z by warner
Huh, d3-v3.3.13 *does* have d3.time merged into the main d3.js file (https://github.com/mbostock/d3/blob/v3.3.13/d3.js#L8626). However the debian packaging of it does not (/usr/share/javascript/d3/d3.js looks like it's been partially minified, but not as much as the d3.min.js sitting next to it). I'm still trying to figure out how the debian package gets built: perhaps there's some bug therein which is failing to include everything that it ought to. In that case, the easiest path may be to get debian to fix the packaging.
comment:14 in reply to: ↑ 13 Changed at 2014-04-09T03:38:48Z by daira
Replying to warner:
Huh, d3-v3.3.13 *does* have d3.time merged into the main d3.js file (https://github.com/mbostock/d3/blob/v3.3.13/d3.js#L8626).
Oh. Well then why does the timeline fail to render correctly with the d3.min.js from here? (It renders just as a bunch of overlapping text in the top-left corner of the view.)
comment:15 Changed at 2014-04-09T03:41:15Z by daira
It does render correctly when d3-2.4.6.time.min.js is loaded after d3.min.js version 3.3.13 (which is the main reason why I thought d3.time wasn't included in the latter).
comment:16 Changed at 2014-04-09T06:00:27Z by warner
Yeah, the way d3 "plugins" work is to add properties to a pre-existing d3 object (basically monkeypatching), so order matters.
I assume this would fail if patched to use a system-provided (debian) libjs-d3 copy of d3.js, but I haven't actually tried it to see. It's possible that the debian copy *does* include d3.time, but the way it's built defies my grep abilities to see it.
comment:17 Changed at 2014-04-14T22:45:23Z by daira
- Milestone changed from soon (release n/a) to 1.11.0
- Owner set to daira
- Status changed from new to assigned
comment:18 Changed at 2014-04-14T22:46:26Z by daira
Putting this in the 1.11 milestone is optimistic because it may depend on cooperation and information from Debian developers and/or other package maintainers (libjs-d3 at least).
comment:19 Changed at 2014-04-21T21:49:47Z by daira
Zooko found the following comment in trac's spam bucket:
First, an aside: I am Ramakrishnan, a Debian developer with some interest in crypto and Functional Programming and file systems, which brought me to Tahoe-LAFS. I hope to look at this from a Debian perspective and work with you folks and get it resolved. As a first step, I filed a wishlist bug against libjs-d3 to upgrade it to the latest upstream release, which was promptly done by the libjs-d3 debian maintainer. I wish that we base further work on top of it, as it is the latest D3 release. If the debian package of D3 has anything lacking, let us fix that as well on the way. I hope to start looking at it from tomorrow.
comment:20 Changed at 2014-04-21T23:01:44Z by daira
- Keywords review-needed added
- Owner changed from daira to zooko
- Status changed from assigned to new
Review needed for https://github.com/tahoe-lafs/tahoe-lafs/pull/89.
comment:21 Changed at 2014-04-21T23:54:57Z by daira
$ diff --ignore-space-change ~/d3-3.4.6/d3.js src/allmydata/web/static/d3.js >d3.diff $ wc --lines d3.diff 15291 d3.diff
Sigh. I don't understand this -- how can there be 15291 lines of differences between upstream d3, and the Debian package with the same version number?
comment:22 Changed at 2014-04-22T00:00:32Z by warner
The d3 git repo includes three things: individual source files (275 .js files in src/), a merged "d3.js" file (330K, about 9k lines), and a merged+minimized "d3.min.js" (150K, 4 really long lines). Each time the author makes a change (or accepts somebody else's change), they also rebuild d3.js and d3.min.js and check in all three things at the same time.
I'm guessing that debian rebuilds their own merged d3.js from src/ , and manage to do it differently than the upstream author does. It'd be nice to fix that, to remove the difference that you see.
comment:23 follow-up: ↓ 24 Changed at 2014-04-22T02:36:00Z by daira
But why?
Why would anyone not understand the importance of being able to trace differences from upstream? This is not comprehensible to me.
comment:24 in reply to: ↑ 23 Changed at 2014-04-22T18:57:40Z by zooko
Replying to daira:
But why?
Why would anyone not understand the importance of being able to trace differences from upstream? This is not comprehensible to me.
So, as far as I understand we can avoid the need to understand such things by just not using minified, compressed, compiled, or bundled, versions of anything, unless in the future we discover a compelling need to do so. This also lets us avoid Debian's uncertainty about what source code we're shipping, in addition to our uncertainty about what source code they're shipping.
comment:25 Changed at 2014-04-23T10:01:12Z by vu3rdd
Perhaps one very simple to use and quickest fix in the Debian package would be to use the CDN:
<https://cdnjs.cloudflare.com/ajax/libs/d3/2.7.4/d3.time.min.js>
and remove the ones in src/allmydata/web/static altogether. This way, we can:
- live without any doubt on the js file included with the OS packages.
- stop shipping a particular version of the JS file with the Tahoe-LAFS.
- circumvent silly bugs like this one, which is holding off an otherwise great software from being distributed with a distro like Debian.
I can't find the version 2.4.6 that is used by the "download-status-timeline.xhtml". JQuery versions are here:
If this solution is acceptable, would any of you, authors of Tahoe-LAFS verify a version of d3 that works fine? I am asking you that instead of doing it myself because I am unfamiliar with Tahoe-LAFS in general and don't even have it setup on my end (which I am going to do this week).
After that I can easily incorporate the patch into the 1.10.0 debian package and upload it.
Thanks Ramakrishnan
edit: fixed a typo.
comment:26 Changed at 2014-04-23T11:43:24Z by daira
Using the CDN isn't acceptable for privacy reasons. Tahoe-LAFS shouldn't be making any requests to third parties (i.e. not servers that it has found via the configured introducer) at run-time.
comment:27 Changed at 2014-04-23T13:27:18Z by tsygrl
Tahoe-lafs should run with full functionality detached from the main internet.
comment:28 Changed at 2014-04-23T17:40:51Z by vu3rdd
Daira and I had a chat on the IRC today. I tried a few things today, here is the summary:
- The d3.js shipped with Debian's libjs-d3 package is different from the upstream d3.js.
- If I download the source and build it with 'npm install' and then 'make' (along with the Debian's nodejs package), it produces exactly the same js file as the one shipped with the upstream.
- Upon further inspection, looks like the uglify.js (I don't know what it does!) shipped with Debian and used by the libjs-d3 is quite old. D3 upstream needs 2.4.0 where as Debian has 1.3.4. Perhaps this needs to be fixed. I will raise a debian bug regarding this.
- I tried changing the dev dependency of the upstream sources of d3 to match that of the debian's dependency and build it but it didn't build for me with some weird error, which I didn't pursue further.
I wonder what can be done at this point. If Debian's current version of D3 and JQuery libraries work fine, then I could quickly change/patch the XHTML source to use those files in Debian (/usr/share/javascript/*), add dependencies on those libraries and do an upload. Alternatively, I could add a patch that removes the d3.min.js and jquery.min.js and add the non-minified versions.
Comments?
comment:29 follow-up: ↓ 30 Changed at 2014-04-23T21:23:23Z by daira
+1 on raising the Debian bug about uglify.js.
At this point I'm inclined to ship the upstream unminified versions of d3 and jquery, and ask on the Debian bug whether that is sufficient. For jquery there's a choice between 1.7.2 and the latest upstream 1.x release (1.11.0) which I'm agnostic about.
comment:30 in reply to: ↑ 29 Changed at 2014-04-24T02:05:57Z by vu3rdd
Replying to daira:
+1 on raising the Debian bug about uglify.js.
Done: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=745687
comment:31 Changed at 2014-04-25T22:18:26Z by warner
I'm +1 on shipping unminified versions. I'd lean towards updating to the latest upstream (I expect v2.1.0 should be fine, nothing in the timeline code I wrote uses anything fancy, and it should be pretty quick to test that the newer version didn't break anything).
comment:32 Changed at 2014-04-25T22:39:32Z by warner
- Owner changed from zooko to warner
- Status changed from new to assigned
I'll work on this. Preliminary tests suggests that moving to current d3-3.4.6 causes zoom to break. Also I'm going to stick with jquery-1.x (1.11.0) because jquery-2.x dropped support for IE6/7/8, which I'm willing to do but maybe after the next release.
comment:33 Changed at 2014-04-25T23:23:52Z by warner
I pushed a branch (using d3-2.4.6 and current upstream jquery-1.11.0): https://github.com/tahoe-lafs/tahoe-lafs/tree/2208-unminified-js
.. and then I noticed that daira had already pushed a branch with d3-3.4.6 and jquery-1.7.2 . The latter should work ok, but in my tests the newer d3 doesn't.
Daira: did your branch handle zoom/pan properly? Any thoughts about which branch to use?
comment:34 Changed at 2014-04-26T23:13:16Z by Brian Warner <warner@…>
comment:35 Changed at 2014-04-26T23:15:54Z by Brian Warner <warner@…>
comment:36 Changed at 2014-04-26T23:18:21Z by daira
warner: my branch did indeed cause zoom to break, so I reviewed and merged yours.
comment:37 Changed at 2014-04-30T02:35:28Z by zooko
- Resolution set to fixed
- Status changed from assigned to closed
Okay, I believe this fixes the original problem of "no-source-code-included files", because now we bundle a copy of the original source (identical to upstream) instead of a minified version. Closing this ticket.
comment:38 Changed at 2014-05-02T18:55:37Z by vu3rdd
Uploaded into Debian unstable: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=735940#106
comment:39 Changed at 2014-05-03T22:59:20Z by daira
- Keywords review-needed removed
For the record, we wrote down our thoughts about this packaging issue when we first introduced those files:
https://tahoe-lafs.org/trac/tahoe-lafs/ticket/1200#comment:8
Would it satisfy the Debian policy if the files were stored in source form instead of minified form?