Re: svn commit: r289334 - head/share/mk

2015-10-14 Thread NGie Cooper
On Wed, Oct 14, 2015 at 3:09 PM, Bryan Drewery  wrote:
...
> So the reason I have been tinkering with this code is because it is so
> mysterious to me, given the lack of comments and seemingly out-of-place
> nature of it.
>
> It turns out that even moving 'buildconfig' as a recurse target creates
> a surprising situation that will break with parallel builds since
> 'buildconfig' is hooked into 'all', thus 'all' and 'buildconfig' both
> end up recursing when calling 'make all'. This explains the make() check
> here to avoid recursing if called with 'all' (even though it is checking
> the opposite, only calling 'buildconfig' directly to recurse).  Comments
> are not a sin.

Agreed.

The original change was done over a decade ago:
https://svnweb.freebsd.org/base/head/share/mk/bsd.subdir.mk?r1=96667&r2=96668&pathrev=289334&;
. It seems that the targets used to have different names and ru@
changed them.

Succinct comments would probably be a good idea in the .mk files. I'm
not sure what the performance is like if bmake needs to
(re-)read/(re-)evaluate make snippets.

Thanks!
-NGie
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r289334 - head/share/mk

2015-10-14 Thread Bryan Drewery
On 10/14/2015 1:30 PM, Bryan Drewery wrote:
> Author: bdrewery
> Date: Wed Oct 14 20:30:32 2015
> New Revision: 289334
> URL: https://svnweb.freebsd.org/changeset/base/289334
> 
> Log:
>   Recurse on 'buildconfig' and 'installconfig'.  Remove the 'config' pseudo 
> target.
>   
>   The 'config' target isn't really needed right now so just remove it to avoid
>   any clashes with config(8) building.  It's also likely misspelled and should
>   be 'configs' if we decide to add it back.  This was just a convenience
>   target recently added.
>   
>   Sponsored by:   EMC / Isilon Storage Division
> 
> Modified:
>   head/share/mk/bsd.subdir.mk
> 
> Modified: head/share/mk/bsd.subdir.mk
> ==
> --- head/share/mk/bsd.subdir.mk   Wed Oct 14 20:28:15 2015
> (r289333)
> +++ head/share/mk/bsd.subdir.mk   Wed Oct 14 20:30:32 2015
> (r289334)
> @@ -32,9 +32,10 @@
>  .if !target()
>  :
>  
> -ALL_SUBDIR_TARGETS= all all-man checkdpadd clean cleandepend cleandir \
> - cleanilinks cleanobj depend distribute lint maninstall \
> - manlint obj objlink realinstall regress tags \
> +ALL_SUBDIR_TARGETS= all all-man buildconfig checkdpadd clean cleandepend \
> + cleandir cleanilinks cleanobj depend distribute \
> + installconfig lint maninstall manlint obj objlink \
> + realinstall regress tags \
>   ${SUBDIR_TARGETS}
>  
>  .include 
> @@ -127,7 +128,7 @@ _sub.${__target}: _SUBDIR
>  # This is to support 'make includes' calling 'make buildincludes' and
>  # 'make installincludes' in the proper order, and to support these
>  # targets as SUBDIR_TARGETS.
> -.for __target in files includes config
> +.for __target in files includes
>  .for __stage in build install
>  ${__stage}${__target}:
>  .if make(${__stage}${__target})
> 

So the reason I have been tinkering with this code is because it is so
mysterious to me, given the lack of comments and seemingly out-of-place
nature of it.

It turns out that even moving 'buildconfig' as a recurse target creates
a surprising situation that will break with parallel builds since
'buildconfig' is hooked into 'all', thus 'all' and 'buildconfig' both
end up recursing when calling 'make all'. This explains the make() check
here to avoid recursing if called with 'all' (even though it is checking
the opposite, only calling 'buildconfig' directly to recurse).  Comments
are not a sin.

-- 
Regards,
Bryan Drewery



signature.asc
Description: OpenPGP digital signature


svn commit: r289334 - head/share/mk

2015-10-14 Thread Bryan Drewery
Author: bdrewery
Date: Wed Oct 14 20:30:32 2015
New Revision: 289334
URL: https://svnweb.freebsd.org/changeset/base/289334

Log:
  Recurse on 'buildconfig' and 'installconfig'.  Remove the 'config' pseudo 
target.
  
  The 'config' target isn't really needed right now so just remove it to avoid
  any clashes with config(8) building.  It's also likely misspelled and should
  be 'configs' if we decide to add it back.  This was just a convenience
  target recently added.
  
  Sponsored by: EMC / Isilon Storage Division

Modified:
  head/share/mk/bsd.subdir.mk

Modified: head/share/mk/bsd.subdir.mk
==
--- head/share/mk/bsd.subdir.mk Wed Oct 14 20:28:15 2015(r289333)
+++ head/share/mk/bsd.subdir.mk Wed Oct 14 20:30:32 2015(r289334)
@@ -32,9 +32,10 @@
 .if !target()
 :
 
-ALL_SUBDIR_TARGETS= all all-man checkdpadd clean cleandepend cleandir \
-   cleanilinks cleanobj depend distribute lint maninstall \
-   manlint obj objlink realinstall regress tags \
+ALL_SUBDIR_TARGETS= all all-man buildconfig checkdpadd clean cleandepend \
+   cleandir cleanilinks cleanobj depend distribute \
+   installconfig lint maninstall manlint obj objlink \
+   realinstall regress tags \
${SUBDIR_TARGETS}
 
 .include 
@@ -127,7 +128,7 @@ _sub.${__target}: _SUBDIR
 # This is to support 'make includes' calling 'make buildincludes' and
 # 'make installincludes' in the proper order, and to support these
 # targets as SUBDIR_TARGETS.
-.for __target in files includes config
+.for __target in files includes
 .for __stage in build install
 ${__stage}${__target}:
 .if make(${__stage}${__target})
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"