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

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

[Heinrich Schuchardt]

Provide dictionary env__efi_fit_tftp_file describing the file used for the
UEFI FIT image test.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
Tested with
https://gitlab.denx.de/u-boot/custodians/u-boot-efi/pipelines/1714

v2:
	remove quotes around tftp_dir
---
 py/travis-ci/travis_tftp.py                   | 20 +++++++++++++++++++
 py/travis-ci/u_boot_boardenv_qemu_arm64_na.py |  1 +
 py/travis-ci/u_boot_boardenv_qemu_arm_na.py   |  1 +
 .../u_boot_boardenv_qemu_x86_64_na.py         |  2 ++
 py/travis-ci/u_boot_boardenv_qemu_x86_na.py   |  2 ++
 .../u_boot_boardenv_vexpress_ca15_tc2_qemu.py |  1 +
 .../u_boot_boardenv_vexpress_ca9x4_qemu.py    |  1 +
 7 files changed, 28 insertions(+)

diff --git a/py/travis-ci/travis_tftp.py b/py/travis-ci/travis_tftp.py
index 4ea5c06..0eb05bb 100644
--- a/py/travis-ci/travis_tftp.py
+++ b/py/travis-ci/travis_tftp.py
@@ -1,6 +1,26 @@
 import os
 import binascii

