Bug#515392: survex: diff for NMU version 1.0.39.1-2.2

2010-05-14 Thread Olly Betts 
On Thu, May 13, 2010 at 03:47:59AM +0100, Olly Betts wrote:
 On Wed, May 12, 2010 at 07:26:04PM -0700, tony mancill wrote:
  Jari made the change because I was getting failures trying to build the
  package inside a chroot environment, but it didn't affect the build.  I
  ended up unsetting my DISPLAY before the build.  I agree that it would be
  better not to disable the entire test suite; I'll test with your patch.
 
 Hmm, it's not good if the package fails to build if DISPLAY is set.
 
 It's hard to know what is happening without seeing the error you got, but my
 best guess is that DISPLAY was set to a value which doesn't work in the build
 environment.  If that was the problem, then the change in the NMU isn't going
 to help at all, and neither will my patch.

OK, I've just tested, and my guess above was essentially correct.

But the change in the NMU does actually help, but only because it isn't doing
what was intended:

-   $(MAKE) check
+   [ ! $DISPLAY ] || $(MAKE) check

Since this is a makefile, $DISPLAY is $D followed by the literal text ISPLAY so
the shell sees:

[ ! ISPLAY ] || $(MAKE) check

Which will simply never run the tests.

The best way to address this is probably just to ensure DISPLAY isn't set, so
we don't try to use an inherited value of DISPLAY which doesn't work in the
chroot-ed environment:

-   $(MAKE) check
+   $(MAKE) check DISPLAY=

I'm tempted to do a general spring clean of the package - there's another open
bug and a few lintian warnings.

Wookey: OK to add myself as co-maintainer and do that?

Cheers,
Olly



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org

Bug#515392: survex: diff for NMU version 1.0.39.1-2.2

2010-05-12 Thread Olly Betts 
On Fri, May 07, 2010 at 09:05:55PM -0700, tony mancill wrote:
 +  * debian/rules
 +- (build-stamp): Check that DISPLAY is set before running tests.
 [...]
 - $(MAKE) check
 + [ ! $DISPLAY ] || $(MAKE) check

I don't understand the need for this change.  What error were you getting that
motivated this change?

The only test script which seems to run anything which might use X is
tests/smoketest.tst, and in there testing of aven is already protected by:

if test -n $DISPLAY  test -r $testdir/../src/aven ; then

The only other tested binary which uses X is xcaverot, and it looks to me like
xcaverot processes --help and --version before calling XOpenDisplay, so I'm not
seeing why that would need an X display here.  If it really does, the attached
patch would be a much better fix than disabling the whole testsuite.

Cheers,
Olly
--- survex-1.0.39/tests/smoke.tst.orig	2006-01-09 15:34:50.0 +
+++ survex-1.0.39/tests/smoke.tst	2010-05-13 01:18:42.692987453 +0100
@@ -30,7 +30,7 @@
 
 PROGS=cad3d cavern printdm printps printpcl printhpgl diffpos extend sorterr\
  3dtopos
-if test -r $testdir/../src/xcaverot ; then
+if test -n $DISPLAY  test -r $testdir/../src/xcaverot ; then
PROGS=$PROGS xcaverot
 fi
 # aven tries to open an X display even for --help and --version

Bug#515392: survex: diff for NMU version 1.0.39.1-2.2

2010-05-12 Thread tony mancill 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 05/12/2010 05:24 PM, Olly Betts wrote:
 On Fri, May 07, 2010 at 09:05:55PM -0700, tony mancill wrote:
 +  * debian/rules
 +- (build-stamp): Check that DISPLAY is set before running tests.
 [...]
 -$(MAKE) check
 +[ ! $DISPLAY ] || $(MAKE) check
 
 I don't understand the need for this change.  What error were you getting that
 motivated this change?
 
 The only test script which seems to run anything which might use X is
 tests/smoketest.tst, and in there testing of aven is already protected by:
 
 if test -n $DISPLAY  test -r $testdir/../src/aven ; then
 
 The only other tested binary which uses X is xcaverot, and it looks to me like
 xcaverot processes --help and --version before calling XOpenDisplay, so I'm 
 not
 seeing why that would need an X display here.  If it really does, the attached
 patch would be a much better fix than disabling the whole testsuite.
 
 Cheers,
 Olly

