Re: version.h rebuild change in stable

[Derek Martin]

On Sun, Jun 02, 2019 at 09:03:55AM +1000, Cameron Simpson wrote:
> >I'm not a Makefile person myself.  My "fix" added the version.sh
> >prerequisite and appended to the version.h explicitly just to
> >match the target below, reldate.h.
> >
> >If there is something to be gained by using the Makefile variables
> >I'm all ears, but personally I find the recipe less readable with
> >that way.
> 
> The only thing gained from using $? and $@ is that renaming the
> target or prerequisite doesn't require matching adjustment of the
> recipe. But the recipe is very small and I agree it is less readable
> with $? and $@.

In this specific instance that may be the case, but in the general
case it may also save a lot of typing (including copying and pasting
other similar targets), and avoids changes to the
recipe if your target or dependencies change.

I think once you've written a few make files, this is the sort of
thing that would tend to become second nature.  The problem with
avoiding using these things because you're not familiar with them is
that you never become familiar with them--they never get the chance to
become second nature.

You also tend to attract criticism from experienced users who expect
them to be used...

-- 
Derek D. Martinhttp://www.pizzashack.org/   GPG Key ID: 0xDFBEAD02
-=-=-=-=-
This message is posted from an invalid address.  Replying to it will result in
undeliverable mail due to spam prevention.  Sorry for the inconvenience.



pgpsECJfc3EPt.pgp
Description: PGP signature

Re: version.h rebuild change in stable

[Vincent Lefevre]

On 2019-06-02 08:31:01 +1000, Cameron Simpson wrote:
> On 01Jun2019 14:16, vincent lefevre  wrote:
> > On 2019-06-01 16:36:38 +1000, Cameron Simpson wrote:
> > > I'm an opinionated make user. Can you explain why you want the
> > > "FORCE"
> > > target at all?
> > 
> > The goal is to make sure that version.h is up-to-date after an update
> > such as "git pull".
> 
> Ah, where the file timestamp is not a reliable indicator of change?

The idea was to update version.h only when there is to be a real
change in its contents (a temporary file was used for the detection).
This is not possible by using timestamp dependencies, AFAIK (well,
this is theoretically possible, but this would have required major
changes in Makefile.am).

Now, in Mutt, when version.h is modified, there aren't many things
to rebuild, thus having a dependency of version.h on every source
file could be a solution. I still much prefer the GNU make solution.

> Just spitballing here, but might it not be reasonable for "make clean" to
> rebuild version.h? It seems like you'd want a make clean after a git pull
> anyway.

No, certainly not! I don't want to rebuild *everything* after a
"git pull".

> > [...]
> > > Coming around to my opinions here, I think the BSD make is more correct:
> > > version.h got remade in the "FORCE" version of the Makefile, and its
> > > dependents need a rebuild. GNU make's presuming that file content are all
> > > there are - suppose an action changed something else significant not
> > > considered? Eg maybe the build process copies an ACL off version.h or some
> > > other thing beyond the content. Obviously we don't actually do that, but 
> > > IMO
> > > GNU make is being too clever.
> > 
> > No, a rule with no prerequisites or recipe is undefined behavior in
> > POSIX, thus can be used as an extension, and this is what GNU make
> > does (see Section 4.7 Rules without Recipes or Prerequisites).
> 
> What if the FORCE target were:
> 
>  FORCE:
>true
> 
> so that it had a do-nothing recipe? That feels portable.

If this works with BSD make, this would be fine, I think.

> The downside is of course that makes version.h unconditionally updated and
> that probably forces a rebuild of lots of stuff.

No, see the use of the .tmp file that was used with FORCE. This avoids
a change of the timestamp of version.h if the contents are not about
to be changed.

-- 
Vincent Lefèvre  - Web: 
100% accessible validated (X)HTML - Blog: 
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)

Re: version.h rebuild change in stable

[Cameron Simpson]

On 01Jun2019 07:18, Kevin J. McCarthy  wrote:

On Sat, Jun 01, 2019 at 04:36:38PM +1000, Cameron Simpson wrote:

If I were writing the conjectured goal above I'd probably do this:

mutt_version='#define MUTT_VERSION "'`sh "$(srcdir)/version.sh`'"'
version_h=`[ -s version.h ] && cat version.h`
[ "$$mutt_version" = "$$version_h" ] || echo "$$mutt_version" >version.h

in "configure", not in the Makefile. As a prebuilt step. Once you've 
got a version.h you're good.


:-) Funnily enough that's the way it was in the first place.


Hah.

But as Vincent mentioned, the change was to save developers from having 
to reconfigure all the time just to bump the version number.