+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",
+        "dn": tftp_dir,
+    }
+    if addr is not None:
+        ret['addr'] = addr
+
+    return ret
+
 def file2env(file_name, addr=None):
     """Create dictionary describing file

diff --git a/py/travis-ci/u_boot_boardenv_qemu_arm64_na.py b/py/travis-ci/u_boot_boardenv_qemu_arm64_na.py
index 2986115..98ce873 100644
--- a/py/travis-ci/u_boot_boardenv_qemu_arm64_na.py
+++ b/py/travis-ci/u_boot_boardenv_qemu_arm64_na.py
@@ -6,3 +6,4 @@ env__net_dhcp_server = True
 env__net_tftp_readable_file = travis_tftp.file2env('u-boot.bin', 0x40400000)
 env__efi_loader_helloworld_file = travis_tftp.file2env('lib/efi_loader/helloworld.efi', 0x40400000)
 env__efi_loader_grub_file = travis_tftp.file2env('grub_arm64.efi', 0x40400000)
+env__efi_fit_tftp_file = travis_tftp.efifit2env(0x40400000)
diff --git a/py/travis-ci/u_boot_boardenv_qemu_arm_na.py b/py/travis-ci/u_boot_boardenv_qemu_arm_na.py
index 391e3c4..3dbedde 100644
--- a/py/travis-ci/u_boot_boardenv_qemu_arm_na.py
+++ b/py/travis-ci/u_boot_boardenv_qemu_arm_na.py
@@ -6,3 +6,4 @@ env__net_dhcp_server = True
 env__net_tftp_readable_file = travis_tftp.file2env('u-boot.bin', 0x40400000)
 env__efi_loader_helloworld_file = travis_tftp.file2env('lib/efi_loader/helloworld.efi', 0x40400000)
 env__efi_loader_grub_file = travis_tftp.file2env('grub_arm.efi', 0x40400000)
+env__efi_fit_tftp_file = travis_tftp.efifit2env(0x40400000)
diff --git a/py/travis-ci/u_boot_boardenv_qemu_x86_64_na.py b/py/travis-ci/u_boot_boardenv_qemu_x86_64_na.py
index 6f7c593..2fe72c8 100644
--- a/py/travis-ci/u_boot_boardenv_qemu_x86_64_na.py
+++ b/py/travis-ci/u_boot_boardenv_qemu_x86_64_na.py
@@ -8,3 +8,5 @@ env__efi_loader_helloworld_file = travis_tftp.file2env('lib/efi_loader/helloworl

 env__efi_loader_check_smbios = True
 env__efi_loader_grub_file = travis_tftp.file2env('grub_x64.efi')
+
+env__efi_fit_tftp_file = travis_tftp.efifit2env()
diff --git a/py/travis-ci/u_boot_boardenv_qemu_x86_na.py b/py/travis-ci/u_boot_boardenv_qemu_x86_na.py
index 70dc0ae..62cc279 100644
--- a/py/travis-ci/u_boot_boardenv_qemu_x86_na.py
+++ b/py/travis-ci/u_boot_boardenv_qemu_x86_na.py
@@ -8,3 +8,5 @@ env__efi_loader_helloworld_file = travis_tftp.file2env('lib/efi_loader/helloworl

 env__efi_loader_check_smbios = True
 env__efi_loader_grub_file = travis_tftp.file2env('grub_x86.efi')
+
+env__efi_fit_tftp_file = travis_tftp.efifit2env()
diff --git a/py/travis-ci/u_boot_boardenv_vexpress_ca15_tc2_qemu.py b/py/travis-ci/u_boot_boardenv_vexpress_ca15_tc2_qemu.py
index 7437ae6..75f287c 100644
--- a/py/travis-ci/u_boot_boardenv_vexpress_ca15_tc2_qemu.py
+++ b/py/travis-ci/u_boot_boardenv_vexpress_ca15_tc2_qemu.py
@@ -5,3 +5,4 @@ env__net_dhcp_server = True
 env__net_tftp_readable_file = travis_tftp.file2env('u-boot')
 env__efi_loader_helloworld_file = travis_tftp.file2env('lib/efi_loader/helloworld.efi')
 env__efi_loader_grub_file = travis_tftp.file2env('grub_arm.efi')
+env__efi_fit_tftp_file = travis_tftp.efifit2env()
diff --git a/py/travis-ci/u_boot_boardenv_vexpress_ca9x4_qemu.py b/py/travis-ci/u_boot_boardenv_vexpress_ca9x4_qemu.py
index 7437ae6..75f287c 100644
--- a/py/travis-ci/u_boot_boardenv_vexpress_ca9x4_qemu.py
+++ b/py/travis-ci/u_boot_boardenv_vexpress_ca9x4_qemu.py
@@ -5,3 +5,4 @@ env__net_dhcp_server = True
 env__net_tftp_readable_file = travis_tftp.file2env('u-boot')
 env__efi_loader_helloworld_file = travis_tftp.file2env('lib/efi_loader/helloworld.efi')
 env__efi_loader_grub_file = travis_tftp.file2env('grub_arm.efi')
+env__efi_fit_tftp_file = travis_tftp.efifit2env()
--
2.24.1

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

[Stephen Warren]

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.

> +        "dn": tftp_dir,
> +    }
> +    if addr is not None:
> +        ret['addr'] = addr
> +
> +    return ret

However, all this function does is create a static dictionary. There's 
no need to introduce a function for this. Instead, simply put the 
dictionary directly into py/travis-ci/u_boot_boardenv_qemu_arm64_na.py 
etc. The only reason that file2env() exists is to automatically 
calculate the crc32 value, which doesn't apply here.

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

[Heinrich Schuchardt]

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.

>> +        "dn": tftp_dir,
>> +    }
>> +    if addr is not None:
>> +        ret['addr'] = addr
>> +
>> +    return ret
>
> However, all this function does is create a static dictionary. There's
> no need to introduce a function for this. Instead, simply put the
> dictionary directly into py/travis-ci/u_boot_boardenv_qemu_arm64_na.py
> etc. The only reason that file2env() exists is to automatically
> calculate the crc32 value, which doesn't apply here.
>

I considered putting 6 lines into each board file:

import os

env__efi_fit_tftp_file = {
	"addr" = 0x4040000,
	"dn" = os.environ['UBOOT_TRAVIS_BUILD_DIR'],
	"fn" = "test-efi-fit.img",
}

I assumed it would be better idea to have a single line in the board files.

env__efi_fit_tftp_file = travis_tftp.efifit2env(0x40400000)

Do you really want to increase the code size?

Best regards

Heinrich

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

[Stephen Warren]

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.

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

[Heinrich Schuchardt]

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.

The tFTP directory "dn" is only known in uboot-test-hooks.git.

test/py/README.md says --persistent-data-dir is used for data that may
be re-used across test runs. Currently the FIT file is generated in
every run. In principal it could be reused. So Christian could consider
adjusting his patch.

Best regards

Heinrich

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

[Cristian Ciocaltea]

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.
 
> The tFTP directory "dn" is only known in uboot-test-hooks.git.

Yes, and this is actually the only mandatory information to be
provided in the dictionary when the test is supposed to generate the
FIT image. We could get rid of this, too, if we make the assumption
that the tFTP root directory is u_boot_console.config.build_dir by
default.

> test/py/README.md says --persistent-data-dir is used for data that may
> be re-used across test runs. Currently the FIT file is generated in
> every run. In principal it could be reused. So Christian could consider
> adjusting his patch.

Actually the FIT image file can be generated multiple times per test
run, with different content, which is determined by various combinations
of flags. E.g. for sandbox we have: enable/disable GZIP compression,
enable/disable FDT usage. Each combination is used in a subtest, that
(re)generates the corresponding FIT image.

To re-use the generated data we need to introduce additional complexity
to handle FIT image identification for each subtest and to invalidate
those images when they become out of sync with the parametrized input 
data samples (ITS, FDT) used to generate their content.

> Best regards
> 
> Heinrich

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

[Cristian Ciocaltea]

On Tue, Dec 31, 2019 at 12:42:36PM +0200, 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.
>  
> > The tFTP directory "dn" is only known in uboot-test-hooks.git.
> 
> Yes, and this is actually the only mandatory information to be
> provided in the dictionary when the test is supposed to generate the
> FIT image. We could get rid of this, too, if we make the assumption
> that the tFTP root directory is u_boot_console.config.build_dir by
> default.
> 
> > test/py/README.md says --persistent-data-dir is used for data that may
> > be re-used across test runs. Currently the FIT file is generated in
> > every run. In principal it could be reused. So Christian could consider
> > adjusting his patch.
> 
> Actually the FIT image file can be generated multiple times per test
> run, with different content, which is determined by various combinations
> of flags. E.g. for sandbox we have: enable/disable GZIP compression,
> enable/disable FDT usage. Each combination is used in a subtest, that
> (re)generates the corresponding FIT image.
> 
> To re-use the generated data we need to introduce additional complexity
> to handle FIT image identification for each subtest and to invalidate
> those images when they become out of sync with the parametrized input 
> data samples (ITS, FDT) used to generate their content.
> 
> > Best regards
> > 
> > Heinrich

I would propose the following:

def efifit2env(file_name=None, addr=None):
    """Create dictionary describing file for EFI fit image test

    @filename:  name of an existing file, optional
    @addr:      address used for loading the file as int (e.g. 0x40400000)
    Return:     dictionary describing the file with entries
                * dn    - tftp directory
                * fn    - filename, optional
                * size  - file size in bytes, optional
                * crc32 - checksum using CRC-32 algorithm, optional
                * addr  - loading address, optional
    """
    tftp_dir = os.environ['UBOOT_TRAVIS_BUILD_DIR']

    ret = {
        'dn': tftp_dir,
    }

    if addr is not None:
        ret['addr'] = addr

    if file_name:
        file_full = tftp_dir + '/' + file_name
        if os.path.isfile(file_full):
            ret['fn'] = file_name
            ret['size'] = os.path.getsize(file_full)
            with open(file_full, 'rb') as f:
                ret['crc32'] = hex(binascii.crc32(f.read()) & 0xffffffff)[2:]

    return ret

This allows the test to run in both scenarios described by Heinrich:
* the FIT image is available outside the test -> efifit2env() is called
  with file_name pointing to an existing FIT image file, optionally
  pass also the addr
* the FIT image is generated by the test itself -> efifit2env() is
  called without arguments, or optionally pass only the addr

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

[Stephen Warren]

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.

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.

[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