[PATCH] Give a path name to mktemp in Makefile.local

[PATCH] Give a path name to mktemp in Makefile.local

[Aaron Ecay]

On some systems (incl. OS X 10.6), mktemp expects an argument giving it
the place to put the new temporary file.
---

On my machine without this patch, make prints a message from mktemp
about expecting an argument each time it is run.  At some point, make
got into a situation where it would print this message and exit cleanly,
but not build any changed files.  A "make clean" was necessary to kick
it into working again.

A disadvantage of this approach is that it drops an empty file into /tmp
on every make run.  It would be better to only create this file when
doing "make debian-snapshot", but I am not sure how to do that (cleanly;
my best idea is to put the build commands into a subshell and export an
environment variable for the temp file).  Any make/debian experts want
to take a stab?

 Makefile.local |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Makefile.local b/Makefile.local
index c94402b..6eb4b18 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -139,7 +139,7 @@ pre-release:
 	mv $(TAR_FILE) $(DEB_TAR_FILE) releases
 
 .PHONY: debian-snapshot
-debian-snapshot: TMPFILE := $(shell mktemp)
+debian-snapshot: TMPFILE := $(shell mktemp /tmp/notmuch.XXXXXX)
 debian-snapshot:
 	make VERSION=$(VERSION) clean
 	cp debian/changelog $(TMPFILE)
-- 
1.7.8

Re: [PATCH] Give a path name to mktemp in Makefile.local

[Tomi Ollila]

On Sat, 17 Dec 2011 10:36:25 -0500, Aaron Ecay <aaronecay@gmail.com> wrote:
> On some systems (incl. OS X 10.6), mktemp expects an argument giving it
> the place to put the new temporary file.
> ---
> 
> On my machine without this patch, make prints a message from mktemp
> about expecting an argument each time it is run.  At some point, make
> got into a situation where it would print this message and exit cleanly,
> but not build any changed files.  A "make clean" was necessary to kick
> it into working again.

All *BSD mktemp's require template argument...

> A disadvantage of this approach is that it drops an empty file into /tmp
> on every make run.  It would be better to only create this file when
> doing "make debian-snapshot", but I am not sure how to do that (cleanly;
> my best idea is to put the build commands into a subshell and export an
> environment variable for the temp file).  Any make/debian experts want
> to take a stab?

Without your patch it does those. Maybe debian-snapshot should
do like:

	cp -f debian/changelog debian/changelog.backup
        ...
        debuild -us -uc
	mv -f debian/changelog.backup debian/changelog

But that is another issue. +1 for your patch.

Tomi

Re: [PATCH] Give a path name to mktemp in Makefile.local

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 389 bytes --]

On Sat, 17 Dec 2011 10:36:25 -0500, Aaron Ecay <aaronecay@gmail.com> wrote:
> On some systems (incl. OS X 10.6), mktemp expects an argument giving it
> the place to put the new temporary file.

Not that I'm saying we shouldn't fix this issue, but just out of
curiosity, under what circumstances would someone want to call the
debian-snapshot target from a non-debian based system?

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH] Give a path name to mktemp in Makefile.local

[Aaron Ecay]

On Sat, 17 Dec 2011 13:20:41 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> Not that I'm saying we shouldn't fix this issue, but just out of
> curiosity, under what circumstances would someone want to call the
> debian-snapshot target from a non-debian based system?

That part of the makefile is called unconditionally, on all systems.  So
it doesn't matter if one does "make debian-snapshot" or not, the error
is still generated.

-- 
Aaron Ecay

[PATCH] build-system: use a shell variable for TMPFILE in debian-snapshot

[David Bremner]

From: David Bremner <bremner@debian.org>

Aaron Ecay points out in
id:"1324136185-4509-1-git-send-email-aaronecay@gmail.com" that the
mktemp in

     debian-snapshot: TMPFILE := $(shell mktemp)

Is being evaluated for every target. As best I can tell, this is
because make is evaluating the right hand side, even though it is not
doing the assignment.

Of course, it isn't quite as nice to edit with the line continuations,
but it is ideomatic make.
---
 Makefile.local |   15 ++++++++-------
 1 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/Makefile.local b/Makefile.local
index 5108a0c..97f397f 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -139,15 +139,16 @@ pre-release:
 	mv $(TAR_FILE) $(DEB_TAR_FILE) releases
 
 .PHONY: debian-snapshot
