Re: [PATCH 1/2] Clean up: Makefile.in: Correct the prerequisites

2017-07-15 Thread Michael Witten
On Tue, 11 Jul 2017 05:08:56 -, Michael Witten wrote:

> Thus, for now, the minimal knowledge of prerequisites to which `make'
> has access during the initial run is basically just the one header
> which potentially needs to be made before almost everything else:
>
>   default_options.h

Naturally, there is also going to be the need to handle `localoptions.h'.

It should be noted that the header file `localoptions.h' previously did
not need to exist; the makefiles would figure out whether it existed and
then set things up accordingly. While it's possible to keep this kind
of behavior, doing so would make it impossible to trigger the targets
of the `libtomcrypt' project when `localoptions.h' is *deleted*, unless
the same "runtime" discovery of prerequisites were also performed for
that builtin dependency. It's not worth the trouble; it's much easier
just to create an empty `localoptions.h' in the build directory so that
its presence can be expected and monitored.

The following patch is an addendum, and will be incorporated into a
revised version of this patch series; of course, criticism is welcome
and desired.


8<8<8<8<8<8<8<8<8<8<8<8<8<


diff --git a/.gitignore b/.gitignore
index 1ef4318..d6f63be 100644
--- a/.gitignore
+++ b/.gitignore
@@ -18,5 +18,6 @@
 Makefile
 config.h
 config.h.in
+/localoptions.h
 configure
 /prerequisites/
diff --git a/Makefile.in b/Makefile.in
index 885a4f0..16f8d32 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -23,10 +23,6 @@ LIBTOM_DEPS=$(LIBTOM_LIBS)
 CFLAGS+=-I$(srcdir)/libtomcrypt/src/headers
 endif
 
-ifneq ($(wildcard localoptions.h),)
-CFLAGS+=-DLOCALOPTIONS_H_EXISTS
-endif
-
 COMMONOBJS=dbutil.o buffer.o dbhelpers.o \
dss.o bignum.o \
signkey.o rsa.o dbrandom.o \
@@ -175,8 +171,12 @@ $(prerequisites_dir):
 
 include $(wildcard $(prerequisites_dir)/*.d)
 
+# This defines $(options_headers) and $(default_options_h)
+options_headers_dir:=.
+include $(srcdir)/options.mk
+
 $(allobjs): CFLAGS+=-MMD -MF $(prerequisites_dir)/$@.d
-$(allobjs): default_options.h | $(prerequisites_dir)
+$(allobjs): $(options_headers) | $(prerequisites_dir)
 
 dropbear$(EXEEXT): $(dropbearobjs) $(LIBTOM_DEPS)
$(CC) $(LDFLAGS) -o $@ $(dropbearobjs) $(LIBTOM_LIBS) $(LIBS) @CRYPTLIB@
@@ -209,7 +209,7 @@ $(STATIC_LTM): libtommath
$(NOTHING)
 
 .PHONY: libtomcrypt
-libtomcrypt: | default_options.h
+libtomcrypt: | $(options_headers)
$(MAKE) -C libtomcrypt
 
 .PHONY: libtommath
@@ -249,7 +249,7 @@ tidy:
 
 # default_options.h is stored in version control, could not find a workaround
 # for parallel "make -j" and dependency rules.
-default_options.h: default_options.h.in 
+$(default_options_h): $(default_options_h).in 
echo "# > > > Generated from $^, edit that file instead !" > $@.tmp
echo >> $@.tmp
$(srcdir)/ifndef_wrapper.sh < $^ > $@.tmp
diff --git a/libtomcrypt/Makefile.in b/libtomcrypt/Makefile.in
index d8f9a6d..edca4c3 100644
--- a/libtomcrypt/Makefile.in
+++ b/libtomcrypt/Makefile.in
@@ -263,13 +263,17 @@ src/hashes/sha2/sha256.o: src/hashes/sha2/sha256.c 
src/hashes/sha2/sha224.c
 #This rule makes the libtomcrypt library.
 library: $(LIBNAME)
 
+# This defines $(options_headers) and $(default_options_h) and 
($default_options_h_name)
+options_headers_dir:=..
+include $(srcdir)/../options.mk
+
 src/misc/zeromem.o: ../dbhelpers.h
 src/math/ltm_desc.o: ../libtommath/tommath.h ../libtommath/tommath_class.h 
../libtommath/tommath_superclass.h
-$(OBJECTS): ../config.h ../options.h ../default_options.h ../sysoptions.h
+$(OBJECTS): ../config.h ../options.h $(options_headers) ../sysoptions.h
 $(OBJECTS): $(HEADERS)
 
-../default_options.h: ../default_options.h.in
-   $(MAKE) -C .. default_options.h
+$(default_options_h): $(default_options_h).in 
+   $(MAKE) -C .. $(default_options_h_name)
 
 testprof/$(LIBTEST): 
cd testprof ; CFLAGS="$(CFLAGS)" LIBTEST_S=$(LIBTEST_S) $(MAKE) 
diff --git a/options.h b/options.h
index c1782d2..7465d91 100644
--- a/options.h
+++ b/options.h
@@ -8,10 +8,7 @@ Local compile-time configuration should be defined in 
localoptions.h
 See default_options.h.in for a description of the available options.
 */
 
-#ifdef LOCALOPTIONS_H_EXISTS
 #include "localoptions.h"
-#endif
-
 #include "default_options.h"
 
 /* Some other defines that mostly should be left alone are defined
diff --git a/options.mk b/options.mk
new file mode 100644
index 000..5ae7d79
--- /dev/null
+++ b/options.mk
@@ -0,0 +1,9 @@
+default_options_h_name:=default_options.h
+
+localoptions_h:=$(options_headers_dir)/localoptions.h
+default_options_h:=$(options_headers_dir)/$(default_options_h_name)
+
+options_headers:=$(localoptions_h) $(default_options_h)
+
+$(localoptions_h):
+   touch $(localoptions_h)


Re: [PATCH 1/2] Clean up: Makefile.in: Correct the prerequisites

2017-07-12 Thread Michael Witten
On Wed, 12 Jul 2017 17:34:49 -, Michael Witten wrote:

> On Tue, 11 Jul 2017 05:08:56 -, Michael Witten wrote:
>
>> diff --git a/libtommath/Makefile.in b/libtommath/Makefile.in
>> index dbcd2a0..c06187a 100644
>> --- a/libtommath/Makefile.in
>> +++ b/libtommath/Makefile.in
>> @@ -17,7 +17,7 @@ endif
>>  ifneq ($V,1)
>>  @echo "   * ${CC} $@"
>>  endif
>> -${silent} ${CC} -c ${CFLAGS} $^ -o $@
>> +${silent} ${CC} -c ${CFLAGS} $*.c -o $@
>>
>>  #default files to install
>>  ifndef LIBNAME
>
> The above change is not quite correct; primarily, it breaks
> out-of-tree builds (which are already broken, and will be fixed
> shortly in another patch that I'll be submitting).
>
> The patch above alters the recipe for all object files (the rule
> not shown is `%.o: %.c'); in the original, the automatic variable
> `$^' is replaced with all of the prerequisites for the target in
> question, but this didn't work, because another part of this patch
> tells `make' that some of the prerequisites are header files, and
> thus `$^' expands to include those header files as well, which the
> compiler doesn't appreciate.
>
> The solution provided here just uses the pattern-matching stem
> of the target, provided by the automatic variable `$*', in order
> to construct the relevant `*.c' file. Alas, during an out-of-tree
> build, the stem is not enough to construct the path to the source
> code that needs to be compiled.
>
> In order to benefit from the search-for-prerequisites performed by
> `make', it's necessary to use the `$^' automatic variable, so the
> real solution to all of the above is just to filter the expansion
> of `$^' so that only `*.c' prerequisites are placed on the command
> line.
>
> More concretely, the above patch *should* look like this:
>
>   diff --git a/libtommath/Makefile.in b/libtommath/Makefile.in
>   index dbcd2a0..c06187a 100644
>   --- a/libtommath/Makefile.in
>   +++ b/libtommath/Makefile.in
>   @@ -17,7 +17,7 @@ endif
>ifneq ($V,1)
>   @echo "   * ${CC} $@"
>endif
>   -   ${silent} ${CC} -c ${CFLAGS} $^ -o $@
>   +   ${silent} ${CC} -c ${CFLAGS} $(filter %.c,$^) -o $@
>
>#default files to install
>ifndef LIBNAME
>
> As out-of-tree builds are already broken, there's little value in
> updating this particular patch; instead, the above improvement on
> this patch will be incorporated in a future patch that I'll submit
> soon for the purpose of fixing out-of-tree builds.

