Fix and enhance support for different bazel metadata versions #4194
Conversation
Signed-off-by: Adrian Braemer <[email protected]>
Signed-off-by: Adrian Braemer <[email protected]>
abraemer
force-pushed
the
fix-conditions-for-bzl-package
branch
from
1647116 to
bdb908c
Compare
* by combining the paths we ensure to extract maximal information * I also added the possibility to extract information from a 'package_url' field Signed-off-by: Adrian Braemer <[email protected]>
abraemer
changed the title
|
Test failures seem unrelated to my changes |
There was a problem hiding this comment.
Thanks a lot @abraemer , a couple nits for your consideration. Looking good otherwise.
| "name": "androidx.compose.animation:animation", | ||
| "upstream_address": "https://developer.android.com/jetpack/androidx/releases/compose-animation#0.0.1", | ||
| "version": "0.0.1", | ||
| "package_url" : "pkg:maven/androidx.compose.animation/[email protected]" |
There was a problem hiding this comment.
Could you maybe link to some examples of this type of manifests with package_url fields and maybe add one of those as tests, it's best to use real world examples probably. Or if you got this from some real example, you can also link that file here.
There was a problem hiding this comment.
Unfortunately, I cannot link you one of our internal files. However my example file is very close to an actual file (the version number is wrong but that's all).
src/packagedcode/build.py
Outdated
| # TODO: Store 'upstream_hash` somewhere | ||
| ) | ||
| if 'vcs_commit_hash' in metadata_fields: | ||
| package_data["extra_data"] = dict(vcs_commit_hash=metadata_fields['vcs_commit_hash']) |
There was a problem hiding this comment.
Wouldn't it be better to do a get() rather than checking if this exists in the mapping?
There was a problem hiding this comment.
When including the upstream_hash, I rewrote this part. Now we always create the extra_data dict with all keys and I use .get to extract the values.
* always create extra_data dictionary * use get to extract information from metadatafields instead of branches * also extract upstream_hash Signed-off-by: Adrian Braemer <[email protected]>
abraemer
force-pushed
the
fix-conditions-for-bzl-package
branch
from
5a9bda6 to
65d2995
Compare
I noticed that the conditions deciding between the two version for
.bzlfiles are somewhat wrong. In Python'a' and 'b' in some_stringis equal to('a' and 'b') in some_stringand thus simplifies to'b' in some_string. In this instance, I think it shouldn't make a huge difference because fortunately the stringsupstream_addressandvcs_commit_hashstill somewhat distinguish between versions.I think just fixing the conditions makes them too restrictive (AFAIK there is no real standard for these METADATA.bzl files), so I combined both paths into one. This ensures that we always extract as much information as possible.
I also added the option to use information from a
package_urlfield which is important to me as we often have someMETADATA.bzlfiles which are generated from Maven dependencies and thus thenameis invalid by PURL specification. I wrote a somewhat realistic test case for this.Tasks
Run tests locally to check for errors.
Closes #4196