Re: [edk2-devel] [PATCH v1 1/1] INF Spec: Add file dependency to [Sources] syntax

["PierreGondois" <pierre.gondois@arm.com>]

Hello Bob, Liming and Michael,
This is just a re-post of my last message on the Bugzilla ticket available at https://bugzilla.tianocore.org/show_bug.cgi?id=2646 

I wanted to be sure we agreed before trying to make an implementation. It seems that in edk2 build system, the steps described below are followed (in the same order): 
 1. Autogen.c/h files are generated, using UNI files among other files.
 2. Makefiles are generated by:
   2.1. Probing the files in the module (from the [Sources] section of the ".inf" file).
   2.2. For each file probed, copy/pasting the [Commands] section of the "Conf/build_rule.txt" file relevant for the file extension. This allows to generate the file of the [OutputFile] section. For an ASL file, the list of commands at [Acpi-Source-Language-File].<Command.GCC> will be copy/pasted (if someone uses GCC).

The UNI files are processes in the first step, when generating Autogen.c/h files. As the [Unicode-Text-File].<Command> section is empty, no command needs to be executed for UNI files in the second step. It is possible for the Makefile to reference the Autogen.h file because it has already been created.

Based on how UNI files are handled, we can process ASL files before the Makefile generation step. The build steps would then be:
1. Autogen.c/h files are generated, using UNI files among other files.
2. ASL files are compiled.
3. Makefiles are generated by:
  3.1. Probing the files in the module (from the [Sources] section of the ".inf" file).
  3.2. For each file probed, copy/pasting the [Commands] section of the "Conf/build_rule.txt" file relevant for the file extension. This allows to generate the file of the [OutputFile] section. For an ASL file, the list of commands at [Acpi-Source-Language-File].<Command.GCC> will be copy/pasted (if someone uses GCC).

However, this would imply:
 - Not building ASL files by calling the Makefile. Indeed, if ASL files are built before the Makefile generation, they cannot be built by the Makefile.
 - Having a hard-coded build order for ASL files. They will always be built before other files (excepted form UNI files. We can still choose to build ASL files before or after UNI files)
 - In a module having ASL files, always including the HEX files generated by the compilation of ASL files. Indeed, the build dependency between C and ASL files still needs to be resolved.

Please let me know if you are ok with this solution,

Regards,
Pierre

P.S.:
The options that we have already tried are:
 - Option 1: When listing the build target in the "CODA_TARGET" variable, the AML build target are put first. This was the first implementation available at https://edk2.groups.io/g/devel/message/49659 . The issue for this solution is that there is no explicit dependency between a C file and an AML/HEX file (the C file needs to include the HEX file generated while compiling an ASL file). For a multi-thread build, the build will fail, as you pointed out at https://edk2.groups.io/g/devel/message/50477
 - Option 2: Modifying INF files to describe dependencies between source files of different languages. This allows to handle multi-thread builds, but the INF specification needs to be modified. Patch available at https://edk2.groups.io/g/devel/message/56663


-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael D Kinney via Groups.Io
Sent: 01 April 2020 17:19
To: devel@edk2.groups.io; Feng, Bob C <bob.c.feng@intel.com>; Pierre Gondois <Pierre.Gondois@arm.com>; Sami Mujawar <Sami.Mujawar@arm.com>; Gao, Liming <liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Cc: nd <nd@arm.com>
Subject: Re: [edk2-devel] [PATCH v1 1/1] INF Spec: Add file dependency to [Sources] syntax

Hi Pierre,

We discussed this in the bug scrub yesterday.

We think there may be a simpler way to address this issue without extending the INF syntax.

It is our understanding that the ASL files need to be processed before C files when both are present in an INF.

This is similar to the requirement that UNI files be processed before C files that is already supported.

Please continue the discussion in Bugzilla with Bob.

