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

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

Hello Bob,
Would one of the 2 solutions detailed in the last message fit?
Just a thought: to allow building ASL files before C files, an ordering must be done somewhere. Either it shall be hard-coded into the build system, either it shall be possible for people to modify it. In the latter case, this order could be in module specific INF file, or in for larger scope in the build_rule.txt file.
If you have any other angle, idea on how we could do it, please let me know.

Regards,
Pierre

-----Original Message-----
From: Feng, Bob C <bob.c.feng@intel.com> 
Sent: Thursday, April 23, 2020 10:37 AM
To: Pierre Gondois <Pierre.Gondois@arm.com>; devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.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,

Sorry for the late response. I have no good idea to resolve the concern about the INF syntax change, but I think the ASL compilation should not be done in AutoGen phase.

I summarize the solutions that we have discussed.
1. Put the ASL build target before the C build target. 
    This solution would not work when the make program enable multiple-threads.
2. Separate your original driver into a lib and a driver. The library include the asl file and the driver include c files. Make the driver depends on the library, so that the library can be built before the driver and the .hex file will be generated before build the c file.
    This solution break the module functional integrity.
3. Add the syntax in INF file to support source file dependency between source file.
    This solution should work. But the dependency descripted in the Makefile looks not accurate since you would see a cfile.obj depends on a aslfile.aml. The cfile.obj should depend on aslfile.hex, but basetool only know the output of a ASL file is a AML file. And this solution looks complicated.
4. Build ASL in AutoGen phase.
    Compilation activities should belong to Make phase. 

We might need to figure out a better solution.

Thanks,
Bob

-----Original Message-----
From: Pierre Gondois <Pierre.Gondois@arm.com>
Sent: Tuesday, April 21, 2020 2:16 AM
To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>; Feng, Bob C <bob.c.feng@intel.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

Hello Bob, Michael and Liming,
I don't have a clear idea on which direction to go in order to enable building ASL files before C files in edk2. I would need an answer to agree on which direction I should continue to investigate. For the record, the matching patch in edk2 is available at https://edk2.groups.io/g/devel/message/56662 and
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2425

Regards,
Pierre

-----Original Message-----
From: Pierre Gondois
Sent: Wednesday, April 15, 2020 4:41 PM
To: devel@edk2.groups.io; michael.d.kinney@intel.com; Feng, Bob C <bob.c.feng@intel.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

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)'
> 
> 
> 
> 
> 
>