See the 1.9-0ubuntu1 release, where the following are addressed: > Packaging: > - you bdep on gettext and po4a, but these are only used in prebuild > which isn't called on the buildds
Fixed. > - get-orig-source has: > [ -d ../../${PACKAGE} ] && mv ../../${PACKAGE} ../../${PACKAGE}-${VER} > || true > you can use a leading "-" to disable error checking: > -[ -d ../../${PACKAGE} ] && mv ../../${PACKAGE} ../../${PACKAGE}-${VER} > or even better, honor errors: > [ ! -d ../../${PACKAGE} ] || mv ../../${PACKAGE} > ../../${PACKAGE}-${VER} > (or use if then fi) Fixed. I'm going with the leading "-". > - There's a bunch of ../../ in the get-orig-source rule; I understand > it needs to be called from the debian/ dir; perhaps you should enforce > this just like dh_testdir does (or require that the rule be run as > "debian/rules get-orig-source" and use dh_testdir itself). Fixed. Will now be caued using './debian/rules get-orig-source' > - clean: > rm -rf debian/${PACKAGE} debian/files debian/${PACKAGE}.debhelper.log > I think you want dh_clean?!? Fixed. Thanks. > - would be nice to pass -i to dh_* calls in binary-indep Hmm, why would that be nice? Doesn't look like it's necessary. > - get-orig-source is .PHONY Fixed. > - you don't need dh_installdirs / debian/dirs Fixed. > - you miss a dh_md5sums call Fixed > - you should depend on ${misc:Depends} with debhelper >= 5 Fixed. > This is what lintian has to say: > I: screen-profiles source: debian-watch-file-is-missing > W: screen-profiles source: debhelper-but-no-misc-depends screen-profiles > I: screen-profiles source: build-depends-without-arch-dep debhelper > I: screen-profiles source: build-depends-without-arch-dep gettext > I: screen-profiles source: build-depends-without-arch-dep po4a > W: screen-profiles: binary-without-manpage usr/bin/screen-launcher Created manpage. > W: screen-profiles: binary-without-manpage usr/bin/screen-profiles- helper Created manpage. > I: screen-profiles: no-md5sums-control-file > These are covered in the above remarks more or less. > Please mention the copyrights of Canonical and Nick Barcet. Fixed. > Typo in man page: > select-screen-profile is an application lists the available screen pro‐ > files on a system and prompts the user to select one. > ("which lists" perhaps?) Fixed. > Would be nice to use "set -e" in non-trivial shell scripts. Fixed. > mycache=/var/tmp/updates-available-$USER is a very bad idea: /var/tmp > is +t and anybody can write there; I can create a > /var/tmp/updates-available-root symlink to /etc/shadow as my user and > wait for updates-available to be run as root. > You need to find another place, or another mechanism. For instance you > could source all files /var/tmp/updates-available-$USER-XXXXXXXX which > are owned by $USER and take the most recent one, delete the others. Fixed. I tried to handle this by ensuring that the user owned the file, with the -O test. I thought that would make it safe? Well, in any case, I've moved this to $HOME/.screenrc-updates-available. This is only really ever used in <jaunty code. It's here so that I don't have to compile separate binaries for hardy/intrepid/jaunty. > So I'd insist on fixing the missing MD5 and the tmpdir race in /var/tmp at > least, but the more you fix the merrier. :-) Thanks for the careful review. I think I've fixed just about everything. :-Dustin -- main inclusion: screen-profiles https://bugs.launchpad.net/bugs/317214 You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs