770498 - Move configure logic out of client.mk and into standalone file

Status: RESOLVED WONTFIX

(Firefox Build System :: General, defect)

Description

[Joey Armstrong [:joey]]

move configure logic out of client.mk and config/rules.mk into config/makefiles/configure.mk and add unit tests.

Comment 1

[Gregory Szorc [:gps]]

This would also make mach's configure invocation much more pleasant and would reduce the redundant definition, which has been complained about in bug 780329.

mach will have requirements for invoking configure. But, I'm content with addressing those as a follow-up bug. Having taken a stab at this myself, this task is hairy enough as it is: it doesn't need any more complication.

Comment 2

[Joey Armstrong [:joey]]

Attachment: move config related logic into config/makefiles/configure.mk

Comment 3

[Joey Armstrong [:joey]]

Attachment: move config related logic into config/makefiles/configure.mk

Comment 4

[Joey Armstrong [:joey]]

Comment on attachment 658577 [details] [diff] [review]
move config related logic into config/makefiles/configure.mk

Move configure logic from client.mk and rules.mk into config/makefiles configure.mk

Comment 5

[Joey Armstrong [:joey]]

ping on the code review

Comment 6

[Gregory Szorc [:gps]]

(In reply to Joey Armstrong [:joey] from comment #5)
> ping on the code review

Sorry. I've been sick for the past week and I considered this beyond the complexity threshold I trust myself with reviewing while sick. I'll try to look at it today.

Comment 7

[Gregory Szorc [:gps]]

Comment on attachment 658577 [details] [diff] [review]
move config related logic into config/makefiles/configure.mk

Review of attachment 658577 [details] [diff] [review]:
-----------------------------------------------------------------

configure.mk is evaluated in 2 contexts: from client.mk and from per-directory rules.mk. This feels awkward to me.

Instead of trying to hack in the rules.mk bits into the same file, either split them into a separate .mk file or remove them from this patch altogether. That is, unless there is a benefit to now having configure logic available to rules.mk. Is there?

Is there a try build anywhere?

::: config/makefiles/configure.mk
@@ +7,5 @@
> +#
> +
> +ifndef INCLUDED_CONFIGURE_MK #{
> +
> +.PHONY: configure

This will get added via per-directory inclusion. I don't think this is intended. See global comment.

@@ +18,5 @@
> +  AUTOCONF ?= $(shell which autoconf-2.13 autoconf2.13 autoconf213 2>/dev/null \
> +      | grep -v '^no autoconf' | head -1)
> +
> +  ifeq (,$(strip $(AUTOCONF)))
> +    AUTOCONF=$(error Could not find autoconf 2.13)

No need for assignment here (I think).

@@ +40,5 @@
> +    $(topsrcdir)/configure \
> +    $(topsrcdir)/js/src/configure \
> +    $(NULL)
> +
> +# gather files once

What's with the different indenting levels here?

@@ +50,5 @@
> +                $(NULL))
> +
> +MAKEFILE      = $(filter %/Makefile,$(tmpfileglob))
> +CONFIG_STATUS = $(filter %/config.status,$(tmpfileglob))
> +CONFIG_CACHE  = $(filter %/config.cache,$(tmpfileglob))

How about defining these 3 variables explicitly (e.g. MAKEFILE := $(OBJDIR)/Makefile) and then composing the full list variable from the glob results + these 3. This will result in much more readable code.

@@ +55,5 @@
> +
> +EXTRA_CONFIG_DEPS := \
> +  $(topsrcdir)/aclocal.m4 \
> +  $(topsrcdir)/js/src/aclocal.m4 \
> +  $(filter %.m4,$(tmpfileglob)) \

Ditto.

@@ +86,5 @@
> +ifeq ($(topsrcdir),$(OBJDIR))
> +  CONFIGURE = ./configure
> +else
> +  CONFIGURE = $(topsrcdir)/configure
> +endif

Hmmm. Do we need to support in-srcdir builds? Also, why not always define configure in terms of $(topsrcdir)? Is that ever wrong?

@@ +98,5 @@
> +  save-mozconfig \
> +  $(NULL)
> +
> +save-mozconfig: $(FOUND_MOZCONFIG)
> +	-cp $(FOUND_MOZCONFIG) $(OBJDIR)/.mozconfig

What happens if ifndef FOUND_MOZCONFIG? I think we need either an errorOnEmpty or conditionals here.

