espressif / idf-component-manager

Tool for installing ESP-IDF components
https://components.espressif.com/
Apache License 2.0
42 stars 15 forks source link

Using environment variables in idf_component.yml causes error (IDFGH-12061) (PACMAN-799) #53

Open ammaree opened 4 months ago

ammaree commented 4 months ago

Answers checklist.

IDF version.

v5.3-dev-1922-g5454d37d49

Operating System used.

macOS

How did you build your project?

VS Code IDE

If you are using Windows, please specify command line type.

None

What is the expected behavior?

The rules test is supposed to work if I am reading the documentation correctly?

What is the actual behavior?

When building the application, 4 errors are generated right at the start.

https://docs.espressif.com/projects/idf-component-manager/en/latest/reference/manifest_file.html#environment-variables-in-manifest specifies that environment variables can be used in values, not sure if that implies or excludes values used in conditional rules.

Steps to reproduce.

Example idf_component.yml:

  idf:
    version: ">=5.2.0"
  espressif/esp_lvgl_port:
    version: ">=1.4.0"
  led_strip:
    version: ">=2.5.3"
  jsmn:
    version: ">=1.1.0"
  joltwallet/littlefs:
    version: ">=1.14.0"
  lvgl/lvgl:
    version: ">=1.4.0"
    path: ../../../z-repo/lvgl
  espressif/esp_wrover_kit:
    version: ">=1.5.0~2"
    rules:
      - if: target == esp32 && ${cmakeMODEL} in [ dk41 ]

Build or installation Logs.

CMake Error at /Users/andremaree/DevSpace/z-sdk/esp-idf.v5x/tools/cmake/build.cmake:544 (message):
  ERROR: 4 problems were found in the manifest file
  /Users/andremaree/DevSpace/z-projects/irmacs/main/idf_component.yml:

  Invalid manifest format

  Invalid dependency format

  Dependency version spec format is invalid

  Invalid if clause format "ac00 in [ dk41 ]".  You can specify rules based
  on current ESP-IDF version or target like: "idf_version >=3.3,<5.0" or
  "target in [esp32, esp32c3]"

  Documentation:
  https://docs.espressif.com/projects/idf-component-manager/en/latest/reference/manifest_file.html#rules

More Information.

No response

kumekay commented 4 months ago

Hello @ammaree Thank you for opening the issue,

The documentation doesn't say it clearly enough, but the if clause only supports idf_version and target in the left of comparison.

Since we introduced support for environment variables in the manifest, we may remove this limitation, but I cannot give an exact timeline right now.

As an ugly workaround, maybe you can “misuse” idf_version in the rule. (Please don't tell anyone I suggested this)

something like:

...
    rules:
      - if: target == esp32 && idf_target ${cmakeMODEL_esp_wrover_kit} 1.0.0

And set cmakeMODEL_esp_wrover_kit to > or <. You also need to write some logic in the project level CMakeLists.txt to define this environment variable. This is definitely not a nice solution, but I cannot come up with anything better right now.

ammaree commented 4 months ago

@kumekay

Thanks for the feedback, have made the changes you suggested and it works.

Challenge now is that our options for dynamically selecting components are VERY limited using this mechanisms. We currently have 4 different components of which 1 or sometimes 2 need to be included based on the hardware platform and library selected, so no way we can solve the problem effectively using this.

Any idea what can be done to try and speed up the process for broader support of environment variables in the manifest?

It will make a huge difference to our ability to optimise the build process, and remove some complexity.....

kumekay commented 4 months ago

@ammaree We had an internal discussion, and we will include this feature in the coming major release next month. Then the syntax like && ${cmakeMODEL} in [ dk41 ] should work.

But unfortunately, I don't have a better solution right now

ammaree commented 4 months ago

@kumekay

Thanks for the feedback. If the patch is reasonably simple you are welcome to send me the patch to try prior to official release.

Working without the functionality continues to be a big pain so happy to be your guinea pig.

kumekay commented 4 months ago

@ammaree Sure, we will mention it in this issue as soon as it is available

ammaree commented 2 months ago

@kumekay

Any update on this functionality, please .......

kumekay commented 2 months ago

Hello @ammaree To make this possible (and fix a few other issues) we are refactoring the whole processing of manifest files, so the change turned out to be massive. However, it's close to being done, you may expect it on GitHub next week.

ammaree commented 2 months ago

Thanks, I will be monitoring this thread for good news...

kumekay commented 2 months ago

@ammaree I apologize, but the path wasn't merged yet, I'll keep you updated.

ammaree commented 2 months ago

Please, sooner the better…

On 24/04/22, 10:52, "Sergei Silnov" @.**@.>> wrote:

@ammareehttps://github.com/ammaree I apologize, but the path wasn't merged yet, I'll keep you updated.

— Reply to this email directly, view it on GitHubhttps://github.com/espressif/idf-component-manager/issues/53#issuecomment-2068848800, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAYT7GAODZ3VFGEGGGO22HLY6TFV7AVCNFSM6AAAAABC4YGVYCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANRYHA2DQOBQGA. You are receiving this because you were mentioned.Message ID: @.***>

kumekay commented 1 month ago

@ammaree

Current main can support this https://github.com/espressif/idf-component-manager?tab=readme-ov-file#installing-a-development-version-of-the-component-manager

Right now you need to use the commit before the version bump, so ESP-IDF dependency checker won't complain: https://github.com/espressif/idf-component-manager/commit/b543e18dd76df29d51a9a481928519ff3a031951

ammaree commented 1 week ago

Any update on the status here? Is it still limited to the development version or has it been included in the latest IDF updates?

ammaree commented 1 week ago

Just saw the answer in the release notes, v1.5.3 released 2 days ago but this feature held back for v2.0.0

We have great reluctance to use beta versions in production development thus would be forced to suffer development pain whilst waiting for the official v2.0.0 release.

Please, please, please....

kumekay commented 1 week ago

@ammaree All the features planned for 2.0 are merged, but a few bugs need to be fixed before the release. It's our priority now.