Skip to content

Commit

Permalink
Merge branch 'feat/drop_root_level_commit_sha' into 'main'
Browse files Browse the repository at this point in the history
feat!: drop support of root level `commit_sha` in the manifest file

Closes PACMAN-927

See merge request espressif/idf-component-manager!418
  • Loading branch information
kumekay committed Aug 5, 2024
2 parents 41935b2 + 1d0d7c3 commit b298db1
Show file tree
Hide file tree
Showing 10 changed files with 57 additions and 41 deletions.
31 changes: 13 additions & 18 deletions docs/en/reference/manifest_file.rst
Original file line number Diff line number Diff line change
Expand Up @@ -231,37 +231,32 @@ Example:
repository: "https://example.com/component.git"
If your component is not in the root of the repository, specify the path to the component in the repository with the ``repository_info`` field.
``repository_info``
===================

The additional information of the repository.

This field is optional. But when it's set, ``repository`` field must be set as well.

If your component is not in the root of the repository, specify the path to the component in the ``path`` field.

.. code:: yaml
repository: "https://example.com/component.git"
repository_info:
commit_sha: "1234567890abcdef1234567890abcdef12345678"
path: "path/to/component"
``commit_sha``
==============

A SHA checksum for the Git commit of the component you intend to use.

This field is optional.

If used, place it under the ``repository_info`` field (recommended), or used along with the ``repository`` field.

Can be passed as an argument to the ``compote component upload --commit-sha [commit_sha]`` command.

Examples:

.. code:: yaml
commit_sha: "1234567890abcdef1234567890abcdef12345678"
You may also put a Git Commit SHA of the component you intend to use in the ``commit_sha`` field.

.. code:: yaml
repository_info:
commit_sha: "1234567890abcdef1234567890abcdef12345678"
Can be passed as an argument to the ``compote component upload --commit-sha [commit_sha]`` command.

Both ``path`` and ``commit_sha`` sub-fields are optional.

``documentation``
=================

Expand Down
9 changes: 8 additions & 1 deletion idf_component_manager/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -557,7 +557,14 @@ def upload_component(
tempdir = tempfile.mkdtemp()
try:
unpack_archive(archive, tempdir)
manifest = ManifestManager(tempdir, name, upload_mode=True).load()
manifest = ManifestManager(
tempdir,
name,
upload_mode=True,
repository=repository,
commit_sha=commit_sha,
repository_path=repository_path,
).load()
finally:
shutil.rmtree(tempdir)
else:
Expand Down
1 change: 0 additions & 1 deletion idf_component_tools/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ def validate(self) -> 'ManifestManager':
self._manifest.repository = self._repository

if self._commit_sha is not None:
self._manifest.commit_sha = self._commit_sha
self._manifest.repository_info = RepositoryInfoField.fromdict({
'commit_sha': self._commit_sha,
'path': self._repository_path,
Expand Down
2 changes: 1 addition & 1 deletion idf_component_tools/manifest/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
COMPILED_FULL_SLUG_REGEX = re.compile(FULL_SLUG_REGEX)
WEB_DEPENDENCY_REGEX = r'^((?:{slug}/{slug})|(?:{slug}))(.*)$'.format(slug=SLUG_BODY_REGEX)

LINKS = ['repository', 'commit_sha', 'documentation', 'issues', 'discussion', 'url']
LINKS = ['repository', 'documentation', 'issues', 'discussion', 'url']
KNOWN_INFO_METADATA_FIELDS = [
'maintainers',
'description',
Expand Down
7 changes: 3 additions & 4 deletions idf_component_tools/manifest/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,6 @@ class Manifest(BaseModel):
examples: t.List[t.Dict[str, t.Any]] = None # type: ignore
url: UrlField = None # type: ignore
repository: GIT_URL_FIELD = None # type: ignore
commit_sha: str = None # type: ignore
documentation: UrlField = None # type: ignore
issues: UrlField = None # type: ignore
discussion: UrlField = None # type: ignore
Expand Down Expand Up @@ -377,8 +376,8 @@ def validate_post_init(self) -> None:
raise ValueError(unknown_keys_errs)

# validate repository and commit sha
if not self.repository and self.commit_sha:
raise ValueError('Invalid field "repository". Must set when "commit_sha" is set')
if not self.repository and self.repository_info:
raise ValueError('Invalid field "repository". Must set when "repository_info" is set')

if self._upload_mode:
self._validate_while_uploading()
Expand Down Expand Up @@ -526,7 +525,7 @@ def metadata(self):

@property
def manifest_hash(self) -> str:
return hash_object(self.model_dump_json())
return hash_object(self.model_dump_json(exclude_unset=True))

@property
def repository_path(self) -> t.Optional[str]:
Expand Down
4 changes: 0 additions & 4 deletions idf_component_tools/sources/git.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,6 @@ def _checkout_git_source(
selected_paths=selected_paths,
)

@property
def component_hash_required(self) -> bool:
return True

@property
def downloadable(self) -> bool:
return True
Expand Down
4 changes: 0 additions & 4 deletions idf_component_tools/sources/web_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -202,10 +202,6 @@ def versions(self, name, spec='*', target=None):

return cmp_with_versions

@property
def component_hash_required(self) -> bool:
return True

@property
def downloadable(self) -> bool:
return True
Expand Down
2 changes: 1 addition & 1 deletion tests/fixtures/locks/dependencies.lock
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@ dependencies:
source:
type: idf
version: 4.4.4
manifest_hash: aedea05dfa3ce0abb1435f08e111503b8e4469ec2378069ea3cc7599bccce6f7
manifest_hash: 1ad192c00dd8498c7f6e4264cffdd7b32d0d8dc41d5f1c3674041bc7d54e0083
target: esp32
version: 2.0.0
2 changes: 1 addition & 1 deletion tests/test_component_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ def test_pack_component_with_replacing_manifest_params(tmp_path, release_compone

assert manifest.version == '2.3.5'
assert manifest.links.repository == repository_url
assert manifest.commit_sha == commit_id
assert manifest.repository_info.commit_sha == commit_id


def test_pack_component_with_examples(tmp_path, example_component_path):
Expand Down
36 changes: 30 additions & 6 deletions tests/test_manifest_common_cases.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD
# SPDX-License-Identifier: Apache-2.0
import typing as t
from copy import deepcopy

import pytest

Expand All @@ -19,7 +20,7 @@ def test_manifest_hash(valid_manifest):
manifest = Manifest.fromdict(valid_manifest)
# ONLY UPDATE MANIFEST HASH WHEN IT'S NECESSARY!!!
assert (
manifest.manifest_hash == '1bd6824f65d801ec3014b407dafde5e6cf6020f04f7bfee88b7e7d723da4bac0'
manifest.manifest_hash == '8dd1abf83989a97bcd7590b795f5436169f3a2d74a99c832cb97a2d3e8b44205'
)


Expand Down Expand Up @@ -111,17 +112,40 @@ def test_invalid_manifest(manifest, errors):
assert error in produced_errors


def test_validator_commit_sha_and_repo(valid_manifest):
valid_manifest['commit_sha'] = (
'252f10c83610ebca1a059c0bae8255eba2f95be4d1d7bcfa89d7248a82d9f111'
)
def test_validator_repo_info_and_repo(valid_manifest):
original_valid_manifest = deepcopy(valid_manifest)

valid_manifest['repository_info'] = {
'commit_sha': '252f10c83610ebca1a059c0bae8255eba2f95be4d1d7bcfa89d7248a82d9f111'
}
del valid_manifest['repository']
errors = Manifest.validate_manifest(valid_manifest)
assert errors == [
'Invalid field "repository". Must set when "repository_info" is set',
]

valid_manifest = deepcopy(original_valid_manifest)
valid_manifest['repository_info'] = {'path': 'foo/bar'}
del valid_manifest['repository']
errors = Manifest.validate_manifest(valid_manifest)
assert errors == [
'Invalid field "repository". Must set when "commit_sha" is set',
'Invalid field "repository". Must set when "repository_info" is set',
]

valid_manifest = deepcopy(original_valid_manifest)
valid_manifest['repository_info'] = {}
del valid_manifest['repository']
errors = Manifest.validate_manifest(valid_manifest)
assert errors == [
'Invalid field "repository". Must set when "repository_info" is set',
]

valid_manifest = deepcopy(original_valid_manifest)
valid_manifest.pop('repository_info', None)
del valid_manifest['repository']
errors = Manifest.validate_manifest(valid_manifest)
assert errors == []


@pytest.mark.parametrize(
'require_field,public,require',
Expand Down

0 comments on commit b298db1

Please sign in to comment.