Add "lsource" for sourcing a list of files

[dobairoland]

Here is the PR I mentioned here: https://github.com/ulfalizer/Kconfiglib/issues/71#issuecomment-532184268

I hope it is worth of adding to your great library. I'm ready to make any necessary adjustments.

[dobairoland]

Thank you @ulfalizer for your suggestions and for the will to work toward this feature. I corrected the commit as per your suggestions.

It's a bit inconsistent in that there's no relative lsource, but at the same time I don't want to put in too much stuff that would never get used, so that might be fine.

Would you like more the idea that source, rsource and osource would accept a list as well (instead of adding lsource)?

[ulfalizer]

Thank you @ulfalizer for your suggestions and for the will to work toward this feature. I corrected the commit as per your suggestions.

Thanks!

Would you like more the idea that source, rsource and osource would accept a list as well (instead of adding lsource)?

One disadvantage is that it breaks paths with spaces in them. Probably very rare to have those, but you never know what people might depend on.

I'm fine with lsource for now. Even if I mark it provisional, I won't go changing stuff willy-nilly, because you probably shouldn't make people's lives too painful. In practice, it'll probably stay forever, unchanged. :)

Will look more over the weekend. Sorry for slowness. Should calm down soon.

[dobairoland]

One disadvantage is that it breaks paths with spaces in them. Probably very rare to have those, but you never know what people might depend on.

Yes, poor Windows users. :-) I forgot about those spaces. However, the list items could be separated with something else, e.g. semicolon.

Will look more over the weekend.

Thank you for your time!

[dobairoland]

I discovered an incompability with Python 2.

https://github.com/ulfalizer/Kconfiglib/blob/master/kconfiglib.py#L2472 token is unicode and not str, so it fails with kconfiglib.KconfigError: /home/dragon/esp/esp-idf/Kconfig:64: couldn't parse 'lsource "$COMPONENT_KCONFIGS_PROJBUILD"': expected string.

EDIT: if type(token) not in [type(b''), type(u'')] seems to fix the issue.

[ulfalizer]

I discovered an incompability with Python 2.

https://github.com/ulfalizer/Kconfiglib/blob/master/kconfiglib.py#L2472 token is unicode and not str, so it fails with kconfiglib.KconfigError: /home/dragon/esp/esp-idf/Kconfig:64: couldn't parse 'lsource "$COMPONENT_KCONFIGS_PROJBUILD"': expected string.

Weird. I wonder how you ended up with unicode there. The library deliberately only deals with plain undecoded str on Python 2, because unicode-ifying everything would get messy and make 2/3 compatibility harder.

Could you check if os.environ["COMPONENT_KCONFIGS_PROJBUILD"] is a unicode string? That would explain it in that case, though I'm not sure how it'd happen, unless you're manually setting it.

The recommended syntax nowadays is this btw:

lsource "$(COMPONENT_KCONFIGS_PROJBUILD)"

That's using the new Kconfig preprocessor. The ()-less syntax is deprecated and only supported for backwards compatibility.

[ulfalizer]

I wonder if lsource should be in _OBL_SOURCE_TOKENS. Allowing a list of zero files seems more flexible, especially if they might be generated by a preprocessor function.

It should really be lsource, olsource, rlsource, and orlsource I guess, but it feels like overkill, and the code footprint for that is pretty large.

@dobairoland Are all the components that can appear in https://github.com/espressif/esp-idf/blob/96b96ae244f360c84b71c8c3670e24a1186e9844/Kconfig#L67 already within the esp-idf repo by the way?

In that case, maybe you could do something like this instead:

if $(has-component,foo)
source "components/foo/Kconfig"
endif

if $(has-component,bar)
source "components/bar/Kconfig"
endif

...

