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

["Bob Feng" <bob.c.feng@intel.com>]

Hi Pierre,

I'd prefer the option 2#. I think that is the easiest way to meet the requirement. Since the aml file is the definite output of asl file, we can use it to descript the dependency relationship between c source file and asl source code.

Thanks,
Bob

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of PierreGondois
Sent: Saturday, April 25, 2020 12:31 AM
To: Feng, Bob C <bob.c.feng@intel.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

Hello Bob,
Thank you for your answer, and I understand you need time to discuss before approving a solution.

I have two suggestions:
1. About the third option you described, it seems one of the issues is that the dependency depicted in the INF file is not accurate. To solve this, we could modify build_rule.txt to add a new rule stating that ".aml" files generate ".amlhex" files. ".amlhex" files would contain the exact same content as what our ".hex" file currently contains, this is just a renaming. Having an ASL file in a module would then trigger 2 rules of build_rule.txt, the first one compiling the ASL file to AML, and a second one compiling the AML file to a ".amlhex" file. Maybe this will require modifications of the Makefile generation.
E.g.: in a ".inf" file:
[Sources]
  file1.asl
  file2.c

The Makefile generated would then look like:
file1.aml:
  "$(ASL)"  file1.asl     # Generate file1.aml from file1.asl

file1.amlhex: file1.aml
  AmlToHex  file1.aml     # Generate file1.amlhex from file1.aml

file2.obj: file1.amlhex   # file2.obj is dependent on amlhex
  "$(CC)"   file2.c

This solution would not require a modification of the INF specification. It would however require to parse file2.c during the Makefile generation step, and if file2.c includes a ".amlhex" file, add it to the list of prerequisite in the Makefile.
The following rule:
file2.obj: file1.amlhex   # file2.obj is dependent on file1.amlhex
  "$(CC)"   file2.c
could also be splitted in 2:
file2.c: file1.amlhex     # file2.c is dependent on file1.amlhex
file2.obj: file2.c        # file2.obj is dependent on file1.c
  "$(CC)"   file2.c


2. One other solution would be to hard code during the Makefile generation that all ".obj" files (obtained by compilation of C files) are dependent on a prerequisite named "AML_FILES" (it's an example), and that the "AML_FILES" prerequisite is fulfilled by building all the ASL files. This would mean that in edk2, ASL files are always built before C files.
E.g.: in a ".inf" file:
[Sources]
  file1.asl
  file2.asl
  file3.c

The Makefile generated would then look like:
AML_FILES:
  file1.aml
  file2.aml

file1.aml:
  "$(ASL)"  file1.asl      # Generate file1.aml from file1.asl

file2.aml:
  "$(ASL)"  file2.asl      # Generate file2.aml from file2.asl

file3.obj: AML_FILES       # Fulfil the "AML_FILES" prerequisite before
  "$(CC)"   file3.c        # creating file3.obj

This solution would also not describe the real dependency, being between file1.hex, file2.hex and file1.c.

Please let me know what you think, or if you have any suggestion on how to make this change, 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)'
> 
> 
> 
> 
> 
>