That wasn't apparent to me before. I still think there's a case for 
putting it in "make clean". Though that again will cause a complete 
rebuild. Though it skips configure.



But I think what I'd really want is:

version.h: $(srcdir)/version.sh
 echo '#define MUTT_VERSION "'`sh '$?'`'"' >'$@'

Why _don't_ we just have this?


I'm not a Makefile person myself.  My "fix" added the version.sh 
prerequisite and appended to the version.h explicitly just to match 
the target below, reldate.h.


If there is something to be gained by using the Makefile variables I'm 
all ears, but personally I find the recipe less readable with that way.


The only thing gained from using $? and $@ is that renaming the target 
or prerequisite doesn't require matching adjustment of the recipe. But 
the recipe is very small and I agree it is less readable with $? and $@.


Cheers,
Cameron Simpson 

Re: version.h rebuild change in stable

[Cameron Simpson]

On 01Jun2019 14:16, vincent lefevre  wrote:

On 2019-06-01 16:36:38 +1000, Cameron Simpson wrote:
I'm an opinionated make user. Can you explain why you want the 
"FORCE"

target at all?


The goal is to make sure that version.h is up-to-date after an update
such as "git pull".


Ah, where the file timestamp is not a reliable indicator of change?

Just spitballing here, but might it not be reasonable for "make clean" 
to rebuild version.h? It seems like you'd want a make clean after a git 
pull anyway.



[...]

Coming around to my opinions here, I think the BSD make is more correct:
version.h got remade in the "FORCE" version of the Makefile, and its
dependents need a rebuild. GNU make's presuming that file content are all
there are - suppose an action changed something else significant not
considered? Eg maybe the build process copies an ACL off version.h or some
other thing beyond the content. Obviously we don't actually do that, but IMO
GNU make is being too clever.


No, a rule with no prerequisites or recipe is undefined behavior in
POSIX, thus can be used as an extension, and this is what GNU make
does (see Section 4.7 Rules without Recipes or Prerequisites).


What if the FORCE target were:

 FORCE:
   true

so that it had a do-nothing recipe? That feels portable.

The downside is of course that makes version.h unconditionally updated 
and that probably forces a rebuild of lots of stuff. Which is why I 
don't really think it should be part of the basic make graph. It should 
be off in "make clean" or some other preparatory step.


My "GNU being too clever" thing is about GNU checking file contents 
before and after the recipe, BTW.


Anyway, it sounds like you have a workaround; I agree with not feeling 
obliged to support having one make during configure and another during 
build.


Cheers,
Cameron Simpson 

Re: version.h rebuild change in stable

[Kevin J. McCarthy]

On Sat, Jun 01, 2019 at 04:36:38PM +1000, Cameron Simpson wrote:

If I were writing the conjectured goal above I'd probably do this:

mutt_version='#define MUTT_VERSION "'`sh "$(srcdir)/version.sh`'"'
version_h=`[ -s version.h ] && cat version.h`
[ "$$mutt_version" = "$$version_h" ] || echo "$$mutt_version" >version.h

in "configure", not in the Makefile. As a prebuilt step. Once you've 
got a version.h you're good.


:-) Funnily enough that's the way it was in the first place.  But as 
Vincent mentioned, the change was to save developers from having to 
reconfigure all the time just to bump the version number.



But I think what I'd really want is:

version.h: $(srcdir)/version.sh
  echo '#define MUTT_VERSION "'`sh '$?'`'"' >'$@'

Why _don't_ we just have this?


I'm not a Makefile person myself.  My "fix" added the version.sh 
prerequisite and appended to the version.h explicitly just to match the 
target below, reldate.h.


If there is something to be gained by using the Makefile variables I'm 
all ears, but personally I find the recipe less readable with that way.


--
Kevin J. McCarthy
GPG Fingerprint: 8975 A9B3 3AA3 7910 385C  5308 ADEF 7684 8031 6BDA


signature.asc
Description: PGP signature

Re: version.h rebuild change in stable

[Kevin J. McCarthy]

On Sat, Jun 01, 2019 at 02:16:24PM +0200, Vincent Lefevre wrote:

On 31May2019 17:36, Kevin J. McCarthy  wrote:

On Fri, May 31, 2019 at 11:19:27PM +0200, Vincent Lefevre wrote:
How about using AM_CONDITIONAL in configure.ac, setting a variable 
GNU_MAKE when GNU Make is used (based on "make --version" output)?


That might be a workable idea.  I worry about making assumptions about 
the 'make' program that will be used, though.  It's possible the user 
will invoke another make program outside of the search path, in 
certain environments.  Or am I just being too paranoid... :-)