@@ +108,5 @@
> +        $(CONFIGURE_ENV_ARGS) \
> +        $(CONFIGURE) $(CONFIGURE_ARGS) \
> +	  || ( echo "*** Fix above errors and then restart with\
> +               \"$(MAKE) -f client.mk build\"" && exit 1 )
> +	@touch $(OBJDIR)/Makefile

I would really like to see this touch go away. What's keeping it around?

@@ +119,5 @@
> +$(OBJDIR)/Makefile: $(CONFIG_STATUS_DEPS)
> +endif
> +	@$(MAKE) -f $(topsrcdir)/client.mk configure
> +
> +ifneq (,$(CONFIG_STATUS))

When is this false?

@@ +121,5 @@
> +	@$(MAKE) -f $(topsrcdir)/client.mk configure
> +
> +ifneq (,$(CONFIG_STATUS))
> +$(OBJDIR)/config/autoconf.mk: $(topsrcdir)/config/autoconf.mk.in
> +	$(PYTHON) $(OBJDIR)/config.status -n --file=$(OBJDIR)/config/autoconf.mk

$(OBJDIR)/config.status -> $(CONFIG_STATUS) ?

::: config/rules.mk
@@ +1133,5 @@
>  	@$(TOUCH) $@
>  endif
>  endif
>  
> +include $(topsrcdir)/config/makefiles/configure.mk

Part of me wishes this not to be part of this patch. But, meh.

::: js/src/config/makefiles/configure.mk
@@ +38,5 @@
> +  # 'configure' scripts generated by autoconf.
> +  CONFIGURES = \
> +    $(topsrcdir)/configure \
> +    $(topsrcdir)/js/src/configure \
> +    $(NULL)

It feels weird having this in js/src. But, such is check-dirs-sync.

Comment 8

[Joey Armstrong [:joey]]

(In reply to Gregory Szorc [:gps] from comment #7)
> Comment on attachment 658577 [details] [diff] [review]
> move config related logic into config/makefiles/configure.mk
> 
> Review of attachment 658577 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> configure.mk is evaluated in 2 contexts: from client.mk and from
> per-directory rules.mk. This feels awkward to me.
> 
> Instead of trying to hack in the rules.mk bits into the same file, either
> split them into a separate .mk file or remove them from this patch
> altogether. That is, unless there is a benefit to now having configure logic
> available to rules.mk. Is there?

Potential refactoring, this patch is only gathering configure logic in the same place.  When config.status pre-exists client.mk might be able to use "config.status -n --recheck".

At a minimum all of the configure logic and file paths are maintained in one place and loaded by a single include.


> Is there a try build anywhere?

There was, I'll have to dig up the url


> ::: config/makefiles/configure.mk
> @@ +7,5 @@
> > +#
> > +
> > +ifndef INCLUDED_CONFIGURE_MK #{
> > +
> > +.PHONY: configure
> 
> This will get added via per-directory inclusion. I don't think this is
> intended. See global comment.
> 
> @@ +18,5 @@
> > +  AUTOCONF ?= $(shell which autoconf-2.13 autoconf2.13 autoconf213 2>/dev/null \
> > +      | grep -v '^no autoconf' | head -1)
> > +
> > +  ifeq (,$(strip $(AUTOCONF)))
> > +    AUTOCONF=$(error Could not find autoconf 2.13)
> 
> No need for assignment here (I think).

Assignment was probably used to only fail if/when autoconf will be called rather than explicitly { ~client.mk logic }.

 
> @@ +40,5 @@
> > +    $(topsrcdir)/configure \
> > +    $(topsrcdir)/js/src/configure \
> > +    $(NULL)
> > +
> > +# gather files once
> 
> What's with the different indenting levels here?

initially to show connection to the ifdef block but not all the macros copied in were reformatted.  Should revert to left justified since target rules will not reformat nicely.


> 
> @@ +50,5 @@
> > +                $(NULL))
> > +
> > +MAKEFILE      = $(filter %/Makefile,$(tmpfileglob))
> > +CONFIG_STATUS = $(filter %/config.status,$(tmpfileglob))
> > +CONFIG_CACHE  = $(filter %/config.cache,$(tmpfileglob))
> 
> How about defining these 3 variables explicitly (e.g. MAKEFILE :=
> $(OBJDIR)/Makefile) and then composing the full list variable from the glob
> results + these 3. This will result in much more readable code.