has-component would be a Python preprocessor function (see https://github.com/ulfalizer/Kconfiglib/blob/f2ce282eca6537210d83f9f0d2c753c2b280943c/kconfiglib.py#L458). It could be defined like this, in kconfigfunctions.py:

import os

import kconfiglib

def has_component(kconf, _, name):
    components = os.getenv("ESP_IDF_COMPONENTS")
    if components is None:
        raise kconfiglib.KconfigError(
            "{}:{}: $(has-component) requires the env. var. "
            "ESP_IDF_COMPONENTS to be set to a space-separated list of "
            "component names".format(kconf.filename, kconf.linenr))

    return "y" if name in components.split() else "n"

functions = {
    "has-component": (has_component, 1, 1)
}

Thoughts?

It's a bit more work on the esp-idf side, but on the upside, it makes it easy to grep for where files get sourced. The preprocessor functionality is pretty flexible.

[dobairoland]

Hi @ulfalizer. Thank you for taking the time and looking at this again.

Could you check if os.environ["COMPONENT_KCONFIGS_PROJBUILD"] is a unicode string?

Yes, it is unicode. Unicode characters in paths are not supported in Kconfiglib?

I wonder if lsource should be in _OBL_SOURCE_TOKENS. Allowing a list of zero files seems more flexible, especially if they might be generated by a preprocessor function.

I can remove it from there if you prefer that solution.

It should really be lsource, olsource, rlsource, and orlsource I guess, but it feels like overkill, and the code footprint for that is pretty large.

Yes, that seems to be too much.

Are all the components that can appear in https://github.com/espressif/esp-idf/blob/96b96ae244f360c84b71c8c3670e24a1186e9844/Kconfig#L67 already within the esp-idf repo by the way?

Unfortunately, no they are not. There are other frameworks which depend on ESP-IDF which has their components. Our customers have their own private components. I cannot even compile a complete list.

I've tried to add $(source-list,"$(COMPONENT_KCONFIGS)") istead of the proposed lsource and created kconfigfunctions.py:

def source_list(kconf, name, path_list):
    return '\n'.join(['source "{}"'.format(path) for path in path_list[1:-1].split()])

functions = {
    "source-list": (source_list, 1, 1),
}

The function returns:

source "/home/xy/esp/esp-idf/components/app_update/Kconfig.projbuild"
source "/home/xy/esp/esp-idf/components/bootloader/Kconfig.projbuild"
...

But unfortunately, this isn't working.

[ulfalizer]

Could you check if os.environ["COMPONENT_KCONFIGS_PROJBUILD"] is a unicode string?

Yes, it is unicode. Unicode characters in paths are not supported in Kconfiglib?

They are supported in the sense that Kconfiglib just avoids decoding any strings on Python 2, and I haven't seen unicode strings show up in os.environ before. Even if you do this, it stays as a regular str:

$ foo=ßö python
>>> import os
>>> os.environ["foo"]
'\xc3\x9f\xc3\xb6'

That's why I'm wondering how you ended up with unicode. Manually setting os.environ entries to unicode strings currently isn't supported at least, if that's what's going on.

Curious how it happened, so I can see if it's worth fixing. :)

I wonder if lsource should be in _OBL_SOURCE_TOKENS. Allowing a list of zero files seems more flexible, especially if they might be generated by a preprocessor function.

I can remove it from there if you prefer that solution.

Might be best if there's only gone be one variant.

Unfortunately, no they are not. There are other frameworks which depend on ESP-IDF which has their components. Our customers have their own private components. I cannot even compile a complete list.

Yeah, that makes it a bit messier.

Another option might be to generate a Kconfig file with a bunch of sources in it, though it isn't super pretty either. Something like Kconfig.modules.

Any chance that might work?

I've tried to add $(source-list,"$(COMPONENT_KCONFIGS)") istead of the proposed lsource and created kconfigfunctions.py:

def source_list(kconf, name, path_list):
    return '\n'.join(['source "{}"'.format(path) for path in path_list[1:-1].split()])

functions = {
    "source-list": (source_list, 1, 1),
}

The function returns:

source "/home/xy/esp/esp-idf/components/app_update/Kconfig.projbuild"
source "/home/xy/esp/esp-idf/components/bootloader/Kconfig.projbuild"
...

But unfortunately, this isn't working.

Nah, the preprocessor works at the single token level, so that won't work. It was designed like that when it was added to the C tools to prevent too much anarchy, though it'd be semi-handy here.

[ulfalizer]

Sorry for stalling, but lsource is starting to feel a bit like an oddball feature that wouldn't get used very often. When I put things in, I have to support them forever, and it has to mesh with whatever gets put in in the future.

I'm a bit afraid that someone will come along in the future and say "hey, I need to source a bunch of glob patterns from an environment variable, why isn't this globbing like everything else?", or "why isn't there a relative (or required) version of this?" At that point, it's hard to come up with a good reason why not, even though it starts to get into messy feature creep. :)

Maybe generating a Kconfig.modules wouldn't be too bad of a hack, if it's just for one spot. Could osource it so that it's still possible to run Kconfig without modules if you need to. Kconfig.modules would be fully general too, and won't suffer from feature creep like lsource might.

FWIW, Kconfig.modules is what Zephyr does.

I know you just want to get it in and get on with the work, so sorry about that. I have to make it work with everything that comes after it though, and deal with issues people have with it. :)

[dobairoland]

That's why I'm wondering how you ended up with unicode. Manually setting os.environ entries to unicode strings currently isn't supported at least, if that's what's going on.

It is getting de-serialized from a JSON object: https://github.com/espressif/esp-idf/blob/master/tools/kconfig_new/confgen.py#L224 and json.load returns unicode.

Another option might be to generate a Kconfig file with a bunch of sources in it, though it isn't super pretty either. Something like Kconfig.modules.

Thank you, I'll try this option.

Sorry for stalling, but lsource is starting to feel a bit like an oddball feature that wouldn't get used very often. When I put things in, I have to support them forever, and it has to mesh with whatever gets put in in the future.

I understand your point. Thank you for your suggestions and your work on this library!

[ulfalizer]

It is getting de-serialized from a JSON object: https://github.com/espressif/esp-idf/blob/master/tools/kconfig_new/confgen.py#L224 and json.load returns unicode.

I'll look into it. Thanks for the report!

Found https://github.com/theskumar/python-dotenv/issues/176, though I can't find anything in the Python 2 os module docs that explicitly forbids it.

Do you still depend on Python 2?

[dobairoland]

Do you still depend on Python 2?

Yes, we have to support 2.7 in all of our active releases.

[ulfalizer]

Yes, we have to support 2.7 in all of our active releases.

Dug around a bit. I think it might be best to encode the strings on the esp-idf side before storing them into os.environ:

• mypy --py2 foo.py prints

  foo.py:3: error: Incompatible types in assignment (expression has type "unicode", target has type "str")

  for this script:

  import os
  os.environ["foo"] = u"bar"

  mypy is an official Python thing, so might be a sign it's not intended. It supports allowing more than one type too.

• Seems unicode values in os.environ can cause issues with subprocess on Windows (which Kconfiglib uses for $(shell), though that one's tricky to use portably anyway): https://github.com/theskumar/python-dotenv/pull/101

Maybe doing something similar to what this PR does in ESP-IDF would make sense.

[dobairoland]

I solved my issue by creating a temporary file with a list of source item.

Thank you for your help and kind suggestions.