Hi Olly,

Jari made the change because I was getting failures trying to build the package
inside a chroot environment, but it didn't affect the build.  I ended up
unsetting my DISPLAY before the build.  I agree that it would be better not to
disable the entire test suite; I'll test with your patch.

Thank you,
Tony
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkvrYzcACgkQpdwBkPlyvgNDLACfdbMrkZpm3w5/kh2biGrgeXB3
xaUAn2uDqe3tiLnDXF79xQiEaW6YQpVD
=PLOE
-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#515392: survex: diff for NMU version 1.0.39.1-2.2

2010-05-12 Thread Olly Betts 
On Wed, May 12, 2010 at 07:26:04PM -0700, tony mancill wrote:
 Jari made the change because I was getting failures trying to build the 
 package
 inside a chroot environment, but it didn't affect the build.  I ended up
 unsetting my DISPLAY before the build.  I agree that it would be better not to
 disable the entire test suite; I'll test with your patch.

Hmm, it's not good if the package fails to build if DISPLAY is set.

It's hard to know what is happening without seeing the error you got, but my
best guess is that DISPLAY was set to a value which doesn't work in the build
environment.  If that was the problem, then the change in the NMU isn't going
to help at all, and neither will my patch.

If you no longer have the build log, I'll see if I can reproduce the issue.

Cheers,
Olly



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org

Bug#515392: survex: diff for NMU version 1.0.39.1-2.2

2010-05-07 Thread tony mancill 
tags 515392 + patch
thanks

Dear maintainer,

I've prepared an NMU for survex (versioned as 1.0.39.1-2.2) and
uploaded it to DELAYED/5. Please feel free to tell me if I
should delay it longer.

Regards.
diff -u survex-1.0.39.1/debian/control survex-1.0.39.1/debian/control
--- survex-1.0.39.1/debian/control
+++ survex-1.0.39.1/debian/control
@@ -3,7 +3,7 @@
 Priority: extra
 Maintainer: Wookey woo...@survex.com
 Standards-Version: 3.7.3
-Build-Depends: debhelper (= 4.2.13), libx11-dev, libxext-dev, x-dev, 
libwxgtk2.6-dev, dpkg-dev (= 1.13.19)
+Build-Depends: debhelper (= 4.2.13), libx11-dev, libxext-dev, 
x11proto-core-dev, libwxgtk2.6-dev, dpkg-dev (= 1.13.19)
 Homepage: http://www.survex.com/
 
 Package: survex
diff -u survex-1.0.39.1/debian/changelog survex-1.0.39.1/debian/changelog
--- survex-1.0.39.1/debian/changelog
+++ survex-1.0.39.1/debian/changelog
@@ -1,3 +1,15 @@
+survex (1.0.39.1-2.2) unstable; urgency=low
+
+  [Jari Aalto]
+  * Non-maintainer upload.
+  * debian/control
+- (Build-Depends): update obsolete x-dev to x11proto-core-dev.
+  (important RC bug; Closes: #515392).
+  * debian/rules
+- (build-stamp): Check that DISPLAY is set before running tests.
+
+ -- Jari Aalto jari.aa...@cante.net  Wed, 31 Mar 2010 07:16:32 +0300
+
 survex (1.0.39.1-2.1) unstable; urgency=medium
 
   * Non-maintainer upload.
diff -u survex-1.0.39.1/debian/rules survex-1.0.39.1/debian/rules
--- survex-1.0.39.1/debian/rules
+++ survex-1.0.39.1/debian/rules
@@ -32,7 +32,7 @@
./configure --prefix=/usr --mandir=\$${prefix}/share/man 
--enable-docdir=\$${prefix}/share/doc/survex CFLAGS=$(CFLAGS) 
CXXFLAGS=$(CXXFLAGS) STRIP=$(STRIP)
$(MAKE)
 ifeq   ($(DEB_HOST_GNU_TYPE),$(DEB_BUILD_GNU_TYPE))
-   $(MAKE) check
+   [ ! $DISPLAY ] || $(MAKE) check
 endif
touch build-stamp
 



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org