Thanks,

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Bob 
> Feng
> Sent: Wednesday, April 1, 2020 1:53 AM
> To: devel@edk2.groups.io; pierre.gondois@arm.com; Sami Mujawar 
> <Sami.Mujawar@arm.com>; Gao, Liming <liming.gao@intel.com>
> Cc: nd <nd@arm.com>
> Subject: Re: [edk2-devel] [PATCH v1 1/1] INF Spec: Add file dependency 
> to [Sources] syntax
> 
> Hi Pierre,
> 
> I will review the spec and code change in this week.
> 
> Thanks,
> Bob
> 
> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of 
> PierreGondois
> Sent: Monday, March 30, 2020 11:52 PM
> To: devel@edk2.groups.io; Sami Mujawar <Sami.Mujawar@arm.com>; Feng, 
> Bob C <bob.c.feng@intel.com>; Gao, Liming <liming.gao@intel.com>
> Cc: nd <nd@arm.com>
> Subject: Re: [edk2-devel] [PATCH v1 1/1] INF Spec: Add file dependency 
> to [Sources] syntax
> 
> Hello Liming and Bob,
> I couldn't find the list of maintainers for the specification files, 
> but it seems Liming is a maintainer.
> If a maintainer is missing, please feel free to cc him,
> 
> Regards,
> Pierre
> 
> -----Original Message-----
> From: PierreGondois <pierre.gondois@arm.com>
> Sent: Monday, March 30, 2020 4:43 PM
> To: devel@edk2.groups.io
> Cc: Pierre Gondois <Pierre.Gondois@arm.com>; liming.gao@intel.com; 
> sami.mujawar@arm.org; nd <nd@arm.com>
> Subject: [PATCH v1 1/1] INF Spec: Add file dependency to [Sources] 
> syntax
> 
> From: Pierre Gondois <Pierre.Gondois@arm.com>
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2464
> 
> When building an edk2 module, a C file was including a .hex file 
> generated by the compilation of an ASL file.
> To describe this dependency between an ASL file and a C file, the edk2 
> patch,
>  - named "BaseTools: Build ASL files before C files",
>  - discussed at:
> https://edk2.groups.io/g/devel/message/52550
> has been created.
> This patch allows to establish build dependencies in the [Sources] 
> section, between files that are not of the same language.
> E.g.:
> [Sources]
>   FileName1.X
>   FileName2.Y : FileName1.X
>   FileName3.Z : FileName1.X FileName2.Y
> 
> Here:
>   * FileName1.X will be built prior to FileName2.Y.
>   * FileName1.X and FileName2.Y will be built prior to
>     FileName3.Z.
> 
> This patch updates the Inf specification accordingly.
> 
> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> ---
> 
> The changes can be seen at
> https://github.com/PierreARM/edk2-
> InfSpecification/tree/Bugzilla_2464_Enable_Build_Dependen
> cies_v1
> 
> Notes:
>     v1:
>      - Enable build dependencies in the [Sources] section
> 
>  2_inf_overview/25_[sources]_section.md
> | 12 ++++++++++++
>  3_edk_ii_inf_file_format/32_component_inf_definition.md
> |  3 +++
>  3_edk_ii_inf_file_format/39_[sources]_sections.md
> |  6 ++++--
>  README.md
> |  1 +
>  4 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/2_inf_overview/25_[sources]_section.md
> b/2_inf_overview/25_[sources]_section.md
> index
> 54757e61e37268eed293a5288e607cf2c7cfacf6..5b9f0a8395ef2be
> 4497d99197dc695625d841830 100644
> --- a/2_inf_overview/25_[sources]_section.md
> +++ b/2_inf_overview/25_[sources]_section.md
> @@ -2,6 +2,7 @@
>    2.5 [Sources] Section
> 
>    Copyright (c) 2007-2019, Intel Corporation. All rights 
> reserved.<BR>
> +  Copyright (c) 2020, ARM Limited. All rights
> reserved.<BR>
> 
>    Redistribution and use in source (original document
> form) and 'compiled'
>    forms (converted to PDF, epub, HTML and other formats) with or 
> without @@ -94,6 +95,17 @@ The following is an example for sources 
> sections.
> 
>  ```
> 
> +The following example depicts the syntax to establish
> dependencies
> +between files of different source types. As shown in the
> example below,
> +Dsdt.asl will be compiled before DadtHandler.c:
> +
> +```ini
> +[Sources.common]
> +  DsdtHandler.c : Dsdt.asl
> +  DsdtHandler.h
> +  Dsdt.asl
> +```
> +
>  All Unicode files must be listed in the source section.
> If a Unicode file,  `A.uni`, has the statement: `#include B.uni`, and 
> `B.uni` has a statement:
>  `#include C.uni`, both `B.uni` and `C.uni` files must be listed in 
> the INF diff --git 
> a/3_edk_ii_inf_file_format/32_component_inf_definition.md
> b/3_edk_ii_inf_file_format/32_component_inf_definition.md
> index
> 164771cb4cfff6e81fbf762a67ff741c190cecde..d776714c24c0baf
> 2348f53dc2576c9feb6f3cb5e 100644
> ---
> a/3_edk_ii_inf_file_format/32_component_inf_definition.md
> +++
> b/3_edk_ii_inf_file_format/32_component_inf_definition.md
> @@ -2,6 +2,7 @@
>    3.2 Component INF Definition
> 
>    Copyright (c) 2007-2019, Intel Corporation. All rights 
> reserved.<BR>
> +  Copyright (c) 2020, ARM Limited. All rights
> reserved.<BR>
> 
>    Redistribution and use in source (original document
> form) and 'compiled'
>    forms (converted to PDF, epub, HTML and other formats) with or 
> without @@ -133,6 +134,8 @@ The following are common definitions used 
> by multiple section types.
>  <Eq>                 ::= <TS> "=" <TS>
>  <FieldSeparator>     ::= "|"
>  <FS>                 ::= <TS> <FieldSeparator> <TS>
> +<SrcDepSeperator>    ::= ":"
> +<DepS>               ::= <TS> <SrcDepSeperator> <TS>
>  <Wildcard>           ::= "*"
>  <CommaSpace>         ::= "," <Space>*
>  <Cs>                 ::= "," <Space>*
> diff --git
> a/3_edk_ii_inf_file_format/39_[sources]_sections.md
> b/3_edk_ii_inf_file_format/39_[sources]_sections.md
> index
> 810995df26ba409ca2cf3ebe6238aa5d55cf81f1..ac966425101fd44
> a57b09d36f95a0f732eab1c59 100644
> --- a/3_edk_ii_inf_file_format/39_[sources]_sections.md
> +++ b/3_edk_ii_inf_file_format/39_[sources]_sections.md
> @@ -2,6 +2,7 @@
>    3.9 [Sources] Sections
> 
>    Copyright (c) 2007-2019, Intel Corporation. All rights 
> reserved.<BR>
> +  Copyright (c) 2020, ARM Limited. All rights
> reserved.<BR>
> 
>    Redistribution and use in source (original document
> form) and 'compiled'
>    forms (converted to PDF, epub, HTML and other formats) with or 
> without @@ -74,7 +75,8 @@ This section is not valid for a generated 
> "As Built" binary INF file.
>  <Options>            ::= <FS> [<Family>] [<opt1>]
>  <opt1>               ::= <FS> [<TagName>] [<opt2>]
>  <opt2>               ::= <FS> [<ToolCode>] [<opt3>]
> -<opt3>               ::= <FS> [<FeatureFlagExpress>]
> +<opt3>               ::= <FS> [<FeatureFlagExpress>]
> [<opt4>]
> +<opt4>               ::= <DepS> <FileNameDependency>+
>  <Family>             ::= {"MSFT"} {"GCC"} {"INTEL"}
> {<Wildcard>}
>  <TagName>            ::= {<ToolWord>} {"*"}
>  <ToolCode>           ::= _CommandCode_
> @@ -83,7 +85,7 @@ This section is not valid for a generated "As Built" 
> binary INF file.
> 
>  #### Parameters
> 
> -**_Filename_**
> +**_Filename_, _FileNameDependency_**
> 
>  Paths listed in the filename elements of the `[Sources]` section must 
> be  relative to the directory the INF file resides in. Use of "..", 
> "." and "../"
> diff --git a/README.md b/README.md
> index
> 60fba19fd67fd8d3dd33199de23f9bfe20aea7c9..4f771fc0f6e4ff5
> 16be95b1879d58329ab3bbecc 100644
> --- a/README.md
> +++ b/README.md
> @@ -203,3 +203,4 @@ Copyright (c) 2007-2017, Intel Corporation. All 
> rights reserved.
>  |            |
> [#1162](https://bugzilla.tianocore.org/show_bug.cgi?id=11
> 62) Correct the item in Table 1 to align with 3.4 section
> |               |
>  | 1.28       |
> [#1453](https://bugzilla.tianocore.org/show_bug.cgi?id=14
> 53) Update INF spec to remove EDK related contents
> | Mar 2019      |
>  | 1.29       |
> [#1952](https://bugzilla.tianocore.org/show_bug.cgi?id=19
> 52) Add new MODULE_TYPE HOST_APPLICATION
> | July 2019      |
> +|            |
> [#2646](https://bugzilla.tianocore.org/show_bug.cgi?id=26
> 46) Add file dependency to [Sources] syntax
> |                |
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> 
> 
> 
> 
> 
>