Re: [edk2-devel] [PATCH v1 1/1] BaseTools/Conf: Remove C-Code-File.AcpiTable rule

["Andrew Fish" <afish@apple.com>]

[-- Attachment #1: Type: text/plain, Size: 5300 bytes --]



> On Jun 25, 2020, at 3:44 AM, PierreGondois <pierre.gondois@arm.com> wrote:
> 
> Hello Laszlo,
> I couldn't find any trace of a ".act" file being created/deleted/modified in edk2 or edk2-platforms. I guess the whole f0373115cc9c commit could be reverted.
> f0373115cc9c commit was authored by "lgao4". Maybe Liming will know more about it.
> 

Pierre,

I’ve got non open source platforms that use *.act file extension. 

My guess is ?.act was in use for Adaptive Differential Pulse Code Modulation compressed audio files and maybe it was inconvenient, so ?.aslc got added. Seems like at some point in the past .act audio files were popular in China so maybe clicking on a *.act file launches an audio player vs. a text editor. 

In general my preference would be to not change supported <InputFile> extensions as that could break other projects that depend on the edk2 build system. Given [C-Code-File.AcpiTable] is unreachable (and does not support Xcode :) ) it would make sense to remove that. 

My guess is ?.aslc got added and the edk2 got cleaned up, but other projects probably based themselves on older projects that used ?.act files and copied them over and modified them, but never got around to updating the extension?

Thanks,

Andrew Fish


> Regards,
> Pierre
> 
> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com <mailto:lersek@redhat.com>> 
> Sent: Thursday, June 25, 2020 11:01 AM
> To: devel@edk2.groups.io <mailto:devel@edk2.groups.io>; Pierre Gondois <Pierre.Gondois@arm.com <mailto:Pierre.Gondois@arm.com>>
> Cc: bob.c.feng@intel.com <mailto:bob.c.feng@intel.com>; liming.gao@intel.com <mailto:liming.gao@intel.com>; nd <nd@arm.com <mailto:nd@arm.com>>
> Subject: Re: [edk2-devel] [PATCH v1 1/1] BaseTools/Conf: Remove C-Code-File.AcpiTable rule
> 
> On 06/25/20 10:19, PierreGondois wrote:
>> From: Pierre Gondois <pierre.gondois@arm.com>
>> 
>> When trying to build a module with the MODULE_TYPE set to:
>>  MODULE_TYPE = AcpiTable
>> 
>> the build fails with the following error:
>> [ModulePath].inf(-1): error 3001: MODULE_TYPE AcpiTable is not 
>> supported for EDK II, valid values are:
>> BASE SEC PEI_CORE PEIM DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER
>> DXE_SAL_DRIVER DXE_SMM_DRIVER UEFI_DRIVER UEFI_APPLICATION
>> USER_DEFINED HOST_APPLICATION SMM_CORE MM_STANDALONE
>> MM_CORE_STANDALONE
>> 
>> Indeed, the "SUP_MODULE_LIST" variable available in 
>> edk2/BaseTools/Source/Python/Common/DataType.py does not include any 
>> "AcpiTable".
>> The "C-Code-File.AcpiTable" rule described in 
>> edk2/BaseTools/Conf/build_rule.template might be outdated.
>> Plus there is no mention of "AcpiTable" module in the INF 
>> specification.
>> 
>> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
>> ---
>> 
>> The changes can be seen at: 
>> https://github.com/PierreARM/edk2/commits/831_Remove_C_AcpiTable_build
>> _rule_v1
>> 
>> Notes:
>>    v1:
>>     - Remove AcpiTable rule. [Pierre]
>> 
>> BaseTools/Conf/build_rule.template | 20 --------------------
>> 1 file changed, 20 deletions(-)
>> 
>> diff --git a/BaseTools/Conf/build_rule.template 
>> b/BaseTools/Conf/build_rule.template
>> index 
>> 0822b681fcd9f61c6508e6f93ffc31fa70fd7059..32984eb3f33fd9d110da54975fc9
>> 0325addb8ecc 100755
>> --- a/BaseTools/Conf/build_rule.template
>> +++ b/BaseTools/Conf/build_rule.template
>> @@ -437,26 +437,6 @@
>>         "$(ASL)" $(ASL_FLAGS) $(ASL_OUTFLAGS)${dst} $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iiii
>>         -AmlToHex $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.aml
>> 
>> -[C-Code-File.AcpiTable]
>> -    <InputFile>
>> -        ?.c
>> -
>> -    <OutputFile>
>> -        $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.acpi
>> -
>> -    <ExtraDependency>
>> -        $(MAKE_FILE)
>> -
>> -    <Command.MSFT, Command.INTEL>
>> -        "$(ASLCC)" $(DEPS_FLAGS) /Fo$(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj $(ASLCC_FLAGS) $(DEPS_FLAGS) $(INC) ${src}
>> -        "$(ASLDLINK)" /OUT:$(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.dll $(ASLDLINK_FLAGS) $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj
>> -        "$(GENFW)" -o ${dst} -c $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.dll $(GENFW_FLAGS)
>> -
>> -    <Command.GCC>
>> -        "$(ASLCC)" $(DEPS_FLAGS) -c -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj $(CC_FLAGS) $(ASLCC_FLAGS) $(DEPS_FLAGS) $(INC) ${src}
>> -        "$(ASLDLINK)" -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.dll $(ASLDLINK_FLAGS) $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj
>> -        "$(GENFW)" -o ${dst} -c $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.dll $(GENFW_FLAGS)
>> -
>> [Acpi-Table-Code-File]
>>     <InputFile>
>>         ?.aslc, ?.act
>> 
> 
> Should we remove "?.act" from under "Acpi-Table-Code-File", too?
> 
> Because, if you run "git-blame" on the file, the code being removed comes originally from commit f0373115cc9c ("Add the special build rule for AcpiTable C Source file.", 2008-09-01). And that commit added "?.act" under "Acpi-Table-Code-File".
> 
> (I don't know if the "?.act" addition in that commit was due to bad patch structuring (i.e., if it was unrelated to AcpiTable modules, and should have been a separate patch), or somehow indeed related to AcpiTable modules. The commit doesn't explain.)
> 
> Thanks
> Laszlo
> 
> 
> 


[-- Attachment #2: Type: text/html, Size: 28432 bytes --]