There would be a problem in practice only if the user has GNU make 
in his search path, but chooses later to use a different make. But 
why would he do that? 


The OpenBSD developer indicated in the ticket he had access to GNU make 
too.  Sometimes people have surprisingly convoluted environments.


Anywway, changing the version of "make" after 
configure is not supported by the configure script, which already 
has:


Okay, that resolves my fear.  You can override which one by setting MAKE 
during configure, but otherwise it uses the one found in the search 
path.


Your proposal sounds fine to me.  I can look into this in a few days, 
but I have a few post-release issues to deal with first.  If you or 
anyone else wants to submit a patch, please feel free.


--
Kevin J. McCarthy
GPG Fingerprint: 8975 A9B3 3AA3 7910 385C  5308 ADEF 7684 8031 6BDA


signature.asc
Description: PGP signature

Re: version.h rebuild change in stable

[Vincent Lefevre]

On 2019-06-01 16:36:38 +1000, Cameron Simpson wrote:
> On 31May2019 17:36, Kevin J. McCarthy  wrote:
> > On Fri, May 31, 2019 at 11:19:27PM +0200, Vincent Lefevre wrote:
> > > On 2019-05-31 09:37:02 -0700, Kevin J. McCarthy wrote:
> > > > Of course, if anyone has an idea how to do this in a way that
> > > > works portably, we can revisit.  But for now I consider their
> > > > build issues more important than the minor convenience for
> > > > developers.
> > > 
> > > How about using AM_CONDITIONAL in configure.ac, setting a variable
> > > GNU_MAKE when GNU Make is used (based on "make --version" output)?
> > 
> > That might be a workable idea.  I worry about making assumptions about
> > the 'make' program that will be used, though.  It's possible the user
> > will invoke another make program outside of the search path, in certain
> > environments.  Or am I just being too paranoid... :-)

There would be a problem in practice only if the user has GNU make
in his search path, but chooses later to use a different make. But
why would he do that? Anywway, changing the version of "make" after
configure is not supported by the configure script, which already
has:

{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${MAKE-make} sets 
\$(MAKE)" >&5
$as_echo_n "checking whether ${MAKE-make} sets \$(MAKE)... " >&6; }
set x ${MAKE-make}
ac_make=`$as_echo "$2" | sed 's/+/p/g; s/[^a-zA-Z0-9_]/_/g'`
if eval \${ac_cv_prog_make_${ac_make}_set+:} false; then :
  $as_echo_n "(cached) " >&6
else
  cat >conftest.make <<\_ACEOF
SHELL = /bin/sh
all:
@echo '@@@%%%=$(MAKE)=@@@%%%'
_ACEOF
# GNU make sometimes prints "make[1]: Entering ...", which would confuse us.
case `${MAKE-make} -f conftest.make 2>/dev/null` in
  *@@@%%%=?*=@@@%%%*)
eval ac_cv_prog_make_${ac_make}_set=yes;;
  *)
eval ac_cv_prog_make_${ac_make}_set=no;;
esac
rm -f conftest.make
fi
if eval test \$ac_cv_prog_make_${ac_make}_set = yes; then
  { $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5
$as_echo "yes" >&6; }
  SET_MAKE=
else
  { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
$as_echo "no" >&6; }
  SET_MAKE="MAKE=${MAKE-make}"
fi

etc.

> I'm an opinionated make user. Can you explain why you want the "FORCE"
> target at all?

The goal is to make sure that version.h is up-to-date after an update
such as "git pull".

[...]
> Coming around to my opinions here, I think the BSD make is more correct:
> version.h got remade in the "FORCE" version of the Makefile, and its
> dependents need a rebuild. GNU make's presuming that file content are all
> there are - suppose an action changed something else significant not
> considered? Eg maybe the build process copies an ACL off version.h or some
> other thing beyond the content. Obviously we don't actually do that, but IMO
> GNU make is being too clever.

No, a rule with no prerequisites or recipe is undefined behavior in
POSIX, thus can be used as an extension, and this is what GNU make
does (see Section 4.7 Rules without Recipes or Prerequisites).

-- 
Vincent Lefèvre  - Web: 
100% accessible validated (X)HTML - Blog: 
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)

Re: version.h rebuild change in stable

[Cameron Simpson]

On 31May2019 17:36, Kevin J. McCarthy  wrote:

On Fri, May 31, 2019 at 11:19:27PM +0200, Vincent Lefevre wrote:

On 2019-05-31 09:37:02 -0700, Kevin J. McCarthy wrote:
Of course, if anyone has an idea how to do this in a way that 
works portably, we can revisit.  But for now I consider their 
build issues more important than the minor convenience for 
developers.


