[PATCH v2 1/1] travis-ci: provide env__efi_fit_tftp_file

[Heinrich Schuchardt]

On 1/2/20 6:05 PM, Stephen Warren wrote:
> On 12/31/19 3:42 AM, Cristian Ciocaltea wrote:
>> On Mon, Dec 30, 2019 at 09:03:38PM +0100, Heinrich Schuchardt wrote:
>>> On 12/30/19 8:32 PM, Stephen Warren wrote:
>>>> On 12/30/19 12:05 PM, Heinrich Schuchardt wrote:
>>>>> On 12/30/19 5:38 PM, Stephen Warren wrote:
>>>>>> On 12/30/19 3:52 AM, Heinrich Schuchardt wrote:
>>>>>>> Provide dictionary env__efi_fit_tftp_file describing the file
>>>>>>> used for
>>>>>>> the
>>>>>>> UEFI FIT image test.
>>>>>>
>>>>>>> diff --git a/py/travis-ci/travis_tftp.py
>>>>>>> b/py/travis-ci/travis_tftp.py
>>>>>>
>>>>>>> +def efifit2env(addr=None):
>>>>>>> +    """Create dictionary describing file for EFI fit image test
>>>>>>> +
>>>>>>> +    @addr:      address used for loading the file as int (e.g.
>>>>>>> 0x40400000)
>>>>>>> +    Return:     dictionary describing the file with entries
>>>>>>> +                * fn    - filename
>>>>>>> +                * addr  - loading address, optional
>>>>>>> +                * dn    - tftp directory
>>>>>>> +    """
>>>>>>> +    tftp_dir = os.environ['UBOOT_TRAVIS_BUILD_DIR']
>>>>>>> +
>>>>>>> +    ret = {
>>>>>>> +        "fn": "test-efi-fit.img",
>>>>>>
>>>>>> If this function were to exist, then the filename shouldn't be
>>>>>> hard-coded; it should be a parameter.
>>>>>>
>>>>>
>>>>> Hello Stephen,
>>>>>
>>>>> thanks for reviewing.
>>>>>
>>>>> This is the name of a generated file. It does not depend on the board.
>>>>
>>>> What generates the file and when/why?
>>>>
>>>> Generated files should generally be put into
>>>> u_boot_console.config.persistent_data_dir, and presumably the name
>>>> hard-coded into the test that uses it.
>>>>
>>>
>>> Hello Stephen,
>>>
>>> this is the test case:
>>>
>>> https://lists.denx.de/pipermail/u-boot/2019-December/394957.html
>>> test/py: Create a test for launching UEFI binaries from FIT images
>>>
>>> The test can be run in different styles:
>>>
>>> * A complete FIT image can be supplied. In this case the dictionary
>>>    must contain a "size" entry.
>>> * The test can generate a FIT image from lib/efi_loader/helloworld.efi.
>>>    In this case no "size" entry shall be supplied. The "fn" field
>>>    provides the name of the generated file. The file is generated in
>>>    cons.config.build_dir. The "dn" field" describes the tFTP root
>>>    directory to which the generated file is copied.
>>
>> A small correction here: if the "size" entry is not provided in the
>> dictionary, the test generates a FIT image using a hardcoded file
>> name (test-efi-fit-helloworld.fit), so any "fn" entry provided in the
>> dictionary is ignored in this case.
>
> Why does the size parameter affect whether the file name parameter is
> used? This test sounds like it's doing very odd things with
> configuration...
>
> I'd propose removing the size and filename parameters completely, and
> instead just telling the test where the TFTP directory is. Then, we
> don't need any common function at all.

We will need addr too.

I was wrong in assuming a filename was needed. If size is not provided
to the test, it defaults to filename 'test-efi-fit-helloworld.fit'.

>
> Also, if we do keep a function, then "efifit2env" is a really bad name;
> this function is just generating some configuration data, not converting
> anything, hence "2" or "to" in the function name doesn't make sense.
>

I will update the patch accordingly. Thanks for reviewing.

Best regards

Heinrich