abderrahim commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r2066481854


##########
src/buildstream/source.py:
##########
@@ -293,8 +299,9 @@ def fetch(self, alias_override: Optional[AliasSubstitution] 
= None, **kwargs) ->
 
         Args:
            alias_override: The alias to use instead of the default one
-               defined by the :ref:`aliases <project_source_aliases>` field
-               in the project's config.
+               defined by the :ref:`aliases <project_source_aliases>` field in 
the
+               project's config. If provided, it must be used in when calling

Review Comment:
   nit: I think you want either "used" or "passed in".



##########
src/buildstream/source.py:
##########
@@ -272,6 +420,208 @@ class AliasSubstitution:
     _mirror: Union[SourceMirror, str]
 
 
+class SourceInfoMedium(FastEnum):
+    """
+    Indicates the medium in which the source is obtained
+
+    *Since: 2.5*
+    """
+
+    WORKSPACE = "workspace"
+    """
+    Files in an open workspace
+    """
+
+    LOCAL = "local"
+    """
+    Files stored locally in the project
+    """
+
+    REMOTE_FILE = "remote-file"
+    """
+    A remote file
+    """
+
+    GIT = "git"
+    """
+    A git repository
+    """
+
+    BAZAAR = "bzr"
+    """
+    The Bazaar revision control system
+    """
+
+    OCI_IMAGE = "oci-image"
+    """
+    An OCI image, such as docker or podman images.
+    """
+
+    PYTHON_PACKAGE_INDEX = "pypi"
+    """
+    A python package obtained from a python package index like https://pypi.org

Review Comment:
   Is there a reason to treat this differently from `remote-file`?



##########
src/buildstream/source.py:
##########
@@ -272,6 +420,208 @@ class AliasSubstitution:
     _mirror: Union[SourceMirror, str]
 
 
+class SourceInfoMedium(FastEnum):
+    """
+    Indicates the medium in which the source is obtained
+
+    *Since: 2.5*
+    """
+
+    WORKSPACE = "workspace"
+    """
+    Files in an open workspace
+    """
+
+    LOCAL = "local"
+    """
+    Files stored locally in the project
+    """
+
+    REMOTE_FILE = "remote-file"
+    """
+    A remote file
+    """
+
+    GIT = "git"
+    """
+    A git repository
+    """
+
+    BAZAAR = "bzr"
+    """
+    The Bazaar revision control system
+    """
+
+    OCI_IMAGE = "oci-image"
+    """
+    An OCI image, such as docker or podman images.
+    """
+
+    PYTHON_PACKAGE_INDEX = "pypi"
+    """
+    A python package obtained from a python package index like https://pypi.org
+    """
+
+
+class SourceVersionType(FastEnum):
+    """
+    Indicates the type of the version string
+
+    *Since: 2.5*
+    """
+
+    COMMIT = "commit"
+    """
+    A commit string which accurately represents a version in a source
+    code repository or VCS
+    """
+
+    SHA256 = "sha256"
+    """
+    An sha256 checksum
+    """
+
+    DIGEST = "digest"
+    """
+    A digest representing the unique version of this source input.
+    """
+
+    INDEXED_VERSION = "indexed-version"
+    """
+    This type of version is used in cases where we have repositories which
+    have an interface to index content by version, and that no additional 
validation
+    is performed to insure the uniqueness of the downloaded content (not 
recommended).
+
+    In the case of plugins which use this version type, it is probable that
+    ``SourceInfo.guess_version == SourceInfo.version``.
+    """
+
+
+class SourceInfo:
+    """SourceInfo()
+
+    An object representing the provenance of input reported by
+    :func:`Source.collect_source_info() 
<buildstream.source.Source.collect_source_info>`
+    and/or :func:`SourceFetcher.get_source_info() 
<buildstream.source.SourceFetcher.get_source_info>`
+
+    See: :ref:`documentation on generating SourceInfo <core_source_info>`.
+
+    .. attention::
+
+       A given SourceInfo for a given element is **not** guaranteed to be 
unique for
+       a given :ref:`cache key <cachekeys>`.
+
+       While given well behaved source plugins, it is expected to obtain the 
same *cache keys*
+       for a project where all of the SourceInfo for all elements are exactly 
the same, the
+       inverse is not to be expected.

Review Comment:
   Is this only referring to the urls below? Maybe this could be reformulated 
to make it clearer? (or maybe dropping the first sentence would help)



##########
src/buildstream/source.py:
##########
@@ -272,6 +420,208 @@ class AliasSubstitution:
     _mirror: Union[SourceMirror, str]
 
 
