> On Mar 17, 2016, at 5:15 AM, [email protected] wrote: > > Hi all, > > from my point of view a library should use a constructor to initialize OWN > resources only. That means that there is no need for external resources at > this point. The library can use PeiService/SystemTable to register for events > or protocols (when they need to react to them). Or they finalize > initialization on the first time a library function gets called. > I don't know how many EDK2 libraries would fulfill this requirement and if it > is worth to note that rule into specification (when you agree) but this way > one can get rid of dependency loops. >
Peter, It might not be a bad idea to make this a recommendation, and implement it in the key packages (MdePkg, MdeModulePkg, etc.). I've been working on the edk2 since the beginning and this is the 1st time I every remember hitting this issue in a real world situation. I'm guessing since most libraries are small and have limited dependencies we don't hit this a lot. However given the error messages you get it is quite painful to debug. I basically had to do a binary search of disabling dependent libraries to figure this out. I think the DebugLib is a special case since it is used by almost every other library, and it has the possibility of being complex and depend on a lot of other libraries. Thanks, Andrew Fish > Best Regards, > Peter Kirmeier > > -----Original Message----- > From: edk2-devel [mailto:[email protected]] On Behalf Of Ard > Biesheuvel > Sent: Thursday, March 17, 2016 11:16 AM > To: Laszlo Ersek; Gao, Liming > Cc: edk2-devel; Andrew Fish; Zeng, Star > Subject: Re: [edk2] Has any one else had issues trying to use > DxeDebugPrintErrorLevelLib? > DebugPrintErrorLevelLib|MdeModulePkg/Library/DxeDebugPrintErrorLevelLib/DxeDebugPrintErrorLevelLib.inf > > On 17 March 2016 at 10:11, Laszlo Ersek <[email protected]> wrote: >> Adding Ard >> >> On 03/17/16 08:28, Zeng, Star wrote: >>> On 2016/3/17 15:16, Andrew Fish wrote: >>>> >>>>> On Mar 17, 2016, at 12:10 AM, Gao, Liming <[email protected]> wrote: >>>>> >>>>> Andrew: >>>>> DxeDebugPrintErrorLevelLib has Constructor. DxeHobLib has >>>>> Constructor. If DebugLib also has Constructor, the circle of >>>>> Constructor() will happen and cause build break. >>>>> DxeDebugPrintErrorLevelLib instance increases the possibility of >>>>> the circle Constructor(). To resolve it, we can update >>>>> DxeDebugPrintErrorLevelLib or DxeHobLib library instance by moving >>>>> their constructor into every function API. >>>>> >>>> >>>> Liming, >>>> >>>> Thanks, I came to the same conclusion. I'm making a local copy >>>> DxeDebugPrintErrorLevelLib that does the HOB accesses in the driver >>>> without the HobLib and that will solve our issue. I was just >>>> surprised the edk2 library was not very useful in the real world. >>>> Not to mention the build error message did not help find the real issue, >>>> the HOB lib. >>> >>> I reproduced the issue locally. With the change below to integrate >>> HobLibConstructor() to GetHobList(), since all other interface of >>> DxeHobLib are just to call GetHobList(), the issue is gone. What is >>> your opinion? >> >> This could make it possible for ArmVirtPkg to remove the >> ArmVirtDxeHobLib library instance. That library instance was cloned >> exactly in order to break this dependency cycle, see commit ad90df8ac0. >> >> I think the proposal and the patch are good. >> > > As I have mentioned in the past, there is a general issue with the > EDK2 BaseTools where a transitive dependency on a library that has a > constructor is not taken into account when checking for cycles. > > For example, > > lib A depends on lib B, which has a constructor lib B depends on lib C, which > has no constructor lib C depends on lib D, which has a constructor lib D > depends on lib A, which has no constructor > > In this case, the build will happily succeed, but which constructor executes > first (lib B or lib D) is undefined, and there is generally no correct > solution as either constructor could transitively depend on the other library > having been constructed already. > > So I think it would be a good step to fix the build tools to detect this > case, but I am afraid it will expose errors in many places in the code base. > > I am kind of glad someone else is hitting this right now, since the last time > I brought it up, nobody really cared afair. > > Thanks, > Ard. > _______________________________________________ > edk2-devel mailing list > [email protected] > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
