[PATCH RFC 1/3] tools: binman: Enable getting file from specific directory

[Simon Glass]

Hi Neha,

On Thu, 27 Jul 2023 at 06:12, Neha Malcom Francis <n-francis at ti.com> wrote:
>
> While we have the option of using '-a <file-dir> to grab a file from a
> specific directory, this is problematic when searching for a filename
> shared by multiple files across directories. An example would be FDT
> generated in SPL_BUILD vs. non-SPL_BUILD that is present in spl/dts and
> arch/arm/dts respectively with the same DTB name. Maybe running binman

Do you mean spl/arch/arm/dts and arch/arch/dts ?

> twice could also solve this issue, but this seems like a valid
> implementation.
>
> Signed-off-by: Neha Malcom Francis <n-francis at ti.com>
> ---
>  tools/binman/etype/blob.py  | 12 ++++++++++--
>  tools/binman/etype/fit.py   | 15 ++++++++++++++-
>  tools/u_boot_pylib/tools.py |  7 ++++++-
>  3 files changed, 30 insertions(+), 4 deletions(-)
>

I don't fully understand this, but perhaps it would make sense to give
the indirs names, like:

binman -I spl:path/to/spl  -I name2:path/to/somewhere-else

then you could use the names below, instead of the paths. It provide
at least a little separation.

Perhaps if you listed out the two dirs and the files they contain,
that would help me understand better.

> diff --git a/tools/binman/etype/blob.py b/tools/binman/etype/blob.py
> index 064fae5036..eccf6f87ac 100644
> --- a/tools/binman/etype/blob.py
> +++ b/tools/binman/etype/blob.py
> @@ -19,6 +19,8 @@ class Entry_blob(Entry):
>
>      Properties / Entry arguments:
>          - filename: Filename of file to read into entry
> +        - indir-path: Specific directory to fetch file from, if not provided
> +        default indir paths will be searched
>          - compress: Compression algorithm to use:
>              none: No compression
>              lz4: Use lz4 compression (via 'lz4' command-line utility)
> @@ -35,6 +37,7 @@ class Entry_blob(Entry):
>          super().__init__(section, etype, node,
>                           auto_write_symbols=auto_write_symbols)
>          self._filename = fdt_util.GetString(self._node, 'filename', self.etype)
> +        self._indir_path = fdt_util.GetString(self._node, 'indir-path', None)
>          self.elf_fname = fdt_util.GetString(self._node, 'elf-filename',
>                                              self.elf_fname)
>          self.elf_base_sym = fdt_util.GetString(self._node, 'elf-base-sym')
> @@ -44,8 +47,13 @@ class Entry_blob(Entry):
>
>      def ObtainContents(self, fake_size=0):
>          self._filename = self.GetDefaultFilename()
> -        self._pathname = tools.get_input_filename(self._filename,
> -            self.external and (self.optional or self.section.GetAllowMissing()))
> +        if self._indir_path:
> +            self._pathname = tools.get_input_filename(self._filename,
> +                self.external and (self.optional or self.section.GetAllowMissing()),
> +                self._indir_path)

I think this might as well just read the file, rather than calling
tools.get_input_filename()

> +        else:
> +            self._pathname = tools.get_input_filename(self._filename,
> +                self.external and (self.optional or self.section.GetAllowMissing()))
>          # Allow the file to be missing
>          if not self._pathname:
>              self._pathname, faked = self.check_fake_fname(self._filename,
> diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py
> index ef4d066757..46db816370 100644
> --- a/tools/binman/etype/fit.py
> +++ b/tools/binman/etype/fit.py
> @@ -87,6 +87,15 @@ class Entry_fit(Entry_section):
>
>                  fit,fdt-list-val = "dtb1", "dtb2";
>
> +        fit,fdt-indir
> +            Indicates the specific directory (if any) to be used to pick up fdts
> +            generated in the gen-fdt-nodes property. For example, picking fdt from
> +            spl/dts instead of arch/arm/dts. This is equivalent to '-a spl/dts' but
> +            helps when multiple directories contain same named fdt to choose a
> +            specific one
> +
> +                fit,fdt-indir = "spl/dts"

Should this be a named indir? Otherwise you have to use the actual
dirs and that could be come quite brittle when things change.

> +
>      Substitutions
>      ~~~~~~~~~~~~~
>
> @@ -362,6 +371,7 @@ class Entry_fit(Entry_section):
>              if pname.startswith('fit,'):
>                  self._fit_props[pname] = prop
>          self._fit_list_prop = self._fit_props.get('fit,fdt-list')
> +        self._fit_indir = fdt_util.GetString(self._node, 'fit,fdt-indir', None)
>          if self._fit_list_prop:
>              fdts, = self.GetEntryArgsOrProps(
>                  [EntryArg(self._fit_list_prop.value, str)])
> @@ -583,7 +593,10 @@ class Entry_fit(Entry_section):
>                  # Generate nodes for each FDT
>                  for seq, fdt_fname in enumerate(self._fdts):
>                      node_name = node.name[1:].replace('SEQ', str(seq + 1))
> -                    fname = tools.get_input_filename(fdt_fname + '.dtb')
> +                    if self._fit_indir:
> +                        fname = tools.get_input_filename(fdt_fname + '.dtb', specific_indir=self._fit_indir)
> +                    else:
> +                        fname = tools.get_input_filename(fdt_fname + '.dtb')
>                      with fsw.add_node(node_name):
>                          for pname, prop in node.props.items():
>                              if pname == 'fit,firmware':
> diff --git a/tools/u_boot_pylib/tools.py b/tools/u_boot_pylib/tools.py
> index 187725b501..1b6f6b71d8 100644
> --- a/tools/u_boot_pylib/tools.py
> +++ b/tools/u_boot_pylib/tools.py
> @@ -123,12 +123,13 @@ def set_input_dirs(dirname):
>      indir = dirname
>      tout.debug("Using input directories %s" % indir)
>
> -def get_input_filename(fname, allow_missing=False):
> +def get_input_filename(fname, allow_missing=False, specific_indir=None):
>      """Return a filename for use as input.
>
>      Args:
>          fname: Filename to use for new file
>          allow_missing: True if the filename can be missing
> +        specific_indir: Directory to use to get file
>
>      Returns:
>          fname, if indir is None;
> @@ -140,6 +141,10 @@ def get_input_filename(fname, allow_missing=False):
>      """
>      if not indir or fname[:1] == '/':
>          return fname
> +    if specific_indir:
> +        pathname = os.path.join(specific_indir, fname)
> +        if os.path.exists(pathname):
> +            return pathname
>      for dirname in indir:
>          pathname = os.path.join(dirname, fname)
>          if os.path.exists(pathname):
> --
> 2.34.1
>

Regards,
Simon