Bug#721567: [PATCH] Re: photofloat: should use separately packaged libjs-* packages (not include convenience code copies)
Quoting Jerome Charaoui (2014-01-27 06:37:13) That should do the trick. Antoine, I should note that instead of creating a distinct update-photofloat-js as we discussed, I reverted to shipping the upstream Makefile and simply call that to minify+bundle js. Shipping the makefile cause the binary package to depend on make. That's inelegant but more importantly it raise the risk of flaws e.g. in how errors are handled. ...and regarding errors: Seems you only only recommend yui-compressor, but the makefile fails hard if that isn't available. ...and regarding yui-compressor: Dependin on that at runtime pulls in Java, which I find inelegant and annoying. Please consider use the scss tool (from package ruby-sass) instead: the SCSS dataformat is a superset of CSS so you can simply use plain CSS as input. If the thought of then pulling in Ruby displeases you, then consider libtext-sass-perl or python-pyscss. ...and regarding CSS compression: Please consider not compressing at all but only cat'ing those files (or use sass --style expanded during build, which then serves to also checks validity of that code) and install it below /etc, and put a symlink at the original location, to allow system owner to customize default look. Oh, and while I am at it - it seems you don't need TeX with lmodern, and thus should depend not on lmodern but fonts-lmodern. If you insist on using make, I suggest to invoke it in make fashion: make -C /usr/share/photofloat/web js/scripts.min.js - Jonas -- * Jonas Smedegaard - idealist Internet-arkitekt * Tlf.: +45 40843136 Website: http://dr.jones.dk/ [x] quote me freely [ ] ask before reusing [ ] keep private signature.asc Description: signature
Bug#721567: [PATCH] Re: photofloat: should use separately packaged libjs-* packages (not include convenience code copies)
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 Le 2014-01-27 06:07, Jonas Smedegaard a écrit : Shipping the makefile cause the binary package to depend on make. That's inelegant but more importantly it raise the risk of flaws e.g. in how errors are handled. ...and regarding errors: Seems you only only recommend yui-compressor, but the makefile fails hard if that isn't available. In the interest of keeping maintenance down, I thought it was better to rely on upstream code rather than ship a custom script. In this state, the Makefile only fails in the absence of yui-compressor if the user modifies the CSS. Not ideal, for sure, but not package-breaking either. ...and regarding yui-compressor: Dependin on that at runtime pulls in Java, which I find inelegant and annoying. Please consider use the scss tool (from package ruby-sass) instead: the SCSS dataformat is a superset of CSS so you can simply use plain CSS as input. If the thought of then pulling in Ruby displeases you, then consider libtext-sass-perl or python-pyscss. ...and regarding CSS compression: Please consider not compressing at all but only cat'ing those files (or use sass --style expanded during build, which then serves to also checks validity of that code) and install it below /etc, and put a symlink at the original location, to allow system owner to customize default look. You've already raised most of these (good, imo) ideas in bug #721568. This bug report and patch are aimed are improving the javascript situation. Please don't assume that we are ignoring the rest of your suggestions just because everything doesn't get fixed in a single patch. :) - -- Jerome -BEGIN PGP SIGNATURE- Version: GnuPG v1 Comment: Using GnuPG with Icedove - http://www.enigmail.net/ iQEcBAEBCAAGBQJS6Tu6AAoJEK/ZGpI6kvtMHdgH/jHEs0RidAyMM/sRw/STT1yY TCe6goR20UFsKDU/lTfyhAHGwgCZSv5UNiVTXNfsowearbo6muMSHJjX+PGLwViI MVDRB7zNHGD+VYlD5YLVUq12vCOMbupA1QdJwTQxjDTmuYvV4C3JddlrHPFKD4sZ x4QnTVDfo9OHixcLVTst2OIb9izmOM2xgCzfiTnmLO7K6cm/SzQdt5rpcE6L+OYr pQjRx9weMDDE0GFP7rNq0hnHgwh3XKmd0sRJPdtkUNtSoMVzVbzkaWutwR/YVHTf TtiPrTKxQ3jUs2auKdhiihJb8dLjOYYZCPsghMNECcxEYFCeZ8NesCe0QAwJZfk= =zAHO -END PGP SIGNATURE- -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#721567: [PATCH] Re: photofloat: should use separately packaged libjs-* packages (not include convenience code copies)
Quoting Jerome Charaoui (2014-01-29 18:35:23) Le 2014-01-27 06:07, Jonas Smedegaard a écrit : Shipping the makefile cause the binary package to depend on make. That's inelegant but more importantly it raise the risk of flaws e.g. in how errors are handled. ...and regarding errors: Seems you only only recommend yui-compressor, but the makefile fails hard if that isn't available. In the interest of keeping maintenance down, I thought it was better to rely on upstream code rather than ship a custom script. In this state, the Makefile only fails in the absence of yui-compressor if the user modifies the CSS. Not ideal, for sure, but not package-breaking either. Fair enough. Just wanted to point it out (lazily: didn't check myself closely) in case you'd missed something. ...and regarding yui-compressor: Dependin on that at runtime pulls in Java, which I find inelegant and annoying. Please consider use the scss tool (from package ruby-sass) instead: the SCSS dataformat is a superset of CSS so you can simply use plain CSS as input. If the thought of then pulling in Ruby displeases you, then consider libtext-sass-perl or python-pyscss. ...and regarding CSS compression: Please consider not compressing at all but only cat'ing those files (or use sass --style expanded during build, which then serves to also checks validity of that code) and install it below /etc, and put a symlink at the original location, to allow system owner to customize default look. You've already raised most of these (good, imo) ideas in bug #721568. Oh, right. Silly me. This bug report and patch are aimed are improving the javascript situation. Please don't assume that we are ignoring the rest of your suggestions just because everything doesn't get fixed in a single patch. :) It was a simple case of failing memory :-) - Jonas -- * Jonas Smedegaard - idealist Internet-arkitekt * Tlf.: +45 40843136 Website: http://dr.jones.dk/ [x] quote me freely [ ] ask before reusing [ ] keep private signature.asc Description: signature
Bug#721567: [PATCH] Re: photofloat: should use separately packaged libjs-* packages (not include convenience code copies)
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 Le 2014-01-27 11:28, Antoine Beaupré a écrit : On 2014-01-27 11:21:19, Jerome Charaoui wrote: diff --git a/debian/rules b/debian/rules index 118de9d..49f0824 100755 --- a/debian/rules +++ b/debian/rules @@ -20,11 +20,5 @@ export DH_OPTIONS %: dh $@ --with=python2 -override_dh_auto_build: - dh_auto_build - ln -s /usr/share/javascript/jquery/jquery.js $(CURDIR)/web/js/000-jquery-1.7.2.js - ln -s /usr/share/javascript/jquery-mousewheel/jquery.mousewheel.js $(CURDIR)/web/js/003-mousewheel.js - cd $(CURDIR)/web $(MAKE) - override_dh_install: - dh_install -X.gitignore -XMakefile -Xutils/ + dh_install -X.gitignore -X*.min.js -Xutils/ Does that mean we still build the minified versions during build, but then don't ship them? Shouldn't just skip that? I removed the whole override_dh_auto_build code block, so the build-time minify doesn't happen anymore. Hum.. I would have thought that without the override, dh_auto_build would still get called and hit the makefile and so on... I've tested with dpkg-buildpackage and it doesn't seem to hit it, probably because the file is in web/ instead of the root of the package source. Sounds pretty reasonable to me! However upstream uses google-compiler, so the change would look like : - -JS_COMPILER = utils/google-compiler --warning_level QUIET +JS_COMPILER =? utils/google-compiler --warning_level QUIET Understood. Can you take care of posting the patch upstream as well? Done! - -- Jerome -BEGIN PGP SIGNATURE- Version: GnuPG v1 Comment: Using GnuPG with Icedove - http://www.enigmail.net/ iQEcBAEBCAAGBQJS50xsAAoJEK/ZGpI6kvtMlNQH/iecsC4YuPgZsG0kc9ECOmR9 bk4z5qKi8E8lebdJxV+PiZmsfZAoEDRNnNR+jO2vZpYWJWoFq7Uw1l0b9eXwLchM lR8kFPepPf8QV2Tc03uWXjJsSVCcwcgJ/2uzjfXYZgzj+doNxBXGY+TL9PCUfeDr BvkaJE4K9DW8uWTP5jqPEMl+gs7sgpZxZMbQHUIuEj50hCuKP54M2+fVlMDQgvLR Kzs5q4BvVXD3L5ksB5FPiGqUFv6VxpGRmjs0eyQJLt3MRxmXsp6WEe78AaTgp2VK OUiNF/G0cD/DRx45My3MIPOFTCANIDJnsO1ohHm5WLayRPtawpwS+o0LVBm6I+g= =H3NS -END PGP SIGNATURE- -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#721567: [PATCH] Re: photofloat: should use separately packaged libjs-* packages (not include convenience code copies)
On 2014-01-27 00:37:13, Jerome Charaoui wrote: That should do the trick. Antoine, I should note that instead of creating a distinct update-photofloat-js as we discussed, I reverted to shipping the upstream Makefile and simply call that to minify+bundle js. Oh yeah, good idea. Quick review of the patch... diff --git a/debian/photofloat.triggers b/debian/photofloat.triggers new file mode 100644 index 000..db0ecc7 --- /dev/null +++ b/debian/photofloat.triggers @@ -0,0 +1,2 @@ +interest /usr/share/javascript/jquery/jquery.js +interest /usr/share/javascript/jquery-mousewheel/jquery.mousewheel.js That seems absurdly easy. Did you actually try this out and it works? :) diff --git a/debian/rules b/debian/rules index 118de9d..49f0824 100755 --- a/debian/rules +++ b/debian/rules @@ -20,11 +20,5 @@ export DH_OPTIONS %: dh $@ --with=python2 -override_dh_auto_build: - dh_auto_build - ln -s /usr/share/javascript/jquery/jquery.js $(CURDIR)/web/js/000-jquery-1.7.2.js - ln -s /usr/share/javascript/jquery-mousewheel/jquery.mousewheel.js $(CURDIR)/web/js/003-mousewheel.js - cd $(CURDIR)/web $(MAKE) - override_dh_install: - dh_install -X.gitignore -XMakefile -Xutils/ + dh_install -X.gitignore -X*.min.js -Xutils/ Does that mean we still build the minified versions during build, but then don't ship them? Shouldn't just skip that? diff --git a/web/Makefile b/web/Makefile index 72aea0d..8224ac5 100644 --- a/web/Makefile +++ b/web/Makefile @@ -7,7 +7,7 @@ CSS_MIN = $(CSS_DIR)/styles.min.css JS_MIN_FILES := $(sort $(patsubst %.js, %.min.js, $(filter-out %.min.js, $(wildcard $(JS_DIR)/*.js CSS_MIN_FILES := $(sort $(patsubst %.css, %.min.css, $(filter-out %.min.css, $(wildcard $(CSS_DIR)/*.css -JS_COMPILER = yui-compressor --type js +JS_COMPILER = uglifyjs CSS_COMPILER = yui-compressor --type css .PHONY: all clean Another patch with upstream, but nothing in debian/patches. I think we should instead use: JS_COMPILER ?= yui-compressor --type js ... to respect upstream's default yet allow ourselves an override. Then we just call make with: cd /usr/share/photofloat/web make JS_COMPILER=uglifyjs We should also add a ?= to the CSS_COMPILER while we're there to make sure we can replace that one as well per #721568. Thanks for the patch! A. -- Wire telegraph is a kind of a very, very long cat. You pull his tail in New York and his head is meowing in Los Angeles. Radio operates exactly the same way: you send signals here, they receive them there. The only difference is that there is no cat. - Albert Einstein pgpenPzA4VH06.pgp Description: PGP signature
Bug#721567: [PATCH] Re: photofloat: should use separately packaged libjs-* packages (not include convenience code copies)
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 Le 2014-01-27 11:09, Antoine Beaupré a écrit : diff --git a/debian/photofloat.triggers b/debian/photofloat.triggers new file mode 100644 index 000..db0ecc7 --- /dev/null +++ b/debian/photofloat.triggers @@ -0,0 +1,2 @@ +interest /usr/share/javascript/jquery/jquery.js +interest /usr/share/javascript/jquery-mousewheel/jquery.mousewheel.js That seems absurdly easy. Did you actually try this out and it works? :) Yes. diff --git a/debian/rules b/debian/rules index 118de9d..49f0824 100755 --- a/debian/rules +++ b/debian/rules @@ -20,11 +20,5 @@ export DH_OPTIONS %: dh $@ --with=python2 -override_dh_auto_build: - dh_auto_build - ln -s /usr/share/javascript/jquery/jquery.js $(CURDIR)/web/js/000-jquery-1.7.2.js - ln -s /usr/share/javascript/jquery-mousewheel/jquery.mousewheel.js $(CURDIR)/web/js/003-mousewheel.js - cd $(CURDIR)/web $(MAKE) - override_dh_install: - dh_install -X.gitignore -XMakefile -Xutils/ + dh_install -X.gitignore -X*.min.js -Xutils/ Does that mean we still build the minified versions during build, but then don't ship them? Shouldn't just skip that? I removed the whole override_dh_auto_build code block, so the build-time minify doesn't happen anymore. diff --git a/web/Makefile b/web/Makefile index 72aea0d..8224ac5 100644 --- a/web/Makefile +++ b/web/Makefile @@ -7,7 +7,7 @@ CSS_MIN = $(CSS_DIR)/styles.min.css JS_MIN_FILES := $(sort $(patsubst %.js, %.min.js, $(filter-out %.min.js, $(wildcard $(JS_DIR)/*.js CSS_MIN_FILES := $(sort $(patsubst %.css, %.min.css, $(filter-out %.min.css, $(wildcard $(CSS_DIR)/*.css -JS_COMPILER = yui-compressor --type js +JS_COMPILER = uglifyjs CSS_COMPILER = yui-compressor --type css .PHONY: all clean Another patch with upstream, but nothing in debian/patches. I think we should instead use: JS_COMPILER ?= yui-compressor --type js ... to respect upstream's default yet allow ourselves an override. Then we just call make with: cd /usr/share/photofloat/web make JS_COMPILER=uglifyjs We should also add a ?= to the CSS_COMPILER while we're there to make sure we can replace that one as well per #721568. Sounds pretty reasonable to me! However upstream uses google-compiler, so the change would look like : - -JS_COMPILER = utils/google-compiler --warning_level QUIET +JS_COMPILER =? utils/google-compiler --warning_level QUIET - -- Jerome -BEGIN PGP SIGNATURE- Version: GnuPG v1 Comment: Using GnuPG with Icedove - http://www.enigmail.net/ iQEcBAEBCAAGBQJS5od4AAoJEK/ZGpI6kvtMRMIH/jYeB9Tgdsp4ZRTHLfJ1SvsK xLWDph6KxWYJd0+GawNQYMMJXh/MTluzKKvNSL4OJymo5k53MW7aH2EETukBjZMU Eq5+7GSjsq88oGikJ1oTWxKYlJ5ji1hFnAwSl4Hz4uSHwcFTd09rSuXKTyPr7Jhp 96M+FczT1vBN4Fc4a5Ywu7qn4RCxkwxcSGvTgnH9lxgJt1mP1eMgsGwTwQpD3zBZ G/3AJ6AkfeQmsvBnnpcNe1qzRcZuq8C3WYbondCejODPn1zd+uz9dcXJR4lSO9Qw iGZvCiWV12Z1VPn8hMt3cd8KWIo5IsGhuZkrfa4fY7Oe+k2LC9IwsBTdGR/o5jY= =wgmF -END PGP SIGNATURE- -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#721567: [PATCH] Re: photofloat: should use separately packaged libjs-* packages (not include convenience code copies)
On 2014-01-27 11:21:19, Jerome Charaoui wrote: diff --git a/debian/rules b/debian/rules index 118de9d..49f0824 100755 --- a/debian/rules +++ b/debian/rules @@ -20,11 +20,5 @@ export DH_OPTIONS %: dh $@ --with=python2 -override_dh_auto_build: - dh_auto_build - ln -s /usr/share/javascript/jquery/jquery.js $(CURDIR)/web/js/000-jquery-1.7.2.js - ln -s /usr/share/javascript/jquery-mousewheel/jquery.mousewheel.js $(CURDIR)/web/js/003-mousewheel.js -cd $(CURDIR)/web $(MAKE) - override_dh_install: -dh_install -X.gitignore -XMakefile -Xutils/ + dh_install -X.gitignore -X*.min.js -Xutils/ Does that mean we still build the minified versions during build, but then don't ship them? Shouldn't just skip that? I removed the whole override_dh_auto_build code block, so the build-time minify doesn't happen anymore. Hum.. I would have thought that without the override, dh_auto_build would still get called and hit the makefile and so on... Sounds pretty reasonable to me! However upstream uses google-compiler, so the change would look like : - -JS_COMPILER = utils/google-compiler --warning_level QUIET +JS_COMPILER =? utils/google-compiler --warning_level QUIET Understood. Can you take care of posting the patch upstream as well? Thanks! A. -- Seul a un caractère scientifique ce qui peut être réfuté. Ce qui n'est pas réfutable relève de la magie ou de la mystique. - Popper, Karl pgpPEpzMVvaB3.pgp Description: PGP signature
Bug#721567: [PATCH] Re: photofloat: should use separately packaged libjs-* packages (not include convenience code copies)
That should do the trick. Antoine, I should note that instead of creating a
distinct update-photofloat-js as we discussed, I reverted to shipping the
upstream Makefile and simply call that to minify+bundle js.
---
minify+bundle JavaScript libraries at installation and update using dpkg trigger
use uglifyjs instead of yui-compressor
---
debian/control | 12 +++-
debian/photofloat.links| 2 ++
debian/photofloat.postinst | 25 +
debian/photofloat.prerm| 19 +++
debian/photofloat.triggers | 2 ++
debian/rules | 8 +---
web/Makefile | 2 +-
7 files changed, 57 insertions(+), 13 deletions(-)
create mode 100644 debian/photofloat.postinst
create mode 100644 debian/photofloat.prerm
create mode 100644 debian/photofloat.triggers
diff --git a/debian/control b/debian/control
index 818effc..098cc4b 100644
--- a/debian/control
+++ b/debian/control
@@ -3,10 +3,7 @@ Section: web
Priority: optional
Maintainer: Antoine Beaupré anar...@debian.org
Build-Depends: debhelper (= 8.0.0),
- python,
- libjs-jquery,
- libjs-jquery-mousewheel,
- yui-compressor
+ python
Standards-Version: 3.9.3
Homepage: http://zx2c4.com/projects/photofloat/
Vcs-Git: git://git.debian.org/collab-maint/photofloat.git -b debian
@@ -18,7 +15,12 @@ Depends: ${shlibs:Depends},
${misc:Depends},
${python:Depends},
python-imaging,
- lmodern
+ lmodern,
+ libjs-jquery,
+ libjs-jquery-mousewheel,
+ node-uglify,
+ make
+Recommends: yui-compressor
Suggests: apache2 | nginx | httpd,
libapache2-mod-php5 | php5-fpm | php5-cgi
Description: Web 2.0 Photo Gallery Done Right via Static JSON Dynamic
Javascript
diff --git a/debian/photofloat.links b/debian/photofloat.links
index b5a5a1b..aa72d04 100644
--- a/debian/photofloat.links
+++ b/debian/photofloat.links
@@ -3,3 +3,5 @@
/usr/share/texmf/fonts/opentype/public/lm/lmroman10-bolditalic.otf
/usr/share/photofloat/web/fonts/lmroman10-bolditalic.otf
/usr/share/texmf/fonts/opentype/public/lm/lmroman10-italic.otf
/usr/share/photofloat/web/fonts/lmroman10-italic.otf
/usr/share/texmf/fonts/opentype/public/lm/lmroman10-regular.otf
/usr/share/photofloat/web/fonts/lmroman10-regular.otf
+/usr/share/javascript/jquery/jquery.js
/usr/share/photofloat/web/js/000-jquery-1.7.2.js
+/usr/share/javascript/jquery-mousewheel/jquery.mousewheel.js
/usr/share/photofloat/web/js/003-mousewheel.js
diff --git a/debian/photofloat.postinst b/debian/photofloat.postinst
new file mode 100644
index 000..3223656
--- /dev/null
+++ b/debian/photofloat.postinst
@@ -0,0 +1,25 @@
+#!/bin/sh
+
+set -e
+
+case $1 in
+ configure)
+cd /usr/share/photofloat/web make
+ ;;
+ triggered)
+cd /usr/share/photofloat/web make js/scripts.min.js
+ ;;
+ abort-upgrade|abort-remove|abort-deconfigure)
+ ;;
+ *)
+echo postinst called with unknown argument \`$1' 2
+exit 1
+ ;;
+esac
+
+# dh_installdeb will replace this with shell code automatically
+# generated by other debhelper scripts.
+
+#DEBHELPER#
+
+exit 0
diff --git a/debian/photofloat.prerm b/debian/photofloat.prerm
new file mode 100644
index 000..e1eda65
--- /dev/null
+++ b/debian/photofloat.prerm
@@ -0,0 +1,19 @@
+#!/bin/sh
+
+set -e
+
+case $1 in
+remove)
+rm -f /usr/share/photofloat/web/js/*.min.js
+;;
+failed-upgrade|upgrade|deconfigure)
+;;
+*)
+echo prerm called with unknown argument \`$1' 2
+exit 1
+;;
+esac
+
+#DEBHELPER#
+
+exit 0
diff --git a/debian/photofloat.triggers b/debian/photofloat.triggers
new file mode 100644
index 000..db0ecc7
--- /dev/null
+++ b/debian/photofloat.triggers
@@ -0,0 +1,2 @@
+interest /usr/share/javascript/jquery/jquery.js
+interest /usr/share/javascript/jquery-mousewheel/jquery.mousewheel.js
diff --git a/debian/rules b/debian/rules
index 118de9d..49f0824 100755
--- a/debian/rules
+++ b/debian/rules
@@ -20,11 +20,5 @@ export DH_OPTIONS
%:
dh $@ --with=python2
-override_dh_auto_build:
- dh_auto_build
- ln -s /usr/share/javascript/jquery/jquery.js
$(CURDIR)/web/js/000-jquery-1.7.2.js
- ln -s /usr/share/javascript/jquery-mousewheel/jquery.mousewheel.js
$(CURDIR)/web/js/003-mousewheel.js
- cd $(CURDIR)/web $(MAKE)
-
override_dh_install:
- dh_install -X.gitignore -XMakefile -Xutils/
+ dh_install -X.gitignore -X*.min.js -Xutils/
diff --git a/web/Makefile b/web/Makefile
index 72aea0d..8224ac5 100644
--- a/web/Makefile
+++ b/web/Makefile
@@ -7,7 +7,7 @@ CSS_MIN = $(CSS_DIR)/styles.min.css
JS_MIN_FILES := $(sort $(patsubst %.js, %.min.js, $(filter-out %.min.js,
$(wildcard $(JS_DIR)/*.js
CSS_MIN_FILES := $(sort $(patsubst %.css, %.min.css, $(filter-out %.min.css,
$(wildcard $(CSS_DIR)/*.css
-JS_COMPILER = yui-compressor --type js
+JS_COMPILER = uglifyjs
