Re: [PATCH] Revert "BaseTools/Conf: Fix Dynamic-Library-File template"

["Konstantin Aladyshev" <aladyshev22@gmail.com>]

Thanks Jake!

I've also verified that this change is enough to fix the cycle dependency issue.
I've sent an updated patch to the mailing list.

Best regards,
Konstantin Aladyshev

On Tue, Dec 13, 2022 at 6:42 PM Jake Garver <jake@nvidia.com> wrote:
>
> Hi, Konstantin,
>
> Thanks for working with me offline to reproduce this.
>
> Short answer:  We can live without the "$(DEBUG_DIR)(+)$(MODULE_NAME).efi" output and I think removing that will resolve your issue.  Rather than revert the original change, can you submit a new patch that removes just that line, after verifying?  Thanks.
>
> Long answer:  The problem is in how AutoGen/ModuleAutoGen.py builds up RuleChain in _ApplyBuildRule().  It cannot handle two outputs of the same type.  In this case, we have "$(OUTPUT_DIR)(+)$(MODULE_NAME).efi" and "$(DEBUG_DIR)(+)$(MODULE_NAME).efi".  In the _ApplyBuildRule(), each gets a FileType="EFI-IMAGE".  That looks like a cyclic dep, but really, it's just a fork in the dep tree.  I think it would be better if _ApplyBuildRule() would consider the output names instead of the output types, but that would be a significant change.  And one that I think is unnecessary at this stage.
>
> In this case, I think we can live without identifying "$(DEBUG_DIR)(+)$(MODULE_NAME).efi" as an output of the build rule.  Then we'll dodge the _ApplyBuildRule() issue.  My original issue was with an unidentified dependency on the "map" output, but I also added the debug efi for good measure because it is also an output of that rule template.  However, it appears to be an output that nothing depends on, so it's unnecessary.
>
> Thanks,
> Jake
>
> ________________________________
> From: Konstantin Aladyshev <aladyshev22@gmail.com>
> Sent: Monday, December 12, 2022 11:07 AM
> To: Jake Garver <jake@nvidia.com>
> Cc: devel@edk2.groups.io <devel@edk2.groups.io>; bob.c.feng@intel.com <bob.c.feng@intel.com>; gaoliming@byosoft.com.cn <gaoliming@byosoft.com.cn>; yuwei.chen@intel.com <yuwei.chen@intel.com>
> Subject: Re: [PATCH] Revert "BaseTools/Conf: Fix Dynamic-Library-File template"
>
> External email: Use caution opening links or attachments
>
>
> Hi, Jake!
>
> No, unfortunately I don't have any way to fix this, besides the patch revert.
>
> To reproduce the issue you can add the mentioned PCI_* defines to some
> simple DXE_DRIVER.
>
> For example:
>
> SimpleDriver/SimpleDriver.inf
> ```
> [Defines]
>   INF_VERSION                    = 1.25
>   BASE_NAME                      = SimpleDriver
>   FILE_GUID                      = 384aeb18-105d-4af1-bf17-5e349e8f4d4c
>   MODULE_TYPE                    = UEFI_DRIVER
>   VERSION_STRING                 = 1.0
>   ENTRY_POINT                    = SimpleDriverEntryPoint
>   UNLOAD_IMAGE                   = SimpleDriverUnload
>   PCI_VENDOR_ID                  = 0x1234
>   PCI_DEVICE_ID                  = 0x5678
>   PCI_CLASS_CODE                 = 0x0001
>   PCI_REVISION                   = 0x0002
>
> [Sources]
>   SimpleDriver.c
>
> [Packages]
>   MdePkg/MdePkg.dec
>
> [LibraryClasses]
>   UefiDriverEntryPoint
>   UefiLib
> ```
>
> SimpleDriver/SimpleDriver.c
> ```
> #include <Library/UefiBootServicesTableLib.h>
> #include <Library/UefiLib.h>
>
>
> EFI_STATUS
> EFIAPI
> SimpleDriverUnload (
>   EFI_HANDLE ImageHandle
>   )
> {
>   return EFI_SUCCESS;
> }
>
> EFI_STATUS
> EFIAPI
> SimpleDriverEntryPoint (
>   IN EFI_HANDLE        ImageHandle,
>   IN EFI_SYSTEM_TABLE  *SystemTable
>   )
> {
>   return EFI_SUCCESS;
> }
> ```
> It is enough to reproduce the issue.
> Normally the build system should compile the driver code fine and then
> call the EfiRom utility to produce the OptionROM image. But with the
> d372ab585a2cdc5348af5f701c56c631235fe698 in place the driver build
> fails.
>
> Best regards,
> Konstantin Aladyshev
>
> On Mon, Dec 12, 2022 at 6:35 PM Jake Garver <jake@nvidia.com> wrote:
> >
> > Hi, Konstantin,
> >
> > Do you have a fix for the cyclic redundancy issue when building OptionROMs?  If not, can you help me reproduce it?
> >
> > I'd hate to revert d372ab as it fixed dependency issues we frequently ran into during parallel builds.
> >
> > Thanks,
> > Jake
> > ________________________________
> > From: Konstantin Aladyshev <aladyshev22@gmail.com>
> > Sent: Monday, December 12, 2022 8:09 AM
> > To: devel@edk2.groups.io <devel@edk2.groups.io>
> > Cc: bob.c.feng@intel.com <bob.c.feng@intel.com>; gaoliming@byosoft.com.cn <gaoliming@byosoft.com.cn>; yuwei.chen@intel.com <yuwei.chen@intel.com>; Jake Garver <jake@nvidia.com>; Konstantin Aladyshev <aladyshev22@gmail.com>
> > Subject: [PATCH] Revert "BaseTools/Conf: Fix Dynamic-Library-File template"
> >
> > External email: Use caution opening links or attachments
> >
> >
> > Revert commit d372ab585a2cdc5348af5f701c56c631235fe698.
> >
> > EdkII build system supports OptionROM generation if particular PCI_*
> > defines are present in the module INF file:
> > ```
> > [Defines]
> >   ...
> >   PCI_VENDOR_ID                  = <...>
> >   PCI_DEVICE_ID                  = <...>
> >   PCI_CLASS_CODE                 = <...>
> >   PCI_REVISION                   = <...>
> > ```
> > Although after the commit d372ab585a2cdc5348af5f701c56c631235fe698
> > it is no longer possible.
> > The build system fails with the error:
> > ```
> > Cyclic dependency detected while generating rule for
> > "<...>/DEBUG/<...>.efi" file
> > ```
> > Revert d372ab585a2cdc5348af5f701c56c631235fe698 until the issue
> > is resolved.
> > ---
> >  BaseTools/Conf/build_rule.template | 2 --
> >  1 file changed, 2 deletions(-)
> >
> > diff --git a/BaseTools/Conf/build_rule.template b/BaseTools/Conf/build_rule.template
> > index af4819de92..32053f6353 100755
> > --- a/BaseTools/Conf/build_rule.template
> > +++ b/BaseTools/Conf/build_rule.template
> > @@ -342,8 +342,6 @@
> >
> >
> >      <OutputFile>
> >
> >          $(OUTPUT_DIR)(+)$(MODULE_NAME).efi
> >
> > -        $(DEBUG_DIR)(+)$(MODULE_NAME).efi
> >
> > -        $(OUTPUT_DIR)(+)$(MODULE_NAME).map
> >
> >
> >
> >      <Command.MSFT, Command.INTEL, Command.CLANGPDB>
> >
> >          "$(GENFW)" -e $(MODULE_TYPE) -o ${dst} ${src} $(GENFW_FLAGS)
> >
> > --
> > 2.34.1
> >