Good news, everyone!

It turns out that the GCC option `-c' can be combined with `-o' only
when there is one translation unit (that is, one `*.c' file) as input
on the command line.

Thus, using the `filter' function of GNU `make' is overkill here; it
would be enough to use `$<', which is described by:

  info '(make)Automatic Variables'

with the following:

  '$<'
   The name of the first prerequisite.  If the target got its recipe
   from an implicit rule, this will be the first prerequisite added by
   the implicit rule (*note Implicit Rules::).

An `implicit rule' is described here:

  info '(make)Implicit Rules'

which states:

  You can define your own implicit rules by writing "pattern rules".

Well, the above rule in question (`%.o: %.c') is a pattern rule,
which means it defines an implicit rule, which means that `$<'
should be the one, desired translation unit.

More concretely, the above patch *should* look like this:

  diff --git a/libtommath/Makefile.in b/libtommath/Makefile.in
  index dbcd2a0..c06187a 100644
  --- a/libtommath/Makefile.in
  +++ b/libtommath/Makefile.in
  @@ -17,7 +17,7 @@ endif
   ifneq ($V,1)
@echo "   * ${CC} $@"
   endif
  - ${silent} ${CC} -c ${CFLAGS} $^ -o $@
  + ${silent} ${CC} -c ${CFLAGS} $< -o $@

   #default files to install
   ifndef LIBNAME

I'll incorporate this change as part of the final, consolidated
patch series.

Sincerely,
Michael Witten


Re: [PATCH 1/2] Clean up: Makefile.in: Correct the prerequisites

2017-07-12 Thread Michael Witten
On Tue, 11 Jul 2017 05:08:56 -, Michael Witten wrote:

> diff --git a/libtommath/Makefile.in b/libtommath/Makefile.in
> index dbcd2a0..c06187a 100644
> --- a/libtommath/Makefile.in
> +++ b/libtommath/Makefile.in
> @@ -17,7 +17,7 @@ endif
>  ifneq ($V,1)
>   @echo "   * ${CC} $@"
>  endif
> - ${silent} ${CC} -c ${CFLAGS} $^ -o $@
> + ${silent} ${CC} -c ${CFLAGS} $*.c -o $@
>
>  #default files to install
>  ifndef LIBNAME

The above change is not quite correct; primarily, it breaks
out-of-tree builds (which are already broken, and will be fixed
shortly in another patch that I'll be submitting).

The patch above alters the recipe for all object files (the rule
not shown is `%.o: %.c'); in the original, the automatic variable
`$^' is replaced with all of the prerequisites for the target in
question, but this didn't work, because another part of this patch
tells `make' that some of the prerequisites are header files, and
thus `$^' expands to include those header files as well, which the
compiler doesn't appreciate.

The solution provided here just uses the pattern-matching stem
of the target, provided by the automatic variable `$*', in order
to construct the relevant `*.c' file. Alas, during an out-of-tree
build, the stem is not enough to construct the path to the source
code that needs to be compiled.

In order to benefit from the search-for-prerequisites performed by
`make', it's necessary to use the `$^' automatic variable, so the
real solution to all of the above is just to filter the expansion
of `$^' so that only `*.c' prerequisites are placed on the command
line.

More concretely, the above patch *should* look like this:

  diff --git a/libtommath/Makefile.in b/libtommath/Makefile.in
  index dbcd2a0..c06187a 100644
  --- a/libtommath/Makefile.in
  +++ b/libtommath/Makefile.in
  @@ -17,7 +17,7 @@ endif
   ifneq ($V,1)
@echo "   * ${CC} $@"
   endif
  - ${silent} ${CC} -c ${CFLAGS} $^ -o $@
  + ${silent} ${CC} -c ${CFLAGS} $(filter %.c,$^) -o $@
  
   #default files to install
   ifndef LIBNAME

As out-of-tree builds are already broken, there's little value in
updating this particular patch; instead, the above improvement on
this patch will be incorporated in a future patch that I'll submit
soon for the purpose of fixing out-of-tree builds.

Sincerely,
Michael Witten