Wildcard was used in client.mk to define the dependency only when files pre-exist.  If I remember correctly this may have been to avoid a chicken-n-egg problem with deps when building after a clean checkout.

If make can build with wildcard removed yes deps can be explicitly listed.  If not, the dep problem would need to be fixed in order to list them.


> @@ +86,5 @@
> > +ifeq ($(topsrcdir),$(OBJDIR))
> > +  CONFIGURE = ./configure
> > +else
> > +  CONFIGURE = $(topsrcdir)/configure
> > +endif
> 
> Hmmm. Do we need to support in-srcdir builds? Also, why not always define
> configure in terms of $(topsrcdir)? Is that ever wrong?

Most of this logic was copied in verbatim from client.mk.  Would probably be good to land the existing logic then refactor in a followup patch to reduce the chance of breakage.

That said if TOPSRCDIR and OBJDIR are both null maybe the conditional was used to provide a reasonable default for srcdir rather than failing downstream trying to build some cryptic, non-existent path ?!?


> @@ +98,5 @@
> > +  save-mozconfig \
> > +  $(NULL)
> > +
> > +save-mozconfig: $(FOUND_MOZCONFIG)
> > +	-cp $(FOUND_MOZCONFIG) $(OBJDIR)/.mozconfig
> 
> What happens if ifndef FOUND_MOZCONFIG? I think we need either an
> errorOnEmpty or conditionals here.

Sounds reasonable.  errorOnEmpty would be preferable, a conditional could make the target disappear.


> @@ +108,5 @@
> > +        $(CONFIGURE_ENV_ARGS) \
> > +        $(CONFIGURE) $(CONFIGURE_ARGS) \
> > +	  || ( echo "*** Fix above errors and then restart with\
> > +               \"$(MAKE) -f client.mk build\"" && exit 1 )
> > +	@touch $(OBJDIR)/Makefile
> 
> I would really like to see this touch go away. What's keeping it around?

Not sure but would wager this is probably to work around a problem with deps or partially updated targets exhibiting different behavior on subsequent calls.

This can be another item for patch #2


> 
> @@ +119,5 @@
> > +$(OBJDIR)/Makefile: $(CONFIG_STATUS_DEPS)
> > +endif
> > +	@$(MAKE) -f $(topsrcdir)/client.mk configure
> > +
> > +ifneq (,$(CONFIG_STATUS))
> 
> When is this false?

existing logic -- possibly a workaround for the chicken-n-egg problem mentioned above.


> @@ +121,5 @@
> > +	@$(MAKE) -f $(topsrcdir)/client.mk configure
> > +
> > +ifneq (,$(CONFIG_STATUS))
> > +$(OBJDIR)/config/autoconf.mk: $(topsrcdir)/config/autoconf.mk.in
> > +	$(PYTHON) $(OBJDIR)/config.status -n --file=$(OBJDIR)/config/autoconf.mk
> 
> $(OBJDIR)/config.status -> $(CONFIG_STATUS) ?

Existing logic but yes replacing with macro would be an improvement.

 
> ::: config/rules.mk
> @@ +1133,5 @@
> >  	@$(TOUCH) $@
> >  endif
> >  endif
> >  
> > +include $(topsrcdir)/config/makefiles/configure.mk
> 
> Part of me wishes this not to be part of this patch. But, meh.
> 
> ::: js/src/config/makefiles/configure.mk
> @@ +38,5 @@
> > +  # 'configure' scripts generated by autoconf.
> > +  CONFIGURES = \
> > +    $(topsrcdir)/configure \
> > +    $(topsrcdir)/js/src/configure \
> > +    $(NULL)
> 
> It feels weird having this in js/src. But, such is check-dirs-sync.

Comment 9

[Mike Hommey [:glandium]]

