Re: LocalApicLib: Why two separate directories?

[Laszlo Ersek <lersek@redhat.com>]

On 10/28/16 21:28, Kinney, Michael D wrote:
> Leo,
> 
> Your observation is correct, but the reason not to make this change now is 
> the DSC file changes required that would break platform builds.  As Laszlo
> points out, it is possible to do this type of change and coordinate update
> to all platforms in edk2/master.  However, there are many other platforms 
> that use edk2 and a change like this would break them on next pull of 
> edk2/master.
> 
> I believe the original X2 APIC implementation did not have as much common
> code, so that was likely why it was added as a different library.
> 
> I recommend we just leave them in their own directories for right now.

It would also be possible to extract the common code into a library
*class* (internal to UefiCpuPkg), along with a single instance for that
new lib class. This would leave the two current INFs undisturbed.

However, as far as I understand, such arbitrary library classes could be
frowned upon -- there's no other "guiding principle" to them than "well
these functions happen to be shared by two other modules".

Sometimes that's a good enough reason for introducing a new library
class (with a single imlementation), sometimes it isn't.

Thanks
Laszlo

>> -----Original Message-----
>> From: Duran, Leo [mailto:leo.duran@amd.com]
>> Sent: Friday, October 28, 2016 12:04 PM
>> To: edk2-devel@ml01.01.org
>> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Fan, Jeff <jeff.fan@intel.com>;
>> Gao, Liming <liming.gao@intel.com>; 'Laszlo Ersek' <lersek@redhat.com>
>> Subject: LocalApicLib: Why two separate directories?
>>
>> All,
>> Just a quick observation to request comments:
>>
>> Since a lot of the code in BaseXApicX2ApicLib.c and BaseXApicLib is the same, how about
>> we merge the common code and build the libraries from the same directory?
>>
>> UefiCpuPkg/Library/LocalApilLib/
>> - LocalApicLib.c --> common code
>> - BaseXApicLib.c --> legacy APIC code
>> - BaseXApicX2ApicLib.c --> X2APIC code
>> - BaseXApicLib.inf -> builds from LocalApicLib.c + BaseXApicLib.c
>> - BaseXApicX2ApicLib.inf -> builds from LocalApicLib.c + BaseXApicX2ApicLib.c
>>
>> Of course, doing this would require modification to existing .DSC files, to point to
>> the appropriate .INF under the merged LocalApicLib directory.
>> Would that be too disruptive?
>>
>> Leo.
>>
>>
>>
>>
>