You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@buildstream.apache.org by "gtristan (via GitHub)" <gi...@apache.org> on 2025/03/04 07:31:14 UTC
[PR] Add new abstract methods for reporting source provenance [buildstream]
gtristan opened a new pull request, #1997:
URL: https://github.com/apache/buildstream/pull/1997
This is an initial implementation for the proposal up at: https://lists.apache.org/thread/q6gxjpld2vb1c9rqlsv24m12c087snc4
Some thoughts about this approach:
* This prioritizes machine readability and standardization of source provenance and version information
Sources have a lot of freedom in how they implement things, and so we may very well need to expand on the types and constants added here, such as `SourceInfoMedium`, `SourceVersionType`, etc.
The idea here is to have greater certainty about how sources are obtained, even if this cannot be covered by all currently existing source implementations (e.g. I didn't initially add a `bzr` medium for which we have a plugin, or a `cvs` medium for which we do not yet have a plugin).
Aspirationally, forcing this data to be precise can allow adjacent tooling to do useful things.
* This drops the freeform "public data" mentioned in the proposal discussion
My rationale for this choice in this branch, is that ultimately we want a data with a constant shape, and if for examlpe, we want the user to be able to override or assist a source with determining the reported *"version"*, then the `Source` implementation already has everything it needs to do so:
* it can add additional configuration keys for the user to configure
* it has the power to implement `collect_source_info()` however it wants.
* This does not yet attempt to cover the concept of *tracking* information
I would like to consider this, but we should think carefully about how this can be useful. For instance, some git plugins have different interpretations of what their "tracking" strings mean, sometimes following a branch head, sometimes looking for the latest tag in history which matches a given regular expression.
If we export this tracking information, it should probably be useful for external tooling to figure out how to do the tracking and come to the same conclusion, otherwise it is unclear what this is useful for.
* This does not cover the CVE information
While the SourceInfo objects representing a source's provenance is a list, I believe that the CVE information continues to be a per-element concept.
For example, when we have applied security patches to a module, those security patches are, themselves, sources, with provenance of being revisioned in the local project
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add new abstract methods for reporting source provenance [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r2011780986
##########
src/buildstream/source.py:
##########
@@ -262,10 +262,123 @@ def __init__(
@dataclass
class AliasSubstitution:
+ """AliasSubstitution()
+ An opaque data structure which may be passed through
+ :func:`SourceFetcher.fetch() <buildstream.source.SourceFetcher.fetch>` and in such cases
+ must be provided to :func:`Source.translate_url() <buildstream.source.Source.translate_url>`.
+ """
+
_effective_alias: str
_mirror: Union[SourceMirror, str]
+class SourceInfoMedium(FastEnum):
+ """
+ Indicates the meduim in which the source is obtained
+
+ *Since: 2.5*
+ """
+
+ LOCAL = "local"
+ """
+ Files stored locally in the project
+ """
+
+ ARCHIVE = "archive"
+ """
+ An archive file
+ """
+
+ GIT = "git"
+ """
+ A git repository
+ """
+
+
+class SourceVersionType(FastEnum):
+ """
+ Indicates the type of the version string
+
+ *Since: 2.5*
+ """
+
+ VERSION = "version"
+ """
+ The upstream version string, which may be semantic version
+ """
+
+ 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 CAS digest representing the unique version of this source input
+ """
+
+
+class SourceInfo:
+ """SourceInfo()
+
+ An object representing the provenance of input reported by
+ :func:`Source.collect_source_info() <buildstream.source.Source.collect_source_info>`
+
+ *Since: 2.5*
+ """
+
+ def __init__(self, url: str, medium: str, version_type: str, version: str):
+ # XXX assert medium and version_type are valid values for the enums
+
+ self.url: str = url
+ """
+ The url of the source input
+ """
+
+ self.medium: str = medium
+ """
+ The :class:`.SourceInfoMedium` of the source input
+ """
+
+ self.version_type: str = version_type
Review Comment:
> In general, I feel that it's way easier to determine the version of a tarball than it is to extract the version from a git tag. The tarball naming is just more consistent.
On the face of it, this *appears* to be true.
Here is my thoughts about trust, though - with a git repo, I have a higher level of *trust* that a given release tag means that the content I am getting *is that actual release*.
With a tarball on the other hand, I get an sha256sum and I get a filename. That filename has numbers in it.
Yes it's true that by convention, for a staggering majority of tarballs out there in the wild, the number in the filename is somehow related to the *version* of software you'll get in the tarball, for a CLI tool you might see that number when you type `cli --version`.
In order to *produce version information* that is reliable, I'm not sure it can be completely automated, perhaps at least a human being, when adding the `.bst` files to a project or when updating those, needs to at least sign off on the *way that we extract the version*.
What I'm getting at, is that overall, I'm not comfortable with any statement like:
> *"This plugin knows how to get the version of this upstream data blob"*
We just don't *really* know what the version is at any time, not without at least looking at the upstream project, reading about its release policy and figuring it out.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WIP: Add new abstract methods for reporting source provenance [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r2055226450
##########
src/buildstream/plugins/sources/local.py:
##########
@@ -110,6 +110,9 @@ def init_workspace_directory(self, directory):
#
self.__do_stage(directory)
+ def collect_source_info(self):
+ return [SourceInfo(self.path, SourceInfoMedium.LOCAL, SourceVersionType.DIGEST, self.get_unique_key())]
Review Comment:
This is interesting upon closer inspection.
It seems that the internal `Directory._get_size()` abstract method is completely unused, while the `_artifact.py` class does *observe* this in `Artifact.cache()`, it does to in a completely meaningless way, in order to satisfy a local `size` variable which is later unused.
This whole thing leads me down an irrelevant trail of unused deadcode, only to confirm that *indeed*, buildstream needs to download a directory digest and crawl the merkle tree without having any foreknowledge of the size of the entire merkle tree to be downloaded... finally observing the `remote_execution.proto` `message Digest` proto definition tells me which size would be appropriate to report in a `{hash}/{size}` version definition.
In conclusion, I will use the `size_bytes` member of the `Digest` proto, rather than asking the `CasBasedDirectory._get_size()` to tell me what "size" to put there.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WIP: Add new abstract methods for reporting source provenance [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r2055283247
##########
src/buildstream/source.py:
##########
@@ -262,10 +262,123 @@ def __init__(
@dataclass
class AliasSubstitution:
+ """AliasSubstitution()
+ An opaque data structure which may be passed through
+ :func:`SourceFetcher.fetch() <buildstream.source.SourceFetcher.fetch>` and in such cases
+ must be provided to :func:`Source.translate_url() <buildstream.source.Source.translate_url>`.
+ """
+
_effective_alias: str
_mirror: Union[SourceMirror, str]
+class SourceInfoMedium(FastEnum):
+ """
+ Indicates the meduim in which the source is obtained
+
+ *Since: 2.5*
+ """
+
+ LOCAL = "local"
+ """
+ Files stored locally in the project
+ """
+
+ ARCHIVE = "archive"
+ """
+ An archive file
+ """
+
+ GIT = "git"
+ """
+ A git repository
+ """
+
+
+class SourceVersionType(FastEnum):
+ """
+ Indicates the type of the version string
+
+ *Since: 2.5*
+ """
+
+ VERSION = "version"
+ """
+ The upstream version string, which may be semantic version
+ """
+
+ 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 CAS digest representing the unique version of this source input
+ """
+
+
+class SourceInfo:
+ """SourceInfo()
+
+ An object representing the provenance of input reported by
+ :func:`Source.collect_source_info() <buildstream.source.Source.collect_source_info>`
+
+ *Since: 2.5*
+ """
+
+ def __init__(self, url: str, medium: str, version_type: str, version: str):
+ # XXX assert medium and version_type are valid values for the enums
+
+ self.url: str = url
+ """
+ The url of the source input
+ """
+
+ self.medium: str = medium
+ """
+ The :class:`.SourceInfoMedium` of the source input
+ """
+
+ self.version_type: str = version_type
Review Comment:
I'm going to leave this open as a reminder that there is STILL a missing part from this patch, which is adding support for specifying explicit versions in existing plugins in yaml.
For instance, if I have a git tag or a tarball with a name like `pony-rider-fork-with-gallop-support.tgz` on some hosting, where they did not specify a meaningful version, but the version is expressed in some other way, e.g. in the project readme, or is listed on a main page and indexed with some sha like `https://flying-ponies.com/releases/5a9121c44e888f7c5cd7060dfb153e0e9f2f2e95/pony-release.tgz`, it cannot be matched or guessed with any pattern regex.
In this case we need `downloadablefilesource.tgz` and any plugins to allow some source level configuration to explicitly specify these user provided overrides, for that plugin to prioritize and use in their `Source.collect_source_info()` implementations.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add new abstract methods for reporting source provenance [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r1995830507
##########
src/buildstream/source.py:
##########
@@ -262,10 +262,123 @@ def __init__(
@dataclass
class AliasSubstitution:
+ """AliasSubstitution()
+ An opaque data structure which may be passed through
+ :func:`SourceFetcher.fetch() <buildstream.source.SourceFetcher.fetch>` and in such cases
+ must be provided to :func:`Source.translate_url() <buildstream.source.Source.translate_url>`.
+ """
+
_effective_alias: str
_mirror: Union[SourceMirror, str]
+class SourceInfoMedium(FastEnum):
+ """
+ Indicates the meduim in which the source is obtained
+
+ *Since: 2.5*
+ """
+
+ LOCAL = "local"
+ """
+ Files stored locally in the project
+ """
+
+ ARCHIVE = "archive"
Review Comment:
Interesting, I consider the *medium* type as the *means in which the data is stored*, and not what the downloaded data *actually is*.
I.e. the medium here explains that it comes from the project itself, while a git medium used a `git` protocol to download, and `archive` downloaded a file.
I rather lean the other way here and think that `ARCHIVE` may be renamed to `REMOTE_FILE` or such, and this would make sense for any `DownloadableFileSource`.
When considering this, I'm trying to balance:
* What is useful for interoperability with external tooling
* What is needed to logically describe the provenance of the source, for reporting purposes
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WIP: Add new abstract methods for reporting source provenance [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r2059968317
##########
src/buildstream/downloadablefilesource.py:
##########
@@ -270,6 +270,13 @@ def fetch(self): # pylint: disable=arguments-differ
"File downloaded from {} has sha256sum '{}', not '{}'!".format(self.url, sha256, self.ref)
)
+ def collect_source_info(self):
+ #
+ # XXX remote sources are not necessarily archives, perhaps we should
+ # allow downloadablefilesource imlementations to choose the SourceInfoMedium
+ #
+ return [SourceInfo(self.url, SourceInfoMedium.ARCHIVE, SourceVersionType.SHA256, self.ref)]
Review Comment:
This is now accounted for in the branch.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add new abstract methods for reporting source provenance [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan commented on PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#issuecomment-2830010589
This is now ready, documentation and testing and all, along with https://github.com/apache/buildstream-plugins/pull/87 which is also ready to the same degree.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add new abstract methods for reporting source provenance [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r2020101150
##########
src/buildstream/source.py:
##########
@@ -262,10 +262,123 @@ def __init__(
@dataclass
class AliasSubstitution:
+ """AliasSubstitution()
+ An opaque data structure which may be passed through
+ :func:`SourceFetcher.fetch() <buildstream.source.SourceFetcher.fetch>` and in such cases
+ must be provided to :func:`Source.translate_url() <buildstream.source.Source.translate_url>`.
+ """
+
_effective_alias: str
_mirror: Union[SourceMirror, str]
+class SourceInfoMedium(FastEnum):
+ """
+ Indicates the meduim in which the source is obtained
+
+ *Since: 2.5*
+ """
+
+ LOCAL = "local"
+ """
+ Files stored locally in the project
+ """
+
+ ARCHIVE = "archive"
Review Comment:
I've now renamed this to be `REMOTE_FILE` which is used by default by `DownloadableFileSource`, and is, in my opinion factually correct for any remotely downloaded file, including possibly compressed archives.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WIP: Add new abstract methods for reporting source provenance [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r2041857031
##########
src/buildstream/source.py:
##########
@@ -262,10 +261,147 @@ def __init__(
@dataclass
class AliasSubstitution:
+ """AliasSubstitution()
+ An opaque data structure which may be passed through
+ :func:`SourceFetcher.fetch() <buildstream.source.SourceFetcher.fetch>` and in such cases
+ must be provided to :func:`Source.translate_url() <buildstream.source.Source.translate_url>`.
+ """
+
_effective_alias: str
_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
+ """
+
+
+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
Review Comment:
> Should we comment something about git specifically? In particular, our plugins can use either the commit sha1, or a the long version of the git describe.
>
> I think the current description fits both, which I think is a good thing. But maybe it's better to give an example.
Interesting. I think that for the git plugins which do this, we can either include the whole git describe string or we can have the plugins parse out the commit sha only, while parsing out other things into the *guess_version*.
I think that *if* the git describe string cannot be directly used in every way a commit sha can be used, it would be preferable to be more strict here and have the plugins parse out the pure commit sha from the ref and report that for the version of type `SourceVersionType.COMMIT`.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WIP: Add new abstract methods for reporting source provenance [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan commented on PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#issuecomment-2820635130
[...]
>
> Do you think there is any reason to have 2 different strings (one for the medium and one for the version) ? Or would it be sufficient to have a single string to define how a given `SourceInfo` instance behaves ?
I came to the following conclusions:
* I liked that we had strong types, it helps how things are documented
* I liked that we had this separation of "where it comes from" and "what the version means", as stated before on this issue, this reduces the need for a lot of redundant enum values
* We do still have a need for third party plugin developers to use freeform strings, as discussed last week
I've updated the branch to use `Union[SourceVersionType, str]` and `Union[SourceInfoMedium, str]` types and reverted to the original implementation with this change, this seems to be a happy compromise.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WIP: Add new abstract methods for reporting source provenance [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r2055251041
##########
src/buildstream/plugins/sources/local.py:
##########
@@ -110,6 +110,9 @@ def init_workspace_directory(self, directory):
#
self.__do_stage(directory)
+ def collect_source_info(self):
+ return [SourceInfo(self.path, SourceInfoMedium.LOCAL, SourceVersionType.DIGEST, self.get_unique_key())]
Review Comment:
Fixed.
##########
src/buildstream/plugins/sources/workspace.py:
##########
@@ -108,6 +108,9 @@ def stage_directory(self, directory):
with self._cache_directory(digest=self.__digest) as cached_directory:
directory._import_files_internal(cached_directory, collect_result=False)
+ def collect_source_info(self):
+ return [SourceInfo(self.path, SourceInfoMedium.WORKSPACE, SourceVersionType.DIGEST, self.get_unique_key())]
Review Comment:
Fixed.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add new abstract methods for reporting source provenance [buildstream]
Posted by "abderrahim (via GitHub)" <gi...@apache.org>.
abderrahim commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r2072638802
##########
pyproject.toml:
##########
@@ -14,7 +14,7 @@
[build-system]
requires = [
# We need at least version 36.6.0 that introduced "build_meta"
- "setuptools>=36.6.0",
+ "setuptools>=36.6.0,<80",
Review Comment:
Yeah, I have a fix for this. I'll open a separate pull request.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add new abstract methods for reporting source provenance [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r2020173848
##########
src/buildstream/source.py:
##########
@@ -262,10 +262,123 @@ def __init__(
@dataclass
class AliasSubstitution:
+ """AliasSubstitution()
+ An opaque data structure which may be passed through
+ :func:`SourceFetcher.fetch() <buildstream.source.SourceFetcher.fetch>` and in such cases
+ must be provided to :func:`Source.translate_url() <buildstream.source.Source.translate_url>`.
+ """
+
_effective_alias: str
_mirror: Union[SourceMirror, str]
+class SourceInfoMedium(FastEnum):
+ """
+ Indicates the meduim in which the source is obtained
+
+ *Since: 2.5*
+ """
+
+ LOCAL = "local"
+ """
+ Files stored locally in the project
+ """
+
+ ARCHIVE = "archive"
+ """
+ An archive file
+ """
+
+ GIT = "git"
+ """
+ A git repository
+ """
+
+
+class SourceVersionType(FastEnum):
+ """
+ Indicates the type of the version string
+
+ *Since: 2.5*
+ """
+
+ VERSION = "version"
+ """
+ The upstream version string, which may be semantic version
+ """
+
+ 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 CAS digest representing the unique version of this source input
+ """
+
+
+class SourceInfo:
+ """SourceInfo()
+
+ An object representing the provenance of input reported by
+ :func:`Source.collect_source_info() <buildstream.source.Source.collect_source_info>`
+
+ *Since: 2.5*
+ """
+
+ def __init__(self, url: str, medium: str, version_type: str, version: str):
+ # XXX assert medium and version_type are valid values for the enums
+
+ self.url: str = url
+ """
+ The url of the source input
+ """
+
+ self.medium: str = medium
+ """
+ The :class:`.SourceInfoMedium` of the source input
+ """
+
+ self.version_type: str = version_type
Review Comment:
I've updated this patches now to make a compromise here.
* We keep the *specific* versioning, which can be an sha256sum or a git hash or anything which can *accurately* describe a unique source version.
* We add a `SourceInfo.version_guess` member to the data type. This is implemented by plugins who implement it, for the `DownloadableFileSource`, there is a default implementation which does the regex thing found in the collect_manifest code, and an added `version-guess-pattern` configure key allowing the user to specify the regex which should be used in order to make a guess in determining the version.
I think essentially this gives us the same functionality, however I'm just more comfortable with calling this a *guess*, and avoid any misinterpretation of the promises BuildStream is making here.
Essentially, really *knowing* a version requires some human understanding of the upstream project, what their release policy is, and what, if anything, their release tags or git branches mean in relation to that.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add new abstract methods for reporting source provenance [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r2072328939
##########
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've updated this entire statement, changing especially the first part and leaving the rest almost completely the same.
I think this is now a more accurate description.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add new abstract methods for reporting source provenance [buildstream]
Posted by "nanonyme (via GitHub)" <gi...@apache.org>.
nanonyme commented on PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#issuecomment-2727308204
We have been having the challenge with bst-to-lorry that we're missing a way for source to explicitly state relevant refspecs for git. Heuristically computed once are commonly incomplete or even wrong. So if there's API, it really should take this into account. The problem is that commonly in git repositories there are branches that need to be ignored (like pull request branches) since they are routinely force pushed to. Also with git_module it's common that upstream doesn't declare at all what the branch is from which commit originated.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WIP: Add new abstract methods for reporting source provenance [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r2055344809
##########
src/buildstream/source.py:
##########
@@ -262,10 +262,123 @@ def __init__(
@dataclass
class AliasSubstitution:
+ """AliasSubstitution()
+ An opaque data structure which may be passed through
+ :func:`SourceFetcher.fetch() <buildstream.source.SourceFetcher.fetch>` and in such cases
+ must be provided to :func:`Source.translate_url() <buildstream.source.Source.translate_url>`.
+ """
+
_effective_alias: str
_mirror: Union[SourceMirror, str]
+class SourceInfoMedium(FastEnum):
+ """
+ Indicates the meduim in which the source is obtained
+
+ *Since: 2.5*
+ """
+
+ LOCAL = "local"
+ """
+ Files stored locally in the project
+ """
+
+ ARCHIVE = "archive"
+ """
+ An archive file
+ """
+
+ GIT = "git"
+ """
+ A git repository
+ """
+
+
+class SourceVersionType(FastEnum):
+ """
+ Indicates the type of the version string
+
+ *Since: 2.5*
+ """
+
+ VERSION = "version"
+ """
+ The upstream version string, which may be semantic version
+ """
+
+ 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 CAS digest representing the unique version of this source input
+ """
+
+
+class SourceInfo:
+ """SourceInfo()
+
+ An object representing the provenance of input reported by
+ :func:`Source.collect_source_info() <buildstream.source.Source.collect_source_info>`
+
+ *Since: 2.5*
+ """
+
+ def __init__(self, url: str, medium: str, version_type: str, version: str):
+ # XXX assert medium and version_type are valid values for the enums
+
+ self.url: str = url
+ """
+ The url of the source input
+ """
+
+ self.medium: str = medium
+ """
+ The :class:`.SourceInfoMedium` of the source input
+ """
+
+ self.version_type: str = version_type
Review Comment:
I've now rectified this last part in downloadablefilesource.
Will rectify this in the external plugins where this applies.
I think this is unnecessary for the automated plugins such as pip and cargo, since *those* plugins obtain their various second order URIs as a *consequence* of already knowing a version, as such there is no sense in having an explicit override.
I'll close this thread now.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add new abstract methods for reporting source provenance [buildstream]
Posted by "abderrahim (via GitHub)" <gi...@apache.org>.
abderrahim commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r2072642659
##########
pyproject.toml:
##########
@@ -14,7 +14,7 @@
[build-system]
requires = [
# We need at least version 36.6.0 that introduced "build_meta"
- "setuptools>=36.6.0",
+ "setuptools>=36.6.0,<80",
Review Comment:
https://github.com/apache/buildstream/pull/2007
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WIP: Add new abstract methods for reporting source provenance [buildstream]
Posted by "abderrahim (via GitHub)" <gi...@apache.org>.
abderrahim commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r2037693646
##########
src/buildstream/plugins/sources/local.py:
##########
@@ -110,6 +110,9 @@ def init_workspace_directory(self, directory):
#
self.__do_stage(directory)
+ def collect_source_info(self):
+ return [SourceInfo(self.path, SourceInfoMedium.LOCAL, SourceVersionType.DIGEST, self.get_unique_key())]
Review Comment:
`get_unique_key()` only uses the hash of the digest, which isn't enough to get the content from CAS.
Buildbox uses the format `{hash}/{size}`, which is what we're planning to use in #1994 and #1991. We should use it here too. This would allow downloading the source from CAS using e.g. `casdownload` from buildbox
##########
src/buildstream/downloadablefilesource.py:
##########
@@ -25,6 +25,46 @@
implementation.
+Built-in functionality
+----------------------
+The DownloadableFileSource class provides built in keys which can be set when
+intantiating any Source which derives from DownloadableFileSource
+
+* Guess pattern
+
+ This ``version-guess-pattern`` sets the regular expression which will be used to attempt
+ to guess the version of a source when parsing the source's URI.
+
+ The DownloadableFileSource provides a default implementation of
+ :func:`Source.collect_source_info() <buildstream.source.Source.collect_source_info>`,
+ which will use the ``version-guess-pattern`` to attempt to extract a human readable
+ version string from the specified URI, in order to fill out the reported
+ :attr:`~buildstream.source.SourceInfo.version_guess`.
+
+ The URI will be *searched* using this regular expression, and is allowed to
+ yield a number of *groups*. For example the value ``(\\d+)_(\\d+)_(\\d+)`` would
+ report 3 *groups* if 3 numerical values separated by underscores were found in
+ the URI.
+
+ The default value for ``version-guess-pattern`` is ``\\d+\\.\\d+(?:\\.\\d+)?``.
+
+ **Since: 2.5**.
+
+Built-in functionality
Review Comment:
I think this section doesn't belong here (or at least it should be merged with the previous, similarly named one)
##########
src/buildstream/plugins/sources/workspace.py:
##########
@@ -108,6 +108,9 @@ def stage_directory(self, directory):
with self._cache_directory(digest=self.__digest) as cached_directory:
directory._import_files_internal(cached_directory, collect_result=False)
+ def collect_source_info(self):
+ return [SourceInfo(self.path, SourceInfoMedium.WORKSPACE, SourceVersionType.DIGEST, self.get_unique_key())]
Review Comment:
same comment as local
##########
src/buildstream/downloadablefilesource.py:
##########
@@ -270,6 +318,21 @@ def fetch(self): # pylint: disable=arguments-differ
"File downloaded from {} has sha256sum '{}', not '{}'!".format(self.url, sha256, self.ref)
)
+ def collect_source_info(self):
+ version_match = self._guess_pattern.search(self.original_url)
+ if not version_match:
+ version_guess = None
+ elif self._guess_pattern.groups == 0:
+ version_guess = version_match.group(0)
+ else:
+ version_guess = ".".join(version_match.groups())
Review Comment:
Maybe we could have this version guessing logic in a function in utils?
##########
src/buildstream/source.py:
##########
@@ -262,10 +261,147 @@ def __init__(
@dataclass
class AliasSubstitution:
+ """AliasSubstitution()
+ An opaque data structure which may be passed through
+ :func:`SourceFetcher.fetch() <buildstream.source.SourceFetcher.fetch>` and in such cases
+ must be provided to :func:`Source.translate_url() <buildstream.source.Source.translate_url>`.
+ """
+
_effective_alias: str
_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
+ """
+
+
+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 CAS digest representing the unique version of this source input
Review Comment:
We could be more explicit about the `{hash}/{size}` format?
##########
src/buildstream/source.py:
##########
@@ -684,6 +821,19 @@ def is_cached(self) -> bool:
"""
raise ImplError("Source plugin '{}' does not implement is_cached()".format(self.get_kind()))
+ def collect_source_info(self) -> Iterable[SourceInfo]:
+ """Get the :class:`.SourceInfo` objects describing this source
+
+ This method is guaranteed to only be called whenever
+ :func:`Source.is_resolved() <buildstream.source.Source.is_resolved>`
+ returns `True`.
+
+ Returns: the :class:`.SourceInfo` objects describing this source
+
+ *Since: 2.5*
+ """
+ raise ImplError("Source plugin '{}' does not implement collect_source_info()".format(self.get_kind()))
Review Comment:
Could we have this collect source info from source fetchers by default? Any plugin that uses source fetchers will definitely want to implement source info in source fetchers.
Of course, this is more for standardisation than anything, one can always implement it like you did in https://github.com/apache/buildstream-plugins/pull/87
##########
src/buildstream/_frontend/widget.py:
##########
@@ -437,6 +437,23 @@ def show_pipeline(self, dependencies, format_):
runtime_deps = [e._get_full_name() for e in element._dependencies(_Scope.RUN, recurse=False)]
line = p.fmt_subst(line, "runtime-deps", _yaml.roundtrip_dump_string(runtime_deps).rstrip("\n"))
+ # Source Information
+ if "%{source-info" in format_:
+
+ # Get all the SourceInfo objects
+ #
+ all_source_infos = []
+ for source in element.sources():
Review Comment:
A was playing with this a little, and came up with this:
```suggestion
source_element = element._get_source_element()
for source in source_element.sources():
```
given `Element._get_source_element()` gives the element that has the sources for this element, it really made sense. It gives a nice solution to the issue of actual sources of filter elements not being accessible.
I'm thinking we can push it further and move this whole loop into an `Element.collect_source_info()` method. This way, something like the `collect_manifest` plugin wouldn't need to call `Element.sources()` at all (and we can deprecate it). It would be able to call `_get_source_element()` as needed, and users won't have to worry about it.
Moving forward with this idea, we could even have the `compose` plugin collect sources from all build dependencies that contribute files to the final image. Though this might need some more thinking.
##########
src/buildstream/source.py:
##########
@@ -262,10 +261,147 @@ def __init__(
@dataclass
class AliasSubstitution:
+ """AliasSubstitution()
+ An opaque data structure which may be passed through
+ :func:`SourceFetcher.fetch() <buildstream.source.SourceFetcher.fetch>` and in such cases
+ must be provided to :func:`Source.translate_url() <buildstream.source.Source.translate_url>`.
+ """
+
_effective_alias: str
_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
+ """
+
+
+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
Review Comment:
Should we comment something about git specifically? In particular, our plugins can use either the commit sha1, or a the long version of the git describe.
I think the current description fits both, which I think is a good thing. But maybe it's better to give an example.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WIP: Add new abstract methods for reporting source provenance [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r2041867203
##########
src/buildstream/source.py:
##########
@@ -262,10 +261,147 @@ def __init__(
@dataclass
class AliasSubstitution:
+ """AliasSubstitution()
+ An opaque data structure which may be passed through
+ :func:`SourceFetcher.fetch() <buildstream.source.SourceFetcher.fetch>` and in such cases
+ must be provided to :func:`Source.translate_url() <buildstream.source.Source.translate_url>`.
+ """
+
_effective_alias: str
_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
+ """
+
+
+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 CAS digest representing the unique version of this source input
Review Comment:
> We could be more explicit about the {hash}/{size} format?
So I'm hoping to minimize how many types we put here, and I'm thinking along the lines of:
* If its a `SourceInfoMedium.LOCAL`, then a `SourceVersionType.DIGEST` is a CAS digest, which contains the necessary info (as you pointed out it should be `{hash}/{size}`.
* If it's a `SourceInfoMedium.OCI_REGISTRY`, it just has the digest hash
Otherwise, we need to have a lot of explicit types.
On the other hand, as you point out... this makes it difficult to clearly document the meaning of things... so perhaps I need a different `DIGEST` type for OCI source plugins like the `docker` source ?
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WIP: Add new abstract methods for reporting source provenance [buildstream]
Posted by "abderrahim (via GitHub)" <gi...@apache.org>.
abderrahim commented on PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#issuecomment-2816573995
> Ok, this all makes sense, however now that I look at it again, I no longer see any need whatsoever of having the `SourceInfoMedium` or `SourceVersionType` information present **at all** in the `SourceInfo` object.
>
> Instead we only need the following members:
>
> * `kind`: Plugin "kind" string
>
> * `url`: Unaliased upstream full URL (not mirrored URL)
>
> * `version`: The machine readable _precise_ version (e.g. git sha, CAS/docker digest, sha256sum, etc)
>
> * `version_guess`: The plugin assisted _guess_ at what the human readable version is
I feel this pushes it too far in the opposite direction. This assumes a single mapping between a plugin kind and a (medium, version_type) pair, which isn't always the case. For instance the cargo2 plugin can use both tarballs and git repositories. Also plugins like git, git_repo and git_module are very similar in what they provide and having a user of the API need to know all the different plugins seems unweildy.
The main goal behind having an API like this is to reduce the amount of knowledge that a user of the API needs about specific plugins.
> Given that we can obtain the precise plugin and version for a given `kind`, and that that governing plugin documentation **must** already document precisely _what it means_ in the `version` member, the whole concept of the `medium` and `version_type` are obsolete and essentially meaningless.
"We" as humans, sure. But as a script using the API, having a well defined meaning for the version field and a standardized way to define things is very important. Even if a new value comes up later, then depending on the use case the script could output a warning or outright fail. But having it fail on every new plugin isn't scalable.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WIP: Add new abstract methods for reporting source provenance [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan commented on PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#issuecomment-2816625478
[...]
>
> I feel this pushes it too far in the opposite direction. This assumes a single mapping between a plugin kind and a (medium, version_type) pair, which isn't always the case. For instance the cargo2 plugin can use both tarballs and git repositories. Also plugins like git, git_repo and git_module are very similar in what they provide and having a user of the API need to know all the different plugins seems unweildy.
Yeah, I was just now thinking about this... its unfortunate :-/
> > Given that we can obtain the precise plugin and version for a given `kind`, and that that governing plugin documentation **must** already document precisely _what it means_ in the `version` member, the whole concept of the `medium` and `version_type` are obsolete and essentially meaningless.
>
> "We" as humans, sure. But as a script using the API, having a well defined meaning for the version field and a standardized way to define things is very important. Even if a new value comes up later, then depending on the use case the script could output a warning or outright fail. But having it fail on every new plugin isn't scalable.
While this point is now moot due to the preceding point, I would have agrued that it is in fact humans which need to know *"What to do with this specific plugin kind"* when encountering one programatically.
This is the same kind of documentation that will be needed for representing what the meaning of these free form strings are.
Do you think there is any reason to have 2 different strings (one for the medium and one for the version) ? Or would it be sufficient to have a single string to define how a given `SourceInfo` instance behaves ?
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add new abstract methods for reporting source provenance [buildstream]
Posted by "juergbi (via GitHub)" <gi...@apache.org>.
juergbi commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r1995346164
##########
src/buildstream/source.py:
##########
@@ -262,10 +262,123 @@ def __init__(
@dataclass
class AliasSubstitution:
+ """AliasSubstitution()
+ An opaque data structure which may be passed through
+ :func:`SourceFetcher.fetch() <buildstream.source.SourceFetcher.fetch>` and in such cases
+ must be provided to :func:`Source.translate_url() <buildstream.source.Source.translate_url>`.
+ """
+
_effective_alias: str
_mirror: Union[SourceMirror, str]
+class SourceInfoMedium(FastEnum):
+ """
+ Indicates the meduim in which the source is obtained
+
+ *Since: 2.5*
+ """
+
+ LOCAL = "local"
+ """
+ Files stored locally in the project
+ """
+
+ ARCHIVE = "archive"
Review Comment:
`LOCAL` vs. `ARCHIVE` seem like orthogonal attributes to me. You can have a local archive in your project and you can fetch a single e.g. config file from a remote server, at least theoretically. Maybe `LOCAL` should rather be a flag than part of this enum? (Or maybe even make it implicit as the corresponding URL will have no host component).
I'm also not sure whether we should have one enum entry for each archive file format (e.g. `TAR` and `ZIP`) or whether a generic `ARCHIVE` makes more sense.
##########
src/buildstream/source.py:
##########
@@ -262,10 +262,123 @@ def __init__(
@dataclass
class AliasSubstitution:
+ """AliasSubstitution()
+ An opaque data structure which may be passed through
+ :func:`SourceFetcher.fetch() <buildstream.source.SourceFetcher.fetch>` and in such cases
+ must be provided to :func:`Source.translate_url() <buildstream.source.Source.translate_url>`.
+ """
+
_effective_alias: str
_mirror: Union[SourceMirror, str]
+class SourceInfoMedium(FastEnum):
+ """
+ Indicates the meduim in which the source is obtained
Review Comment:
```suggestion
Indicates the medium in which the source is obtained
```
##########
src/buildstream/downloadablefilesource.py:
##########
@@ -270,6 +270,13 @@ def fetch(self): # pylint: disable=arguments-differ
"File downloaded from {} has sha256sum '{}', not '{}'!".format(self.url, sha256, self.ref)
)
+ def collect_source_info(self):
+ #
+ # XXX remote sources are not necessarily archives, perhaps we should
+ # allow downloadablefilesource imlementations to choose the SourceInfoMedium
+ #
+ return [SourceInfo(self.url, SourceInfoMedium.ARCHIVE, SourceVersionType.SHA256, self.ref)]
Review Comment:
`self.url` is a fully qualified URL after alias translation. The unique key of a source (and thus, the cache key of an element) only covers the aliased URL, though. I.e., `collect_source_info()` may return different fully qualified URLs (including URLs of internal mirrors) for builds with the same cache key. I'm not sure how to solve this but it seems like a potential issue. Or am I misreading the code?
This issue is not specific to `DownloadableFileSource`, of course.
##########
src/buildstream/source.py:
##########
@@ -262,10 +262,123 @@ def __init__(
@dataclass
class AliasSubstitution:
+ """AliasSubstitution()
+ An opaque data structure which may be passed through
+ :func:`SourceFetcher.fetch() <buildstream.source.SourceFetcher.fetch>` and in such cases
+ must be provided to :func:`Source.translate_url() <buildstream.source.Source.translate_url>`.
+ """
+
_effective_alias: str
_mirror: Union[SourceMirror, str]
+class SourceInfoMedium(FastEnum):
+ """
+ Indicates the meduim in which the source is obtained
+
+ *Since: 2.5*
+ """
+
+ LOCAL = "local"
+ """
+ Files stored locally in the project
+ """
+
+ ARCHIVE = "archive"
+ """
+ An archive file
+ """
+
+ GIT = "git"
+ """
+ A git repository
+ """
+
+
+class SourceVersionType(FastEnum):
+ """
+ Indicates the type of the version string
+
+ *Since: 2.5*
+ """
+
+ VERSION = "version"
+ """
+ The upstream version string, which may be semantic version
+ """
+
+ 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 CAS digest representing the unique version of this source input
+ """
+
+
+class SourceInfo:
+ """SourceInfo()
+
+ An object representing the provenance of input reported by
+ :func:`Source.collect_source_info() <buildstream.source.Source.collect_source_info>`
+
+ *Since: 2.5*
+ """
+
+ def __init__(self, url: str, medium: str, version_type: str, version: str):
+ # XXX assert medium and version_type are valid values for the enums
+
+ self.url: str = url
+ """
+ The url of the source input
+ """
+
+ self.medium: str = medium
+ """
+ The :class:`.SourceInfoMedium` of the source input
+ """
+
+ self.version_type: str = version_type
Review Comment:
A single source URL may have both an upstream version and e.g. a commit or hash. Do we want/need to support this? Or is the idea to handle this with multiple `SourceInfo` objects pointing to the same URL?
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WIP: Add new abstract methods for reporting source provenance [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r2055253546
##########
src/buildstream/downloadablefilesource.py:
##########
@@ -25,6 +25,46 @@
implementation.
+Built-in functionality
+----------------------
+The DownloadableFileSource class provides built in keys which can be set when
+intantiating any Source which derives from DownloadableFileSource
+
+* Guess pattern
+
+ This ``version-guess-pattern`` sets the regular expression which will be used to attempt
+ to guess the version of a source when parsing the source's URI.
+
+ The DownloadableFileSource provides a default implementation of
+ :func:`Source.collect_source_info() <buildstream.source.Source.collect_source_info>`,
+ which will use the ``version-guess-pattern`` to attempt to extract a human readable
+ version string from the specified URI, in order to fill out the reported
+ :attr:`~buildstream.source.SourceInfo.version_guess`.
+
+ The URI will be *searched* using this regular expression, and is allowed to
+ yield a number of *groups*. For example the value ``(\\d+)_(\\d+)_(\\d+)`` would
+ report 3 *groups* if 3 numerical values separated by underscores were found in
+ the URI.
+
+ The default value for ``version-guess-pattern`` is ``\\d+\\.\\d+(?:\\.\\d+)?``.
+
+ **Since: 2.5**.
+
+Built-in functionality
Review Comment:
This was a copy/paste error, that part was erronously carried over from the Source base class, fixed now.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WIP: Add new abstract methods for reporting source provenance [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r2055264172
##########
src/buildstream/source.py:
##########
@@ -684,6 +821,19 @@ def is_cached(self) -> bool:
"""
raise ImplError("Source plugin '{}' does not implement is_cached()".format(self.get_kind()))
+ def collect_source_info(self) -> Iterable[SourceInfo]:
+ """Get the :class:`.SourceInfo` objects describing this source
+
+ This method is guaranteed to only be called whenever
+ :func:`Source.is_resolved() <buildstream.source.Source.is_resolved>`
+ returns `True`.
+
+ Returns: the :class:`.SourceInfo` objects describing this source
+
+ *Since: 2.5*
+ """
+ raise ImplError("Source plugin '{}' does not implement collect_source_info()".format(self.get_kind()))
Review Comment:
I've added this and updated the plugins which use fetchers.
In addition, I've added a public facing `SourceImplError` and updated the `collect_source_info()` docstring to specify that callers of the API need to handle `SourceImplError` exceptions.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WIP: Add new abstract methods for reporting source provenance [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r2059970628
##########
src/buildstream/source.py:
##########
@@ -262,10 +261,147 @@ def __init__(
@dataclass
class AliasSubstitution:
+ """AliasSubstitution()
+ An opaque data structure which may be passed through
+ :func:`SourceFetcher.fetch() <buildstream.source.SourceFetcher.fetch>` and in such cases
+ must be provided to :func:`Source.translate_url() <buildstream.source.Source.translate_url>`.
+ """
+
_effective_alias: str
_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
+ """
+
+
+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
Review Comment:
I'm satisfied with this branch and the https://github.com/apache/buildstream-plugins/pull/87 now to an extent that I'm sure we can do something correct in the git_repo and git_tag plugins.
Preferably, they will report a git sha only as the *version* and some user comprehensible-ish string in the *version_guess*.
Closing out this comment.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add new abstract methods for reporting source provenance [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r2068689644
##########
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:
Hmmm ok that's interesting and true.
For example, even considering unchanging element configurations, the directory where a source is *staged* is a source configuration which will/must effect cache keys, but is unrelated to source *provenance* and is not necessary in the SourceInfo.
I'll think of a better way to reword this.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WIP: Add new abstract methods for reporting source provenance [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan commented on PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#issuecomment-2765809160
> In terms of implementing this into plugins that currently use the internal API, would someone be able to write up an example plugin that uses the new API - I'm a little lost on the implementation semantics...
See the core plugins in this PR (separate commits), or the changes in the sister PRs
* https://gitlab.com/BuildStream/buildstream-plugins-community/-/merge_requests/364
* https://github.com/apache/buildstream-plugins/pull/870
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WIP: Add new abstract methods for reporting source provenance [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r2055261885
##########
src/buildstream/source.py:
##########
@@ -262,10 +261,147 @@ def __init__(
@dataclass
class AliasSubstitution:
+ """AliasSubstitution()
+ An opaque data structure which may be passed through
+ :func:`SourceFetcher.fetch() <buildstream.source.SourceFetcher.fetch>` and in such cases
+ must be provided to :func:`Source.translate_url() <buildstream.source.Source.translate_url>`.
+ """
+
_effective_alias: str
_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
+ """
+
+
+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 CAS digest representing the unique version of this source input
Review Comment:
I've updated the docstring to specify this format for cased *when* the `DIGEST` type refers to a CAS digest.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WIP: Add new abstract methods for reporting source provenance [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r2055262725
##########
src/buildstream/_frontend/widget.py:
##########
@@ -437,6 +437,23 @@ def show_pipeline(self, dependencies, format_):
runtime_deps = [e._get_full_name() for e in element._dependencies(_Scope.RUN, recurse=False)]
line = p.fmt_subst(line, "runtime-deps", _yaml.roundtrip_dump_string(runtime_deps).rstrip("\n"))
+ # Source Information
+ if "%{source-info" in format_:
+
+ # Get all the SourceInfo objects
+ #
+ all_source_infos = []
+ for source in element.sources():
Review Comment:
We agreed to postpone this.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add new abstract methods for reporting source provenance [buildstream]
Posted by "abderrahim (via GitHub)" <gi...@apache.org>.
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add new abstract methods for reporting source provenance [buildstream]
Posted by "shymega (via GitHub)" <gi...@apache.org>.
shymega commented on PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#issuecomment-2759196146
In terms of implementing this into plugins that currently use the internal API, would someone be able to write up an example plugin that uses the new API - I'm a little lost on the implementation semantics...
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WIP: Add new abstract methods for reporting source provenance [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r2041869048
##########
src/buildstream/source.py:
##########
@@ -684,6 +821,19 @@ def is_cached(self) -> bool:
"""
raise ImplError("Source plugin '{}' does not implement is_cached()".format(self.get_kind()))
+ def collect_source_info(self) -> Iterable[SourceInfo]:
+ """Get the :class:`.SourceInfo` objects describing this source
+
+ This method is guaranteed to only be called whenever
+ :func:`Source.is_resolved() <buildstream.source.Source.is_resolved>`
+ returns `True`.
+
+ Returns: the :class:`.SourceInfo` objects describing this source
+
+ *Since: 2.5*
+ """
+ raise ImplError("Source plugin '{}' does not implement collect_source_info()".format(self.get_kind()))
Review Comment:
This seems sensible, I'll add something to the `SourceFetcher` API for that.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add new abstract methods for reporting source provenance [buildstream]
Posted by "nanonyme (via GitHub)" <gi...@apache.org>.
nanonyme commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r2017682739
##########
src/buildstream/source.py:
##########
@@ -262,10 +262,123 @@ def __init__(
@dataclass
class AliasSubstitution:
+ """AliasSubstitution()
+ An opaque data structure which may be passed through
+ :func:`SourceFetcher.fetch() <buildstream.source.SourceFetcher.fetch>` and in such cases
+ must be provided to :func:`Source.translate_url() <buildstream.source.Source.translate_url>`.
+ """
+
_effective_alias: str
_mirror: Union[SourceMirror, str]
+class SourceInfoMedium(FastEnum):
+ """
+ Indicates the meduim in which the source is obtained
+
+ *Since: 2.5*
+ """
+
+ LOCAL = "local"
+ """
+ Files stored locally in the project
+ """
+
+ ARCHIVE = "archive"
+ """
+ An archive file
+ """
+
+ GIT = "git"
+ """
+ A git repository
+ """
+
+
+class SourceVersionType(FastEnum):
+ """
+ Indicates the type of the version string
+
+ *Since: 2.5*
+ """
+
+ VERSION = "version"
+ """
+ The upstream version string, which may be semantic version
+ """
+
+ 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 CAS digest representing the unique version of this source input
+ """
+
+
+class SourceInfo:
+ """SourceInfo()
+
+ An object representing the provenance of input reported by
+ :func:`Source.collect_source_info() <buildstream.source.Source.collect_source_info>`
+
+ *Since: 2.5*
+ """
+
+ def __init__(self, url: str, medium: str, version_type: str, version: str):
+ # XXX assert medium and version_type are valid values for the enums
+
+ self.url: str = url
+ """
+ The url of the source input
+ """
+
+ self.medium: str = medium
+ """
+ The :class:`.SourceInfoMedium` of the source input
+ """
+
+ self.version_type: str = version_type
Review Comment:
This seems like a hard part of the problem to me. This is currently easily overridable through public data. If that cannot be done with this new API, then it feels like a step backwards.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WIP: Add new abstract methods for reporting source provenance [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan commented on PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#issuecomment-2816508216
@abderrahim, @juergbi
Some additional thoughts now that I get down to business...
## Abolish the whole freeform string entirely
In our meeting this week, we discussed that we would prefer to keep the flexibility and not require that `SourceInfoMedium` and `SourceVersionType` enumeration values be strictly followed (resulting in the requirement of many distinct values being introduced into the core)...
**And** we discussed that we would instead prefer freeform strings for these attributes of the `SourceInfo` object, and that we should:
* Automatically encode the plugin *"kind"* name into the SourceInfo reported
* Provide a machine readable way (e.g. via `bst show`) to provide the plugin provenance information for given plugin kinds
* Have the deciding information about the *meaning* of the `SourceInfo.version` string (e.g. is it a git sha or is it a sha256sum of a downloaded file or what), documented in the governing source plugin documentation individually.
Ok, this all makes sense, however now that I look at it again, I no longer see any need whatsoever of having the `SourceInfoMedium` or `SourceVersionType` information present **at all** in the `SourceInfo` object.
Instead we only need the following members:
* `kind`: Plugin "kind" string
* `version`: The machine readable *precise* version (e.g. git sha, CAS/docker digest, sha256sum, etc)
* `version_guess`: The plugin assisted *guess* at what the human readable version is
Given that we can obtain the precise plugin and version for a given `kind`, and that that governing plugin documentation **must** already document precisely *what it means* in the `version` member, the whole concept of the `medium` and `version_type` are obsolete and essentially meaningless.
## Automate the plugin specific documentation
As a separate thought, I feel like the need to associate the governing documentation of *what a `version` means* with a given *plugin `kind`* is overly onerous.
I think that while this can continue to be true, we could make this easier in the following way:
* Source plugins *must document the meaning of the `version`* in their `Source.collect_source_info()` implementation's docstring
* BuildStream can then inspect that docstring at runtime and provide a way to also report that docstring in some machine readable way.
This approach would allow, from both a `collect_manifest` plugin perspective, **and** from a CLI wrapper script perspective, to generate reports which both:
* Have a section including the governing plugin documentation about the *version* for each plugin "kind" in the context of that pipeline
* Have the SourceInfo members for each Source instance, including the plugin "kind" name in the context of that pipeline
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add new abstract methods for reporting source provenance [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r2068504672
##########
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:
Yes.
I believe the `pypi` plugin uses `remote-file` (it should) and downloads tarballs. The `pip` plugin on the other hand requires a pypi instance in order to fetch things (it is actually using host pip to interact with a pypi instance to download specific package versions discovered at track time).
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add new abstract methods for reporting source provenance [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r2072877148
##########
pyproject.toml:
##########
@@ -14,7 +14,7 @@
[build-system]
requires = [
# We need at least version 36.6.0 that introduced "build_meta"
- "setuptools>=36.6.0",
+ "setuptools>=36.6.0,<80",
Review Comment:
Yup thanks a lot for that !
Rebased and this commit is gone now.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add new abstract methods for reporting source provenance [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r1995801257
##########
src/buildstream/downloadablefilesource.py:
##########
@@ -270,6 +270,13 @@ def fetch(self): # pylint: disable=arguments-differ
"File downloaded from {} has sha256sum '{}', not '{}'!".format(self.url, sha256, self.ref)
)
+ def collect_source_info(self):
+ #
+ # XXX remote sources are not necessarily archives, perhaps we should
+ # allow downloadablefilesource imlementations to choose the SourceInfoMedium
+ #
+ return [SourceInfo(self.url, SourceInfoMedium.ARCHIVE, SourceVersionType.SHA256, self.ref)]
Review Comment:
So the `ref` for a `DownloadableFileSource`, if not overridden, is the checksum of the downloaded archive (or "file"), so it is correct for `SourceVersionType.SHA256`.
The `self.url` is a fully qualified URL, my intention here, ideally, would be to guarantee that it is the original fully qualified URL after the initial project aliasing, and guarantee that no mirrors are involved.
The only time that a `self.url` would **not** be the original qualified URL of where the source *originally came from*, is if `Source.collect_source_info()` were to be called by the plugin inside `Source.fetch()` or `Source.track()`, possibly we can make this illegal with an assertion.
I would further like to protect against any *"original URL"* mangling would could result from junctioning and alias mappings, but I'm pretty sure that the alias mapping feature used for mirror plugins does not affect what the original upstream URLs would be.
If the project(s) are written in such a way to not ever write down the real provenance of upstream sources, of course we would get those URLs.
Does this make sense ?
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add new abstract methods for reporting source provenance [buildstream]
Posted by "abderrahim (via GitHub)" <gi...@apache.org>.
abderrahim commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r2005102810
##########
src/buildstream/source.py:
##########
@@ -262,10 +262,123 @@ def __init__(
@dataclass
class AliasSubstitution:
+ """AliasSubstitution()
+ An opaque data structure which may be passed through
+ :func:`SourceFetcher.fetch() <buildstream.source.SourceFetcher.fetch>` and in such cases
+ must be provided to :func:`Source.translate_url() <buildstream.source.Source.translate_url>`.
+ """
+
_effective_alias: str
_mirror: Union[SourceMirror, str]
+class SourceInfoMedium(FastEnum):
+ """
+ Indicates the meduim in which the source is obtained
+
+ *Since: 2.5*
+ """
+
+ LOCAL = "local"
+ """
+ Files stored locally in the project
+ """
+
+ ARCHIVE = "archive"
+ """
+ An archive file
+ """
+
+ GIT = "git"
+ """
+ A git repository
+ """
+
+
+class SourceVersionType(FastEnum):
+ """
+ Indicates the type of the version string
+
+ *Since: 2.5*
+ """
+
+ VERSION = "version"
+ """
+ The upstream version string, which may be semantic version
+ """
+
+ 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 CAS digest representing the unique version of this source input
+ """
+
+
+class SourceInfo:
+ """SourceInfo()
+
+ An object representing the provenance of input reported by
+ :func:`Source.collect_source_info() <buildstream.source.Source.collect_source_info>`
+
+ *Since: 2.5*
+ """
+
+ def __init__(self, url: str, medium: str, version_type: str, version: str):
+ # XXX assert medium and version_type are valid values for the enums
+
+ self.url: str = url
+ """
+ The url of the source input
+ """
+
+ self.medium: str = medium
+ """
+ The :class:`.SourceInfoMedium` of the source input
+ """
+
+ self.version_type: str = version_type
Review Comment:
In general, I feel that it's way easier to determine the version of a tarball than it is to extract the version from a git tag. The tarball naming is just more consistent.
I understand the value of not having plugins care about these things and leave the processing to whatever uses this info, but at the same time it's useful to have plugins care about it because they sometimes have more info available.
> Or is the idea to handle this with multiple SourceInfo objects pointing to the same URL?
The issue with this is that you can't tell if they're referring to the same thing if e.g. you have a git repo and one SourceInfo refers to a tag and the other refers to a commit.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add new abstract methods for reporting source provenance [buildstream]
Posted by "nanonyme (via GitHub)" <gi...@apache.org>.
nanonyme commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r2017682739
##########
src/buildstream/source.py:
##########
@@ -262,10 +262,123 @@ def __init__(
@dataclass
class AliasSubstitution:
+ """AliasSubstitution()
+ An opaque data structure which may be passed through
+ :func:`SourceFetcher.fetch() <buildstream.source.SourceFetcher.fetch>` and in such cases
+ must be provided to :func:`Source.translate_url() <buildstream.source.Source.translate_url>`.
+ """
+
_effective_alias: str
_mirror: Union[SourceMirror, str]
+class SourceInfoMedium(FastEnum):
+ """
+ Indicates the meduim in which the source is obtained
+
+ *Since: 2.5*
+ """
+
+ LOCAL = "local"
+ """
+ Files stored locally in the project
+ """
+
+ ARCHIVE = "archive"
+ """
+ An archive file
+ """
+
+ GIT = "git"
+ """
+ A git repository
+ """
+
+
+class SourceVersionType(FastEnum):
+ """
+ Indicates the type of the version string
+
+ *Since: 2.5*
+ """
+
+ VERSION = "version"
+ """
+ The upstream version string, which may be semantic version
+ """
+
+ 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 CAS digest representing the unique version of this source input
+ """
+
+
+class SourceInfo:
+ """SourceInfo()
+
+ An object representing the provenance of input reported by
+ :func:`Source.collect_source_info() <buildstream.source.Source.collect_source_info>`
+
+ *Since: 2.5*
+ """
+
+ def __init__(self, url: str, medium: str, version_type: str, version: str):
+ # XXX assert medium and version_type are valid values for the enums
+
+ self.url: str = url
+ """
+ The url of the source input
+ """
+
+ self.medium: str = medium
+ """
+ The :class:`.SourceInfoMedium` of the source input
+ """
+
+ self.version_type: str = version_type
Review Comment:
This seems like a hard part of the problem to me. This is currently easily overridable through public data. If that cannot be done with this new API, then it feels like a step backwards.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WIP: Add new abstract methods for reporting source provenance [buildstream]
Posted by "juergbi (via GitHub)" <gi...@apache.org>.
juergbi commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r2028451472
##########
src/buildstream/downloadablefilesource.py:
##########
@@ -270,6 +270,13 @@ def fetch(self): # pylint: disable=arguments-differ
"File downloaded from {} has sha256sum '{}', not '{}'!".format(self.url, sha256, self.ref)
)
+ def collect_source_info(self):
+ #
+ # XXX remote sources are not necessarily archives, perhaps we should
+ # allow downloadablefilesource imlementations to choose the SourceInfoMedium
+ #
+ return [SourceInfo(self.url, SourceInfoMedium.ARCHIVE, SourceVersionType.SHA256, self.ref)]
Review Comment:
I forgot that `self.url` won't be a mirror URL outside `Source.fetch()` and `Source.track()`, so it should indeed be less of an issue. However, even the original URL is not part of the cache key, so changing the alias definition would still result in `collect_source_info()` without a cache key change. Or am I missing something else? Maybe this is acceptable but we should at least be aware of this.
> I would further like to protect against any "original URL" mangling would could result from junctioning and alias mappings, but I'm pretty sure that the alias mapping feature used for mirror plugins does not affect what the original upstream URLs would be.
I'm not sure whether that's the case. Won't the alias definition from the parent project be used if the alias from the subproject is mapped in the junction? That alias definition could potentially point to a different URL without affecting the cache key.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WIP: Add new abstract methods for reporting source provenance [buildstream]
Posted by "abderrahim (via GitHub)" <gi...@apache.org>.
abderrahim commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r2041951159
##########
src/buildstream/source.py:
##########
@@ -262,10 +261,147 @@ def __init__(
@dataclass
class AliasSubstitution:
+ """AliasSubstitution()
+ An opaque data structure which may be passed through
+ :func:`SourceFetcher.fetch() <buildstream.source.SourceFetcher.fetch>` and in such cases
+ must be provided to :func:`Source.translate_url() <buildstream.source.Source.translate_url>`.
+ """
+
_effective_alias: str
_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
+ """
+
+
+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 CAS digest representing the unique version of this source input
Review Comment:
Wouldn't `SHA256` (which we also have) be more appropriate for an OCI registry? `DIGEST` explicitly says "A CAS digest", so it wouldn't be applicable to an OCI registry.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WIP: Add new abstract methods for reporting source provenance [buildstream]
Posted by "shymega (via GitHub)" <gi...@apache.org>.
shymega commented on PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#issuecomment-2794848513
> [apache/buildstream-plugins/pull/870](https://github.com/apache/buildstream-plugins/pull/870)
@gtristan This should be https://github.com/apache/buildstream-plugins/pull/87, not https://github.com/apache/buildstream-plugins/pull/870.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WIP: Add new abstract methods for reporting source provenance [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r2055273339
##########
src/buildstream/downloadablefilesource.py:
##########
@@ -270,6 +270,13 @@ def fetch(self): # pylint: disable=arguments-differ
"File downloaded from {} has sha256sum '{}', not '{}'!".format(self.url, sha256, self.ref)
)
+ def collect_source_info(self):
+ #
+ # XXX remote sources are not necessarily archives, perhaps we should
+ # allow downloadablefilesource imlementations to choose the SourceInfoMedium
+ #
+ return [SourceInfo(self.url, SourceInfoMedium.ARCHIVE, SourceVersionType.SHA256, self.ref)]
Review Comment:
I don't believe that we should worry about congruency of cache keys with the result of URLs found in SBoMs.
However I agree that we should be *aware* of that detail.
I am *more* concerned that the URLs reported in the SBoMs are the intentionally expressed *upstreams* rather than downstream mirrors, as this is a more accurate description of the *provenance* of the data which led, under some configuration, to an artifact with a cache key,
> I'm not sure whether that's the case. Won't the alias definition from the parent project be used if the alias from > the subproject is mapped in the junction? That alias definition could potentially point to a different URL without > affecting the cache key.
This sounds like a *you get what you pay for* scenario. I.e. if you have overridden the defaults for the default aliases of sources in a subproject, rather than just adding your own mirrors, then the SBoMs would still accurately describe what your toplevel project has expressed in terms of inputs.
It is however undesirable for toplevels to redirect the default aliases, and as with the above note about cache keys, I agree that it should be *known*.
I'll leave this comment open for now as a reminder that this indeed should be documented in the appropriate places.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add new abstract methods for reporting source provenance [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r2070360095
##########
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:
Sure, for absolutely no reason at all, except that you like it, I have updated that :)
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WIP: Add new abstract methods for reporting source provenance [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan commented on PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#issuecomment-2764597022
I've been testing this locally with freedesktop-sdk master and the sister merge requests applied:
* https://gitlab.com/BuildStream/buildstream-plugins-community/-/merge_requests/364
* https://github.com/apache/buildstream-plugins/pull/87
There are still more plugins to implement, notably we can have version guessing on the `git_tag` or similar community plugins which encode more context than just the commit sha into the `ref`.
The `cargo2` plugin works, but *only* if you correct `gstreamer-plugins-rs.bst` to set the `git-mirrors` parameter (that whole thing looks like it needs support from buildstream to work with the project aliases automatically... there is probably an issue open about that).
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add new abstract methods for reporting source provenance [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r1995815595
##########
src/buildstream/source.py:
##########
@@ -262,10 +262,123 @@ def __init__(
@dataclass
class AliasSubstitution:
+ """AliasSubstitution()
+ An opaque data structure which may be passed through
+ :func:`SourceFetcher.fetch() <buildstream.source.SourceFetcher.fetch>` and in such cases
+ must be provided to :func:`Source.translate_url() <buildstream.source.Source.translate_url>`.
+ """
+
_effective_alias: str
_mirror: Union[SourceMirror, str]
+class SourceInfoMedium(FastEnum):
+ """
+ Indicates the meduim in which the source is obtained
+
+ *Since: 2.5*
+ """
+
+ LOCAL = "local"
+ """
+ Files stored locally in the project
+ """
+
+ ARCHIVE = "archive"
+ """
+ An archive file
+ """
+
+ GIT = "git"
+ """
+ A git repository
+ """
+
+
+class SourceVersionType(FastEnum):
+ """
+ Indicates the type of the version string
+
+ *Since: 2.5*
+ """
+
+ VERSION = "version"
+ """
+ The upstream version string, which may be semantic version
+ """
+
+ 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 CAS digest representing the unique version of this source input
+ """
+
+
+class SourceInfo:
+ """SourceInfo()
+
+ An object representing the provenance of input reported by
+ :func:`Source.collect_source_info() <buildstream.source.Source.collect_source_info>`
+
+ *Since: 2.5*
+ """
+
+ def __init__(self, url: str, medium: str, version_type: str, version: str):
+ # XXX assert medium and version_type are valid values for the enums
+
+ self.url: str = url
+ """
+ The url of the source input
+ """
+
+ self.medium: str = medium
+ """
+ The :class:`.SourceInfoMedium` of the source input
+ """
+
+ self.version_type: str = version_type
Review Comment:
This is a good question.
My initial thoughts were that some plugins, e.g. some of the git plugins out there in the wild, provide `git describe` format for their `ref`, and thus would provide both contexts in the same string, but always something useful and precise (e.g. merely a tag, or a tag + number of child commits + commit sha).
Another thought I have been playing with along these similar lines is that it may be possible for `Source` classes to require `Source.fetch()` be run and that they have access to the staged sources in order to implement `Source.collect_source_info()`.
Also, we can consider formatting of reports separately, as long as we are strict about what the version type *means*, so we could potentially have a version type which is a *"git sha or git describe"* output, and parse these strings into more fancy reports when generating html and such, if we want to *display* the version and sha separately.
That said, indeed I wonder how one could possibly determine the *version* of a tarball... and in that light I wonder how meaningful the version of a tarball is.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add new abstract methods for reporting source provenance [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r2072329264
##########
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:
This whole block is now updated as per the comment before, and is updated to clarify that it is indeed *only* the URLs which are expected to possibly change for a given set of identical cache keys.
I think this is clear now and addresses the comment appropriately.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WIP: Add new abstract methods for reporting source provenance [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r2055260155
##########
src/buildstream/downloadablefilesource.py:
##########
@@ -270,6 +318,21 @@ def fetch(self): # pylint: disable=arguments-differ
"File downloaded from {} has sha256sum '{}', not '{}'!".format(self.url, sha256, self.ref)
)
+ def collect_source_info(self):
+ version_match = self._guess_pattern.search(self.original_url)
+ if not version_match:
+ version_guess = None
+ elif self._guess_pattern.groups == 0:
+ version_guess = version_match.group(0)
+ else:
+ version_guess = ".".join(version_match.groups())
Review Comment:
I know this guessing is used by collect_manifest also for git plugins, but I'd rather not extend the API at this point, it's a bit unwieldy as an API and includes the definition of a default pattern, overridable pattern and some few lines for the matching - it's a lot of mental gymnastics just to share the few lines of code which only make up a portion of the functionality.
Maybe later.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add new abstract methods for reporting source provenance [buildstream]
Posted by "nanonyme (via GitHub)" <gi...@apache.org>.
nanonyme commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r2072631433
##########
pyproject.toml:
##########
@@ -14,7 +14,7 @@
[build-system]
requires = [
# We need at least version 36.6.0 that introduced "build_meta"
- "setuptools>=36.6.0",
+ "setuptools>=36.6.0,<80",
Review Comment:
This is a pretty nasty thing to do to distro packagers. Is this really necessary or can it be worked around?
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add new abstract methods for reporting source provenance [buildstream]
Posted by "abderrahim (via GitHub)" <gi...@apache.org>.
abderrahim commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r2072565692
##########
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:
Fair enough.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add new abstract methods for reporting source provenance [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan merged PR #1997:
URL: https://github.com/apache/buildstream/pull/1997
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add new abstract methods for reporting source provenance [buildstream]
Posted by "nanonyme (via GitHub)" <gi...@apache.org>.
nanonyme commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r2017554594
##########
src/buildstream/source.py:
##########
@@ -262,10 +262,123 @@ def __init__(
@dataclass
class AliasSubstitution:
+ """AliasSubstitution()
+ An opaque data structure which may be passed through
+ :func:`SourceFetcher.fetch() <buildstream.source.SourceFetcher.fetch>` and in such cases
+ must be provided to :func:`Source.translate_url() <buildstream.source.Source.translate_url>`.
+ """
+
_effective_alias: str
_mirror: Union[SourceMirror, str]
+class SourceInfoMedium(FastEnum):
+ """
+ Indicates the meduim in which the source is obtained
+
+ *Since: 2.5*
+ """
+
+ LOCAL = "local"
+ """
+ Files stored locally in the project
+ """
+
+ ARCHIVE = "archive"
+ """
+ An archive file
+ """
+
+ GIT = "git"
+ """
+ A git repository
+ """
+
+
+class SourceVersionType(FastEnum):
+ """
+ Indicates the type of the version string
+
+ *Since: 2.5*
+ """
+
+ VERSION = "version"
+ """
+ The upstream version string, which may be semantic version
+ """
+
+ 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 CAS digest representing the unique version of this source input
+ """
+
+
+class SourceInfo:
+ """SourceInfo()
+
+ An object representing the provenance of input reported by
+ :func:`Source.collect_source_info() <buildstream.source.Source.collect_source_info>`
+
+ *Since: 2.5*
+ """
+
+ def __init__(self, url: str, medium: str, version_type: str, version: str):
+ # XXX assert medium and version_type are valid values for the enums
+
+ self.url: str = url
+ """
+ The url of the source input
+ """
+
+ self.medium: str = medium
+ """
+ The :class:`.SourceInfoMedium` of the source input
+ """
+
+ self.version_type: str = version_type
Review Comment:
With how we operate now, the person who writes the element that has the source is able to write regexp to parse version from tarball name/ref. Is that possible with this new API?
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add new abstract methods for reporting source provenance [buildstream]
Posted by "nanonyme (via GitHub)" <gi...@apache.org>.
nanonyme commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r2017690063
##########
src/buildstream/source.py:
##########
@@ -262,10 +262,123 @@ def __init__(
@dataclass
class AliasSubstitution:
+ """AliasSubstitution()
+ An opaque data structure which may be passed through
+ :func:`SourceFetcher.fetch() <buildstream.source.SourceFetcher.fetch>` and in such cases
+ must be provided to :func:`Source.translate_url() <buildstream.source.Source.translate_url>`.
+ """
+
_effective_alias: str
_mirror: Union[SourceMirror, str]
+class SourceInfoMedium(FastEnum):
+ """
+ Indicates the meduim in which the source is obtained
+
+ *Since: 2.5*
+ """
+
+ LOCAL = "local"
+ """
+ Files stored locally in the project
+ """
+
+ ARCHIVE = "archive"
+ """
+ An archive file
+ """
+
+ GIT = "git"
+ """
+ A git repository
+ """
+
+
+class SourceVersionType(FastEnum):
+ """
+ Indicates the type of the version string
+
+ *Since: 2.5*
+ """
+
+ VERSION = "version"
+ """
+ The upstream version string, which may be semantic version
+ """
+
+ 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 CAS digest representing the unique version of this source input
+ """
+
+
+class SourceInfo:
+ """SourceInfo()
+
+ An object representing the provenance of input reported by
+ :func:`Source.collect_source_info() <buildstream.source.Source.collect_source_info>`
+
+ *Since: 2.5*
+ """
+
+ def __init__(self, url: str, medium: str, version_type: str, version: str):
+ # XXX assert medium and version_type are valid values for the enums
+
+ self.url: str = url
+ """
+ The url of the source input
+ """
+
+ self.medium: str = medium
+ """
+ The :class:`.SourceInfoMedium` of the source input
+ """
+
+ self.version_type: str = version_type
Review Comment:
Yes. Right. So I get the problem now. The API I was proposing *did not* specifically give version because that cannot be provided in any useful form. Instead it was providing git ref (in commit or git-describe format) or tar source and letting consumer provide a way to process this into version through out-of-band public data. This is *exactly* because, as you said, mechanisms to read version varies from project to project. IMHO us putting a default matcher to begin with was a mistake, we should have just forced element author to add an explicit matcher whenever there was intent to have version for a source. This would have allowed making it a fatal error always if the match fails.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add new abstract methods for reporting source provenance [buildstream]
Posted by "nanonyme (via GitHub)" <gi...@apache.org>.
nanonyme commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r2017692856
##########
src/buildstream/source.py:
##########
@@ -262,10 +262,123 @@ def __init__(
@dataclass
class AliasSubstitution:
+ """AliasSubstitution()
+ An opaque data structure which may be passed through
+ :func:`SourceFetcher.fetch() <buildstream.source.SourceFetcher.fetch>` and in such cases
+ must be provided to :func:`Source.translate_url() <buildstream.source.Source.translate_url>`.
+ """
+
_effective_alias: str
_mirror: Union[SourceMirror, str]
+class SourceInfoMedium(FastEnum):
+ """
+ Indicates the meduim in which the source is obtained
+
+ *Since: 2.5*
+ """
+
+ LOCAL = "local"
+ """
+ Files stored locally in the project
+ """
+
+ ARCHIVE = "archive"
+ """
+ An archive file
+ """
+
+ GIT = "git"
+ """
+ A git repository
+ """
+
+
+class SourceVersionType(FastEnum):
+ """
+ Indicates the type of the version string
+
+ *Since: 2.5*
+ """
+
+ VERSION = "version"
+ """
+ The upstream version string, which may be semantic version
+ """
+
+ 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 CAS digest representing the unique version of this source input
+ """
+
+
+class SourceInfo:
+ """SourceInfo()
+
+ An object representing the provenance of input reported by
+ :func:`Source.collect_source_info() <buildstream.source.Source.collect_source_info>`
+
+ *Since: 2.5*
+ """
+
+ def __init__(self, url: str, medium: str, version_type: str, version: str):
+ # XXX assert medium and version_type are valid values for the enums
+
+ self.url: str = url
+ """
+ The url of the source input
+ """
+
+ self.medium: str = medium
+ """
+ The :class:`.SourceInfoMedium` of the source input
+ """
+
+ self.version_type: str = version_type
Review Comment:
The problem is that even though git release tags may have release version in it, the specific formattings vary wildly. Extracting version in a way that you manage to extract the wanted version and nothing else than the wanted version is harder than solving Halting problem.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add new abstract methods for reporting source provenance [buildstream]
Posted by "gtristan (via GitHub)" <gi...@apache.org>.
gtristan commented on code in PR #1997:
URL: https://github.com/apache/buildstream/pull/1997#discussion_r2020173848
##########
src/buildstream/source.py:
##########
@@ -262,10 +262,123 @@ def __init__(
@dataclass
class AliasSubstitution:
+ """AliasSubstitution()
+ An opaque data structure which may be passed through
+ :func:`SourceFetcher.fetch() <buildstream.source.SourceFetcher.fetch>` and in such cases
+ must be provided to :func:`Source.translate_url() <buildstream.source.Source.translate_url>`.
+ """
+
_effective_alias: str
_mirror: Union[SourceMirror, str]
+class SourceInfoMedium(FastEnum):
+ """
+ Indicates the meduim in which the source is obtained
+
+ *Since: 2.5*
+ """
+
+ LOCAL = "local"
+ """
+ Files stored locally in the project
+ """
+
+ ARCHIVE = "archive"
+ """
+ An archive file
+ """
+
+ GIT = "git"
+ """
+ A git repository
+ """
+
+
+class SourceVersionType(FastEnum):
+ """
+ Indicates the type of the version string
+
+ *Since: 2.5*
+ """
+
+ VERSION = "version"
+ """
+ The upstream version string, which may be semantic version
+ """
+
+ 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 CAS digest representing the unique version of this source input
+ """
+
+
+class SourceInfo:
+ """SourceInfo()
+
+ An object representing the provenance of input reported by
+ :func:`Source.collect_source_info() <buildstream.source.Source.collect_source_info>`
+
+ *Since: 2.5*
+ """
+
+ def __init__(self, url: str, medium: str, version_type: str, version: str):
+ # XXX assert medium and version_type are valid values for the enums
+
+ self.url: str = url
+ """
+ The url of the source input
+ """
+
+ self.medium: str = medium
+ """
+ The :class:`.SourceInfoMedium` of the source input
+ """
+
+ self.version_type: str = version_type
Review Comment:
I've updated the patches now to make a compromise here.
* We keep the *specific* versioning, which can be an sha256sum or a git hash or anything which can *accurately* describe a unique source version.
* We add a `SourceInfo.version_guess` member to the data type. This is implemented by plugins who implement it, for the `DownloadableFileSource`, there is a default implementation which does the regex thing found in the collect_manifest code, and an added `version-guess-pattern` configure key allowing the user to specify the regex which should be used in order to make a guess in determining the version.
I think essentially this gives us the same functionality, however I'm just more comfortable with calling this a *guess*, and avoid any misinterpretation of the promises BuildStream is making here.
Essentially, really *knowing* a version requires some human understanding of the upstream project, what their release policy is, and what, if anything, their release tags or git branches mean in relation to that.
--
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: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org