Re: [ptxdist] [PATCH v2] ptxd_lib_dgen: warn if host or image packages have targetinstall stage

[Michael Olbrich <m.olbrich@pengutronix.de>]

On Mon, Apr 08, 2019 at 11:39:50AM +0200, Roland Hieber wrote:
> This point came up multiple times in the past and the special handling
> of targetinstall stages lead to confusion when trying to depend on image
> packages. Document it to prevent further confusion in that matter, and
> warn the user if such a make target exists.
> 
> Signed-off-by: Roland Hieber <rhi@pengutronix.de>
> ---
> On Fri, Apr 05, 2019 at 04:18:37PM +0200, Michael Olbrich wrote:
> > Hmmm, how would we detect this? I don't think make allows me to check
> > if a specific target is explicitly defined. And packages can use only
> > the default stages. 
> 
> I was first thinking about using 'make -q' to check if building an
> image-*.targetinstall stage errors out, but on the way I found out that
> the dgen script already parses all rules/*.make files, so we can hook in
> there:
> 
> ---
>  doc/ref_rule_file_layout.inc  | 12 ++++++++++++
>  scripts/lib/ptxd_lib_dgen.awk | 10 ++++++++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/doc/ref_rule_file_layout.inc b/doc/ref_rule_file_layout.inc
> index ea7eb8c4903d..f58b07b44817 100644
> --- a/doc/ref_rule_file_layout.inc
> +++ b/doc/ref_rule_file_layout.inc
> @@ -23,6 +23,18 @@ Each rule file provides PTXdist with the required steps (in PTXdist called
>  
>     - targetinstall.post
>  
> +.. note::
> +
> +  Host and image packages don't need to install anything in the target file system.
> +  Therefore, PTXdist only respects the *targetinstall* and *targetinstall.post*
> +  stages for packages whose name doesn't start with ``host-`` or ``image-``.
> +
> +  When you want to depend on the output of a certain image package, you can
> +  usually use its image name as an `additional prerequisite <make-prereq-types_>`_
> +  in your make rule for the dependent stage.
> +
> +.. _make-prereq-types: https://www.gnu.org/software/make/manual/make.html#Prerequisite-Types
> +
>  Default stage rules
>  ~~~~~~~~~~~~~~~~~~~
>  
> diff --git a/scripts/lib/ptxd_lib_dgen.awk b/scripts/lib/ptxd_lib_dgen.awk
> index f7143c02885f..4d97cbd2fa28 100644
> --- a/scripts/lib/ptxd_lib_dgen.awk
> +++ b/scripts/lib/ptxd_lib_dgen.awk
> @@ -94,6 +94,16 @@ function dump_file(src, dst, tmp) {
>  }
>  
>  
> +#
> +# warn user if an image-* or host-* package contains a targetinstall rule
> +# which will not be executed
> +#
> +match($0, /\$\(STATEDIR\)\/((image-.*|host-.*)\.targetinstall(.post)?):/, m) {

Please use the same syntax as everywhere else in the file. It's complicated
enough.

> +	print "Warning: " m[1] " stage will be ignored. " \
> +		"See section 'Rule File Layout' in the PTXdist reference for more info."

No, this should be fatal. Add a '$(error ...)' and maybe some $(warning
...) for multiple lines. Also, reference the filename and line number to
make this easier to find.

Michael

> +}
> +
> +
>  #
>  # parse "PACKAGES-$(PTXCONF_PKG) += pkg" lines, i.e. rules-files from
>  # rules/*.make. Setup mapping between upper and lower case pkg names
> -- 
> 2.20.1
> 
> 
> _______________________________________________
> ptxdist mailing list
> ptxdist@pengutronix.de

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de