search

espressif / idf-component-manager

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

Fix non-semantic version for git dependencies (PACMAN-301) #5

Closed FrancescoVaiani closed 2 years ago

FrancescoVaiani commented 2 years ago

Summary

If a git dependency is declared in an idf_component.yml with a version not formatted as major.minor.patch[-prerelease] (where major, minor and patch are integer numbers) a ValueError: Invalid version string is returned by the semantic_version package.

It's very common for git projects to have tags starting with v or release branches starting with release- and this error makes it difficult to refer to a specific branch or tag.

Because the semantic-version.Version call is used only to evaluate if the given version is semantic, the function should be surrounded by a try / except and move on if the given version is not in the semantic format.

Impact

The current PR skips the blocking error generated if the version of a git dependency is not in the semantic format, making it possible to refer to a specific branch or tag as implied by git_client.py:159.

kumekay commented 2 years ago

Hi @FrancescoVaiani thank you for your PR, could you please share an example of an idf_component.yml manifest file, where you faced the issue?

So far I tried a manifest like:

dependencies:
  nghttp:
    version: "feature/ci/run_test_app"
    git: https://github.com/espressif/idf-extra-components.git
    path: "nghttp"

But wasn't able to reproduce the issue

FrancescoVaiani commented 2 years ago

Hi @kumekay, sure, here is the idf_component.py with the issue.

dependencies:
  # Required IDF version
  idf: ">=4.1"
  astarte-device-sdk-esp32:
    version: "release-1.0"
    git: https://github.com/astarte-platform/astarte-device-sdk-esp32.git

The weird thing is that I have the same issue with your example too.