How about using AM_CONDITIONAL in configure.ac, setting a variable 
GNU_MAKE when GNU Make is used (based on "make --version" output)?


That might be a workable idea.  I worry about making assumptions about 
the 'make' program that will be used, though.  It's possible the user 
will invoke another make program outside of the search path, in certain 
environments.  Or am I just being too paranoid... :-)


I'm an opinionated make user. Can you explain why you want the "FORCE" 
target at all? Just looking at the diff in the ticket:


 -version.h: FORCE
 -echo '#define MUTT_VERSION "'`sh "$(srcdir)/version.sh"`'"' > $@.tmp
 -cmp -s $@ $@.tmp && rm -f $@.tmp || mv $@.tmp $@
 -FORCE:
 +version.h: $(srcdir)/version.sh
 +echo '#define MUTT_VERSION "'`sh "$(srcdir)/version.sh"`'"' > version.h

Is the purpose of the FORCE to (effectively) always examine the 
version.sh script output, but to only require a rebuild if version.sh 
changed contents?


If I were writing the conjectured goal above I'd probably do this:

 mutt_version='#define MUTT_VERSION "'`sh "$(srcdir)/version.sh`'"'
 version_h=`[ -s version.h ] && cat version.h`
 [ "$$mutt_version" = "$$version_h" ] || echo "$$mutt_version" >version.h

in "configure", not in the Makefile. As a prebuilt step. Once you've got 
a version.h you're good.


Then everything else can just depend in version.h as normal because the 
second make is working from a "clean" tree, prepared. This avoids any 
odd semantics like GNU make being particularly clever.


But I think what I'd really want is:

 version.h: $(srcdir)/version.sh
   echo '#define MUTT_VERSION "'`sh '$?'`'"' >'$@'

Why _don't_ we just have this?

Coming around to my opinions here, I think the BSD make is more correct: 
version.h got remade in the "FORCE" version of the Makefile, and its 
dependents need a rebuild. GNU make's presuming that file content are 
all there are - suppose an action changed something else significant not 
considered? Eg maybe the build process copies an ACL off version.h or 
some other thing beyond the content. Obviously we don't actually do 
that, but IMO GNU make is being too clever.


I think my end opinion would be that version.h should get made if 
version.sh gets modified (timestampwise). Just a straight dependency.


Rather than constructing progressively more complex "make flavour" 
dependent makefiles which try to encompass unknown make behaviour 
weirdness, expecially since as you point out we don't know the make at 
build time is the make which configure detects/infers/guesses.


Why _isn't_ is a straight dependncy of version.h on 
$(srcdir)/version.sh?


Cheers,
Cameron Simpson 

Re: version.h rebuild change in stable

[Kevin J. McCarthy]

On Fri, May 31, 2019 at 11:19:27PM +0200, Vincent Lefevre wrote:

On 2019-05-31 09:37:02 -0700, Kevin J. McCarthy wrote:
Of course, if anyone has an idea how to do this in a way that works 
portably, we can revisit.  But for now I consider their build issues 
more important than the minor convenience for developers.


How about using AM_CONDITIONAL in configure.ac, setting a variable 
GNU_MAKE when GNU Make is used (based on "make --version" output)?


That might be a workable idea.  I worry about making assumptions about 
the 'make' program that will be used, though.  It's possible the user 
will invoke another make program outside of the search path, in certain 
environments.  Or am I just being too paranoid... :-)


--
Kevin J. McCarthy
GPG Fingerprint: 8975 A9B3 3AA3 7910 385C  5308 ADEF 7684 8031 6BDA


signature.asc
Description: PGP signature

Re: version.h rebuild change in stable

[Vincent Lefevre]

On 2019-05-31 09:37:02 -0700, Kevin J. McCarthy wrote:
> Of course, if anyone has an idea how to do this in a way that works
> portably, we can revisit.  But for now I consider their build issues more
> important than the minor convenience for developers.

How about using AM_CONDITIONAL in configure.ac, setting a variable
GNU_MAKE when GNU Make is used (based on "make --version" output)?

Then, in Makefile.am, have

if GNU_MAKE

version.h: FORCE
echo '#define MUTT_VERSION "'`sh "$(srcdir)/version.sh"`'"' > $@.tmp
cmp -s $@ $@.tmp && rm -f $@.tmp || mv $@.tmp $@
FORCE:

else

version.h: $(srcdir)/version.sh
echo '#define MUTT_VERSION "'`sh "$(srcdir)/version.sh"`'"' > version.h

endif

-- 
Vincent Lefèvre  - Web: 
100% accessible validated (X)HTML - Blog: 
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)