(In reply to Joey Armstrong [:joey] from comment #8)
> (In reply to Gregory Szorc [:gps] from comment #7)
> > Comment on attachment 658577 [details] [diff] [review]
> > move config related logic into config/makefiles/configure.mk
> > 
> > Review of attachment 658577 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > configure.mk is evaluated in 2 contexts: from client.mk and from
> > per-directory rules.mk. This feels awkward to me.
> > 
> > Instead of trying to hack in the rules.mk bits into the same file, either
> > split them into a separate .mk file or remove them from this patch
> > altogether. That is, unless there is a benefit to now having configure logic
> > available to rules.mk. Is there?
> 
> Potential refactoring, this patch is only gathering configure logic in the
> same place.  When config.status pre-exists client.mk might be able to use
> "config.status -n --recheck".

If you are using rules.mk, it means you have Makefiles. If you have Makefiles, it means they were created by config.status from Makefile.ins. If they were created by config.status, config.status exists. Considering config.status exists when Makefiles exist, Makefiles can just use config.status.

Comment 10

[Joey Armstrong [:joey]]

(In reply to Mike Hommey [:glandium] from comment #9)
> (In reply to Joey Armstrong [:joey] from comment #8)
> > (In reply to Gregory Szorc [:gps] from comment #7)
> > > Comment on attachment 658577 [details] [diff] [review]
> > > move config related logic into config/makefiles/configure.mk
> > > 
> > > Review of attachment 658577 [details] [diff] [review]:
> > > -----------------------------------------------------------------
> > > 
> > > configure.mk is evaluated in 2 contexts: from client.mk and from
> > > per-directory rules.mk. This feels awkward to me.
> > > 
> > > Instead of trying to hack in the rules.mk bits into the same file, either
> > > split them into a separate .mk file or remove them from this patch
> > > altogether. That is, unless there is a benefit to now having configure logic
> > > available to rules.mk. Is there?
> > 
> > Potential refactoring, this patch is only gathering configure logic in the
> > same place.  When config.status pre-exists client.mk might be able to use
> > "config.status -n --recheck".
> 
> If you are using rules.mk, it means you have Makefiles.

client.mk will still be in play.  The file contains more configure related logic to refactor out and rule.mk would not be a dependency/expectation for the logic.

In fact once the logic is gathered together in the same makefile deps can be added to regenerate config.status if/when needed.  That simplifies testing greatly as configure can be tested as a standalone element w/o requiring makefiles to be generated or introducing test permutations by requiring dependencies.

Comment 11

[Joey Armstrong [:joey]]

Attachment: move config related logic into config/makefiles/configure.mk

Comment 12

[Joey Armstrong [:joey]]

Comment on attachment 663118 [details] [diff] [review]
move config related logic into config/makefiles/configure.mk

First round of refactoring.
Common paths extracted, use CONFIG_STATUS in place of $(OBJDIR)/config.status.

Deps and logic in client.mk cleaned up to generate configure & friends the old fashioned way only when files do not exist.  Default to invoking config.status whenever possible.

$(TOUCH) $(OBJDIR)/Makefile removed since bootstrap deps for running configure are no longer needed.

save-mozconfig <=> $(FOUND_MOZCONFIG) logic ok as-is.  '-cp' will prevent the command from failing when a mozconfig file cannot be archived.

References to client.mk and rules.mk removed.  Replaced by 'CREATE_CONFIGURE_OK'.

Added a bogus path definition for OBJDIR= in the autotargets unit test so it would not fail when configure.mk is included.

Renamed unit test makefile template to have a .mk suffix for the code review script to pickup on.

Cleaned up minor nits picked up by the code review script.  Trailing whitespace line continuation w/o leading space, etc.

try results pending.
patch commit will eventually be bound for b-s.

Comment 13

[Gregory Szorc [:gps]]

Comment on attachment 663118 [details] [diff] [review]
move config related logic into config/makefiles/configure.mk

Review of attachment 663118 [details] [diff] [review]:
-----------------------------------------------------------------

Granting a hesitant r+. This all seems correct. However (and I haven't verified any of this locally or with try) I suspect this may break js/src builds because things now reference js/src from within js/src. So, it will look for paths like $topsrcdir/js/src/jsr/src. I could be wrong. That being said, I /think/ some (if not all?) js people run autoconf and configure manually, so maybe they won't even hit this code. Still, it seems a bit weird. But, I'm not going to let that hold up r+.

Also, there may be some bit rot from bug 793953 that you need to incorporate.

Sorry for the long review lag.

::: config/makefiles/configure.mk
@@ +9,5 @@
> +ifndef INCLUDED_CONFIGURE_MK #{
> +
> +topsrcdir ?= $(TOPSRCDIR)
> +$(call errorIfEmpty,topsrcdir)
> +OBJDIR ?= $(error OBJDIR= requires a value)

Why the different conventions for topsrcdir and OBJDIR? I think I prefer $(call errorIfEmpty) because of its immediate nature. If you really need deferred evaluation, please comment why.

@@ +127,5 @@
> +endif #}
> +
> +$(autoconf_mk): $(topsrcdir)/config/autoconf.mk.in
> +	$(config_py) --file=$@/autoconf.mk \
> +      && $(TOUCH) $@

This rule isn't part of configure per se. I don't think moving it should be part of this patch.

::: js/src/config/makefiles/configure.mk
@@ +64,5 @@
> +
> +# 'configure' scripts generated by autoconf.
> +CONFIGURES = \
> +  $(configure_sh) \
> +  $(topsrcdir)/js/src/configure \

Is this right in the context of js/src?

@@ +88,5 @@
> +	@cd $(OBJDIR) && $(BUILD_PROJECT_ARG) \
> +        $(CONFIGURE_ENV_ARGS) \
> +        $(configure_sh) $(CONFIGURE_ARGS) \
> +	  || ( echo "*** Fix above errors and then restart with\
> +               \"$(MAKE) -f client.mk build\"" && exit 1 )

This probably doesn't apply to js/src builds. But, I'm not sure the js people really care.

Comment 14

[Joey Armstrong [:joey]]

(In reply to Gregory Szorc [:gps] from comment #13)
> Comment on attachment 663118 [details] [diff] [review]
> move config related logic into config/makefiles/configure.mk
> 
> Review of attachment 663118 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Granting a hesitant r+. This all seems correct. However (and I haven't
> verified any of this locally or with try) I suspect this may break js/src
> builds because things now reference js/src from within js/src. So, it will
> look for paths like $topsrcdir/js/src/jsr/src. I could be wrong. That being
> said, I /think/ some (if not all?) js people run autoconf and configure
> manually, so maybe they won't even hit this code. Still, it seems a bit
> weird. But, I'm not going to let that hold up r+.
> 
> Also, there may be some bit rot from bug 793953 that you need to incorporate.
> 
> Sorry for the long review lag.
> 
> ::: config/makefiles/configure.mk
> @@ +9,5 @@
> > +ifndef INCLUDED_CONFIGURE_MK #{
> > +
> > +topsrcdir ?= $(TOPSRCDIR)
> > +$(call errorIfEmpty,topsrcdir)
> > +OBJDIR ?= $(error OBJDIR= requires a value)
> 
> Why the different conventions for topsrcdir and OBJDIR? I think I prefer
> $(call errorIfEmpty) because of its immediate nature. If you really need
> deferred evaluation, please comment why.

Of the two methods, deferred would be the better option - only fail if/when a variable is actually used.  If a makefile is able to scribble beneath dot, failing because OBJ= is not set would not be very friendly behavior.

Topsrcdir was an exception because the var is explicitly used by configure.mk, checkIfEmpty can report a simple error early on.

 
> @@ +127,5 @@
> > +endif #}
> > +
> > +$(autoconf_mk): $(topsrcdir)/config/autoconf.mk.in
> > +	$(config_py) --file=$@/autoconf.mk \
> > +      && $(TOUCH) $@
> 
> This rule isn't part of configure per se. I don't think moving it should be
> part of this patch.

The target should live in a library somewhere, the logic is not reusable while bundled inside client.mk.  A new makefile could be created  to hold this logic but there is usually pushback against creating new files that contain minimal content.

How about leaving the target in configure.mk so the re-use attribute is retained {autoconf is somewhat config related}.  Later when a more appropriate home is found the target can simply move between libraries or removed outright if no longer needed.


> ::: js/src/config/makefiles/configure.mk
> @@ +64,5 @@
> > +
> > +# 'configure' scripts generated by autoconf.
> > +CONFIGURES = \
> > +  $(configure_sh) \
> > +  $(topsrcdir)/js/src/configure \
> 
> Is this right in the context of js/src?

The path was moved verbatim from client.mk and still appears valid:
   % find js -name 'configure'
   js/src/configure


> @@ +88,5 @@
> > +	@cd $(OBJDIR) && $(BUILD_PROJECT_ARG) \
> > +        $(CONFIGURE_ENV_ARGS) \
> > +        $(configure_sh) $(CONFIGURE_ARGS) \
> > +	  || ( echo "*** Fix above errors and then restart with\
> > +               \"$(MAKE) -f client.mk build\"" && exit 1 )
> 
> This probably doesn't apply to js/src builds. But, I'm not sure the js
> people really care.

Ugh this message would be ugly in that context.  Probably should construct a message from available values so the error would make sense reported in either directory.

Comment 15

[Gregory Szorc [:gps]]

(In reply to Joey Armstrong [:joey] from comment #14)

> Of the two methods, deferred would be the better option - only fail if/when
> a variable is actually used.  If a makefile is able to scribble beneath dot,
> failing because OBJ= is not set would not be very friendly behavior.

I can't think of a scenario where we'd want a makefile to adjust OBJDIR after including rules.mk.
  
> > @@ +127,5 @@
> > > +endif #}
> > > +
> > > +$(autoconf_mk): $(topsrcdir)/config/autoconf.mk.in
> > > +	$(config_py) --file=$@/autoconf.mk \
> > > +      && $(TOUCH) $@
> > 
> > This rule isn't part of configure per se. I don't think moving it should be
> > part of this patch.
> 
> The target should live in a library somewhere, the logic is not reusable
> while bundled inside client.mk.  A new makefile could be created  to hold
> this logic but there is usually pushback against creating new files that
> contain minimal content.

This target isn't in client.mk: you moved it from rules.mk! Does it need to be in configure.mk rather than rules.mk?

> > ::: js/src/config/makefiles/configure.mk
> > @@ +64,5 @@
> > > +
> > > +# 'configure' scripts generated by autoconf.
> > > +CONFIGURES = \
> > > +  $(configure_sh) \
> > > +  $(topsrcdir)/js/src/configure \
> > 
> > Is this right in the context of js/src?
> 
> The path was moved verbatim from client.mk and still appears valid:
>    % find js -name 'configure'
>    js/src/configure
> 
> 
> > @@ +88,5 @@
> > > +	@cd $(OBJDIR) && $(BUILD_PROJECT_ARG) \
> > > +        $(CONFIGURE_ENV_ARGS) \
> > > +        $(configure_sh) $(CONFIGURE_ARGS) \
> > > +	  || ( echo "*** Fix above errors and then restart with\
> > > +               \"$(MAKE) -f client.mk build\"" && exit 1 )
> > 
> > This probably doesn't apply to js/src builds. But, I'm not sure the js
> > people really care.
> 
> Ugh this message would be ugly in that context.  Probably should construct a
> message from available values so the error would make sense reported in
> either directory.

At the same time I'm inclined to let it slide and have the js people complain about a follow-up to make the UX nicer.

Comment 16

[Joey Armstrong [:joey]]

(In reply to Gregory Szorc [:gps] from comment #15)
> (In reply to Joey Armstrong [:joey] from comment #14)
> 
> > Of the two methods, deferred would be the better option - only fail if/when
> > a variable is actually used.  If a makefile is able to scribble beneath dot,
> > failing because OBJ= is not set would not be very friendly behavior.
> 
> I can't think of a scenario where we'd want a makefile to adjust OBJDIR
> after including rules.mk.

Why should including rules.mk be a dependency/requirement for using configure.mk (not being flip) ?  If we were writing a standalone test for the library the include would pull in unrelated deps that could taint the environment.

Likewise if we have a makefile that expects to be working beneath $(OBJ) and can simply write in pwd w/o knowledge or use of the variable, calling checkIfEmpty(OBJ) forces either the include or artificially specifying OBJ=/dev/null on the command line to become a requirement.

Deferred checking will allow unset variables to be detected but will not break a usage case like the one above.


> > > @@ +127,5 @@
> > > > +endif #}
> > > > +
> > > > +$(autoconf_mk): $(topsrcdir)/config/autoconf.mk.in
> > > > +	$(config_py) --file=$@/autoconf.mk \
> > > > +      && $(TOUCH) $@
> > > 
> 
> This target isn't in client.mk: you moved it from rules.mk! Does it need to
> be in configure.mk rather than rules.mk?

It has been two weeks so I forgot about the entry in rules.mk :).  The line is also in client.mk ~line 319.  The autoconf target was refactored from both files to also help shrink and isolate logic in rules.mk.

client.mk line 319:
===================
ifneq (,$(CONFIG_STATUS))
$(OBJDIR)/config/autoconf.mk: $(TOPSRCDIR)/config/autoconf.mk.in
	$(PYTHON) $(OBJDIR)/config.status -n --file=$(OBJDIR)/config/autoconf.mk
endif

Comment 17

[Joey Armstrong [:joey]]

Question: this will need a refresh but are the patches still relevant ?
Refactoring config* related logic out of client.mk and friends and moving it into config/makefiles/config.mk ?

Comment 18

[Joey Armstrong [:joey]]

client.mk should be going away at some point so no need to keep this open for a refactoring.