+class SourceInfoMedium(FastEnum):
+    """
+    Indicates the medium in which the source is obtained
+
+    *Since: 2.5*
+    """
+
+    WORKSPACE = "workspace"
+    """
+    Files in an open workspace
+    """
+
+    LOCAL = "local"
+    """
+    Files stored locally in the project
+    """
+
+    REMOTE_FILE = "remote-file"
+    """
+    A remote file
+    """
+
+    GIT = "git"
+    """
+    A git repository
+    """
+
+    BAZAAR = "bzr"
+    """
+    The Bazaar revision control system
+    """
+
+    OCI_IMAGE = "oci-image"
+    """
+    An OCI image, such as docker or podman images.
+    """
+
+    PYTHON_PACKAGE_INDEX = "pypi"
+    """
+    A python package obtained from a python package index like https://pypi.org
+    """
+
+
+class SourceVersionType(FastEnum):
+    """
+    Indicates the type of the version string
+
+    *Since: 2.5*
+    """
+
+    COMMIT = "commit"
+    """
+    A commit string which accurately represents a version in a source
+    code repository or VCS
+    """
+
+    SHA256 = "sha256"
+    """
+    An sha256 checksum
+    """
+
+    DIGEST = "digest"
+    """
+    A digest representing the unique version of this source input.
+    """
+
+    INDEXED_VERSION = "indexed-version"
+    """
+    This type of version is used in cases where we have repositories which
+    have an interface to index content by version, and that no additional 
validation
+    is performed to insure the uniqueness of the downloaded content (not 
recommended).
+
+    In the case of plugins which use this version type, it is probable that
+    ``SourceInfo.guess_version == SourceInfo.version``.

Review Comment:
   This should be `SourceInfo.version_guess`



##########
tests/frontend/source-info/plugins/extradata.py:
##########
@@ -0,0 +1,44 @@
+from buildstream import Source
+
+
+class ExtraData(Source):
+    BST_MIN_VERSION = "2.0"
+
+    def configure(self, node):
+        pass
+
+    def preflight(self):
+        pass
+
+    def get_unique_key(self):
+        return {}
+
+    def load_ref(self, node):
+        pass
+
+    def get_ref(self):
+        return {}
+
+    def set_ref(self, ref, node):
+        pass
+
+    def is_cached(self):
+        return False
+
+    def collect_source_info(self):
+        return [
+            self.create_source_info(
+                "http://ponyfarm.com/ponies";,
+                "pony-ride",
+                "pony-age",
+                "1234567",
+                version_guess="12",
+                extra_data={"pony": "green"},
+            )
+        ]
+
+
+# Plugin entry point
+def setup():
+

Review Comment:
   nit: drop this newline? (here and in the other sources)



##########
src/buildstream/source.py:
##########
@@ -272,6 +420,208 @@ class AliasSubstitution:
     _mirror: Union[SourceMirror, str]
 
 
+class SourceInfoMedium(FastEnum):
+    """
+    Indicates the medium in which the source is obtained
+
+    *Since: 2.5*
+    """
+
+    WORKSPACE = "workspace"
+    """
+    Files in an open workspace
+    """
+
+    LOCAL = "local"
+    """
+    Files stored locally in the project
+    """
+
+    REMOTE_FILE = "remote-file"
+    """
+    A remote file
+    """
+
+    GIT = "git"
+    """
+    A git repository
+    """
+
+    BAZAAR = "bzr"
+    """
+    The Bazaar revision control system
+    """
+
+    OCI_IMAGE = "oci-image"
+    """
+    An OCI image, such as docker or podman images.
+    """
+
+    PYTHON_PACKAGE_INDEX = "pypi"
+    """
+    A python package obtained from a python package index like https://pypi.org
+    """
+
+
+class SourceVersionType(FastEnum):
+    """
+    Indicates the type of the version string
+
+    *Since: 2.5*
+    """
+
+    COMMIT = "commit"
+    """
+    A commit string which accurately represents a version in a source
+    code repository or VCS
+    """
+
+    SHA256 = "sha256"
+    """
+    An sha256 checksum
+    """
+
+    DIGEST = "digest"
+    """
+    A digest representing the unique version of this source input.
+    """
+
+    INDEXED_VERSION = "indexed-version"
+    """
+    This type of version is used in cases where we have repositories which
+    have an interface to index content by version, and that no additional 
validation
+    is performed to insure the uniqueness of the downloaded content (not 
recommended).
+
+    In the case of plugins which use this version type, it is probable that
+    ``SourceInfo.guess_version == SourceInfo.version``.
+    """
+
+
+class SourceInfo:
+    """SourceInfo()
+
+    An object representing the provenance of input reported by
+    :func:`Source.collect_source_info() 
<buildstream.source.Source.collect_source_info>`
+    and/or :func:`SourceFetcher.get_source_info() 
<buildstream.source.SourceFetcher.get_source_info>`
+
+    See: :ref:`documentation on generating SourceInfo <core_source_info>`.
+
+    .. attention::
+
+       A given SourceInfo for a given element is **not** guaranteed to be 
unique for
+       a given :ref:`cache key <cachekeys>`.
+
+       While given well behaved source plugins, it is expected to obtain the 
same *cache keys*

Review Comment:
   I don't think this is a reasonable expectation. There are other things that 
could influence cache keys besides the source infos. I don't think it's 
reasonable to expect all the source plugin configuration to end up in the 
source info.



##########
src/buildstream/source.py:
##########
@@ -311,6 +661,25 @@ def fetch(self, alias_override: 
Optional[AliasSubstitution] = None, **kwargs) ->
         """
         raise ImplError("SourceFetcher '{}' does not implement 
fetch()".format(type(self)))
 
+    def get_source_info(self) -> SourceInfo:
+        """Get the :class:`.SourceInfo` object describing this source
+
+        This method should only be called whenever
+        :func:`Source.is_resolved() <buildstream.source.Source.is_resolved>`
+        returns ``True``.
+
+        SourceInfo objects created by implementors should be created with
+        :func:`Source.is_resolved() 
<buildstream.source.Source.create_source_info>`.

Review Comment:
   copy-paste error? (s/is_resolved/create_source_info/)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to