-debian-snapshot: TMPFILE := $(shell mktemp)
 debian-snapshot:
 	make VERSION=$(VERSION) clean
-	cp debian/changelog $(TMPFILE)
-	EDITOR=/bin/true dch -b -v $(VERSION)+1 -D UNRELEASED 'test build, not for upload'
-	echo '3.0 (native)' > debian/source/format
-	debuild -us -uc
-	mv -f $(TMPFILE) debian/changelog
-	echo '3.0 (quilt)' > debian/source/format
+	TMPFILE=$$(mktemp /tmp/notmuch.XXXXXX);		\
+	  cp debian/changelog $${TMPFILE};		\
+	  EDITOR=/bin/true dch -b -v $(VERSION)+1	\
+	    -D UNRELEASED 'test build, not for upload';	\
+	  echo '3.0 (native)' > debian/source/format; 	\
+	  debuild -us -uc;				\
+	  mv -f $${TMPFILE} debian/changelog;		\
+	  echo '3.0 (quilt)' > debian/source/format
 
 .PHONY: release-message
 release-message:
-- 
1.7.7.3

Re: [PATCH] build-system: use a shell variable for TMPFILE in debian-snapshot

[Tomi Ollila]

On Sat, 17 Dec 2011 23:16:51 -0400, David Bremner <david@tethera.net> wrote:
> From: David Bremner <bremner@debian.org>
> 
> Aaron Ecay points out in
> id:"1324136185-4509-1-git-send-email-aaronecay@gmail.com" that the
> mktemp in
> 
>      debian-snapshot: TMPFILE := $(shell mktemp)
> 
> Is being evaluated for every target. As best I can tell, this is
> because make is evaluating the right hand side, even though it is not
> doing the assignment.
> 
> Of course, it isn't quite as nice to edit with the line continuations,
> but it is ideomatic make.
> ---

I was originally suggesting to add 'set -e' and trap 'cleanup' 0
to the code but that starts looking ever messier. In case of
debian-snapshot: one needs to check whether output is procuded
as it should be; building this goal will always exit with
zero value (provided that last echo ... succeeds).

Ok, provided that the above is OK with this particular target.

(please push fast, I'm tired with all the /tmp/tmp.XXXXXX files
I've got to clean up so far >;)

Tomi

>  Makefile.local |   15 ++++++++-------
>  1 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/Makefile.local b/Makefile.local
> index 5108a0c..97f397f 100644
> --- a/Makefile.local
> +++ b/Makefile.local
> @@ -139,15 +139,16 @@ pre-release:
>  	mv $(TAR_FILE) $(DEB_TAR_FILE) releases
>  
>  .PHONY: debian-snapshot
> -debian-snapshot: TMPFILE := $(shell mktemp)
>  debian-snapshot:
>  	make VERSION=$(VERSION) clean
> -	cp debian/changelog $(TMPFILE)
> -	EDITOR=/bin/true dch -b -v $(VERSION)+1 -D UNRELEASED 'test build, not for upload'
> -	echo '3.0 (native)' > debian/source/format
> -	debuild -us -uc
> -	mv -f $(TMPFILE) debian/changelog
> -	echo '3.0 (quilt)' > debian/source/format
> +	TMPFILE=$$(mktemp /tmp/notmuch.XXXXXX);		\
> +	  cp debian/changelog $${TMPFILE};		\
> +	  EDITOR=/bin/true dch -b -v $(VERSION)+1	\
> +	    -D UNRELEASED 'test build, not for upload';	\
> +	  echo '3.0 (native)' > debian/source/format; 	\
> +	  debuild -us -uc;				\
> +	  mv -f $${TMPFILE} debian/changelog;		\
> +	  echo '3.0 (quilt)' > debian/source/format
>  
>  .PHONY: release-message
>  release-message:
> -- 
> 1.7.7.3

Re: [PATCH] build-system: use a shell variable for TMPFILE in debian-snapshot

[David Bremner]

On Sun, 18 Dec 2011 10:48:44 +0200, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> On Sat, 17 Dec 2011 23:16:51 -0400, David Bremner <david@tethera.net> wrote:
> 
> I was originally suggesting to add 'set -e' and trap 'cleanup' 0
> to the code but that starts looking ever messier. In case of
> debian-snapshot: one needs to check whether output is procuded
> as it should be; building this goal will always exit with
> zero value (provided that last echo ... succeeds).
> 

I think that's acceptable here. It is really a convenience target. 
Pushed.

d