CMake Error at [path_to_my_home]/Desktop/esp-idf/tools/cmake/project.cmake:209 (message):
  Traceback (most recent call last):

    File "[path_to_my_home]\.espressif\python_env\idf4.3_py3.8_env\lib\site-packages\idf_component_manager\version_solver\helper.py", line 21, in parse_constraint
      clause = SimpleSpec(spec).clause
    File "[path_to_my_home]\.espressif\python_env\idf4.3_py3.8_env\lib\site-packages\semantic_version\base.py", line 618, in __init__
      self.clause = self._parse_to_clause(expression)
    File "[path_to_my_home]\.espressif\python_env\idf4.3_py3.8_env\lib\site-packages\semantic_version\base.py", line 1014, in _parse_to_clause
      return cls.Parser.parse(expression)
    File "[path_to_my_home]\.espressif\python_env\idf4.3_py3.8_env\lib\site-packages\semantic_version\base.py", line 1034, in parse
      raise ValueError("Invalid simple block %r" % block)

  ValueError: Invalid simple block 'feature/ci/run_test_app'

  During handling of the above exception, another exception occurred:

  Traceback (most recent call last):

    File "runpy.py", line 194, in _run_module_as_main
    File "runpy.py", line 87, in _run_code
    File "[path_to_my_home]\.espressif\python_env\idf4.3_py3.8_env\lib\site-packages\idf_component_manager\prepare_components\__main__.py", line 3, in <module>
      main()
    File "[path_to_my_home]\.espressif\python_env\idf4.3_py3.8_env\lib\site-packages\idf_component_manager\prepare_components\prepare.py", line 110, in main
      args.func(args)
    File "[path_to_my_home]\.espressif\python_env\idf4.3_py3.8_env\lib\site-packages\idf_component_manager\prepare_components\prepare.py", line 37, in prepare_dep_dirs
      ComponentManager(args.project_dir).prepare_dep_dirs(
    File "[path_to_my_home]\.espressif\python_env\idf4.3_py3.8_env\lib\site-packages\idf_component_manager\core.py", line 325, in prepare_dep_dirs
      downloaded_component_paths = download_project_dependencies(
    File "[path_to_my_home]\.espressif\python_env\idf4.3_py3.8_env\lib\site-packages\idf_component_manager\dependencies.py", line 36, in download_project_dependencies
      solution = solver.solve()
    File "[path_to_my_home]\.espressif\python_env\idf4.3_py3.8_env\lib\site-packages\idf_component_manager\version_solver\version_solver.py", line 28, in solve
      self.solve_manifest(manifest)
    File "[path_to_my_home]\.espressif\python_env\idf4.3_py3.8_env\lib\site-packages\idf_component_manager\version_solver\version_solver.py", line 47, in solve_manifest
      self.solve_component(requirement)
    File "[path_to_my_home]\.espressif\python_env\idf4.3_py3.8_env\lib\site-packages\idf_component_manager\version_solver\version_solver.py", line 54, in solve_component
      self._source.add(
    File "[path_to_my_home]\.espressif\python_env\idf4.3_py3.8_env\lib\site-packages\idf_component_manager\version_solver\helper.py", line 93, in add
      dependencies.append(Dependency(dep_package, spec))
    File "[path_to_my_home]\.espressif\python_env\idf4.3_py3.8_env\lib\site-packages\idf_component_manager\version_solver\helper.py", line 58, in __init__
      self.constraint = parse_constraint(spec)
    File "[path_to_my_home]\.espressif\python_env\idf4.3_py3.8_env\lib\site-packages\idf_component_manager\version_solver\helper.py", line 23, in parse_constraint
      constraint = parse_single_constraint(HashedComponentVersion(spec))
    File "[path_to_my_home]\.espressif\python_env\idf4.3_py3.8_env\lib\site-packages\idf_component_tools\manifest\manifest.py", line 228, in __init__
      super(HashedComponentVersion, self).__init__(*args, **kwargs)
    File "[path_to_my_home]\.espressif\python_env\idf4.3_py3.8_env\lib\site-packages\idf_component_tools\manifest\manifest.py", line 187, in __init__
      self._semver = semver.Version(self._version_string)
    File "[path_to_my_home]\.espressif\python_env\idf4.3_py3.8_env\lib\site-packages\semantic_version\base.py", line 105, in __init__
      major, minor, patch, prerelease, build = self.parse(version_string, partial)
    File "[path_to_my_home]\.espressif\python_env\idf4.3_py3.8_env\lib\site-packages\semantic_version\base.py", line 311, in parse
      raise ValueError('Invalid version string: %r' % version_string)

  ValueError: Invalid version string: 'feature/ci/run_test_app'

Call Stack (most recent call first):
  [path_to_my_home]/Desktop/esp-idf/tools/cmake/project.cmake:296 (__project_init)
  CMakeLists.txt:11 (project)

-- Configuring incomplete, errors occurred!

Could it be an idf-component-manager version issue?

pip show idf-component-manager
Name: idf-component-manager
Version: 1.0.1
Summary: The component manager for ESP-IDF
Home-page: https://github.com/espressif/idf-component-manager
Author: Sergei Silnov
Author-email: sergei.silnov@esspressif.com
License: Apache License 2.0
Location: c:\users\udooer\.espressif\python_env\idf4.3_py3.8_env\lib\site-packages
Requires: tqdm, requests, pyyaml, six, future, requests-toolbelt, semantic-version, schema
Required-by:
kumekay commented 2 years ago

@FrancescoVaiani Thank you for the logs, again your manifest works fine for me with the idf-component-manager 1.0.1 from pypi and IDF 4.3

Solving dependencies requirements
Updating lock file at /home/user/test_project/dependencies.lock
Processing 2 dependencies:
[1/2] astarte-device-sdk-esp32 (b47e453777f774835b3f86eeb0ce5d3b3a890c9c)
[2/2] idf (4.3.2)

I'll take one more look tomorrow to better understand the issue,

You are on Windows, right?

FrancescoVaiani commented 2 years ago

Hi @kumekay, we found the issue both on Windows 10 and on Ubuntu 20.04, both with IDF 4.3 and with idf-component-manager 1.0.1 downloaded from pip last week.

If it can be of any help, pip downloads semantic-version 2.9.0 as a dependency.

pip show semantic-version
Name: semantic-version
Version: 2.9.0
Summary: A library implementing the 'SemVer' scheme.
Home-page: https://github.com/rbarrois/python-semanticversion
Author: Raphaël Barrois
Author-email: raphael.barrois+semver@polytechnique.org
License: BSD
Location: c:\users\udooer\.espressif\python_env\idf4.3_py3.8_env\lib\site-packages
Requires:
Required-by: idf-component-manager

Looking in semantic-version code it looks like that it tests the version code against this regular expression ^(\d+)\.(\d+)\.(\d+)(?:-([0-9a-zA-Z.-]+))?(?:\+([0-9a-zA-Z.-]+))?$ (semantic_version/base.py:310) and, because "release-1.0.1" doesn't match, it raises the ValueError exception that looks like no one catches or suppress.

Let me know if I can be of further help.

hfudev commented 2 years ago

@FrancescoVaiani Hi, I still can't reproduce your issue with the following environment:

could you provide these info and I could know your case better?

hfudev commented 2 years ago

Looking in semantic-version code it looks like that it tests the version code against this regular expression ^(\d+).(\d+).(\d+)(?:-([0-9a-zA-Z.-]+))?(?:+([0-9a-zA-Z.-]+))?$ (semantic_version/base.py:310) and, because "release-1.0.1" doesn't match, it raises the ValueError exception that looks like no one catches or suppress.

we're using a different method for git sources. branches/tags/commit_sha should be all supported.

FrancescoVaiani commented 2 years ago

@hfudev Hi, I have the same exact configuration as yours except for the operating system, so that can not be the source of the issue.

we're using a different method for git sources. branches/tags/commit_sha should be all supported.

Ok, I'm glad to hear that it was not supposed to do that, but, as you can see in my stack trace, it is exactly what the code is doing for me. It crashes running self._semver = semver.Version(self._version_string) in manifest.py in __init__ and it raises an exception because self._version_string is not matching the regex I mentioned.

The only other thing I can think of is that the idf_component.yml that rase the issue is not the one in the main project, but is in a component whose in turn is a dependency of the idf_component of the main project. Maybe being the dependency of a dependency is what makes the code go rogue?

hfudev commented 2 years ago

@FrancescoVaiani

Maybe being the dependency of a dependency is what makes the code go rogue?

Yes you're right. I can reproduce now. I'll debug on it. Thanks for reporting :)

FrancescoVaiani commented 2 years ago

@hfudev Great! Glad to hear we found the issue!

hfudev commented 2 years ago

Hi @FrancescoVaiani, this issue should be fixed with https://github.com/espressif/idf-component-manager/commit/ded8c291846752607ce711e37ace2bf2f5aae012

kumekay commented 2 years ago

@FrancescoVaiani I'm closing this PR since we merged an alternative solution to the issue, if you have any issues with the solution, feel free to reopen or create a new ticket.

Thank you for your contribution!