Fix and enhance support for different bazel metadata versions #4194

abraemer · 2025-03-17T14:22:39Z

I noticed that the conditions deciding between the two version for .bzl files are somewhat wrong. In Python 'a' and 'b' in some_string is equal to ('a' and 'b') in some_string and thus simplifies to 'b' in some_string. In this instance, I think it shouldn't make a huge difference because fortunately the strings upstream_address and vcs_commit_hash still 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_url field which is important to me as we often have some METADATA.bzl files which are generated from Maven dependencies and thus the name is invalid by PURL specification. I wrote a somewhat realistic test case for this.

Tasks

Reviewed contribution guidelines
PR is descriptively titled 📑 and links the original issue above 🔗
Tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR
Run tests locally to check for errors.
Commits are in uniquely-named feature branch and has no merge conflicts 📁

Closes #4196

Signed-off-by: Adrian Braemer <[email protected]>

* 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 · 2025-03-18T07:22:19Z

Test failures seem unrelated to my changes

AyanSinhaMahapatra

Thanks a lot @abraemer , a couple nits for your consideration. Looking good otherwise.

AyanSinhaMahapatra · 2025-03-19T15:55:54Z

tests/packagedcode/data/build/metadatabzl/with-package-url/METADATA.bzl

+    "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]"


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.

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).

Unfortunately, I cannot link you one of our internal files.

No worries on that obviously, but can you find some other real examples with package_url anywhere on github/elsewhere, or is this relatively new?

I searched on GitHub but did not find any other files that use that field but also I did not find many examples of bzl files at all. So perhaps it is not very common (yet?).
As this doesn't interfere with other things, would you be okay with merging this anyways? After all it just adds another possible data source for bzl-files that has the potential to be useful for others :)

src/packagedcode/build.py

AyanSinhaMahapatra · 2025-03-19T16:02:33Z

src/packagedcode/build.py

+            # 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'])


Wouldn't it be better to do a get() rather than checking if this exists in the mapping?

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.

abraemer · 2025-04-04T05:51:26Z

friendly ping since it has been 2 weeks now and I think this ready :) What do you think @AyanSinhaMahapatra?

AyanSinhaMahapatra

Thanks for the changes @abraemer, this is almost ready with a couple small changes, apologies for the late review 😅

src/licensedcode/juliacode.jl

src/packagedcode/build.py

AyanSinhaMahapatra · 2025-04-04T19:02:45Z

tests/packagedcode/data/build/metadatabzl/with-package-url/METADATA.bzl

+    "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]"


Unfortunately, I cannot link you one of our internal files.

No worries on that obviously, but can you find some other real examples with package_url anywhere on github/elsewhere, or is this relatively new?

abraemer · 2025-04-15T09:59:48Z

@AyanSinhaMahapatra friendly ping :)

AyanSinhaMahapatra

Thanks++ @abraemer, last nit for your considerations. Ready to merge otherwise. Thanks for your patience.

Could you also merge from develop again, should get rid of the flaky test failures.

src/packagedcode/build.py

* 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 · 2025-04-17T14:09:30Z

@AyanSinhaMahapatra last nit is fixed and branch is up-to-date with develop. Let's see what CI says :)

As we now store the upstream hash in the field `extra_data`, we need to make the test aware of that. Signed-off-by: Adrian Braemer <[email protected]>

AyanSinhaMahapatra · 2025-04-23T12:45:04Z

Thanks++ @abraemer, all green and merging!

abraemer added 2 commits March 17, 2025 15:36

Fix conditions for bzl package versions

45990c4

Signed-off-by: Adrian Braemer <[email protected]>

add myself to AUTHORS

bdb908c

Signed-off-by: Adrian Braemer <[email protected]>

abraemer force-pushed the fix-conditions-for-bzl-package branch from 1647116 to bdb908c Compare March 17, 2025 14:36

combine paths for different bzl versions

8f7875b

* 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 ~~Fix conditions for bzl package versions~~ Fix and enhance support for different bazel metadata versions Mar 17, 2025

abraemer mentioned this pull request Mar 18, 2025

Extraction of package data from Bazel Metadata files is too strict #4196

Closed

7 tasks

AyanSinhaMahapatra requested changes Mar 19, 2025

View reviewed changes

abraemer requested a review from AyanSinhaMahapatra March 21, 2025 07:13

abraemer force-pushed the fix-conditions-for-bzl-package branch from 5a9bda6 to 65d2995 Compare March 21, 2025 07:21

vasily-pozdnyakov mentioned this pull request Mar 27, 2025

Error when scanning metadata files with empty type #4216

Open

AyanSinhaMahapatra requested changes Apr 4, 2025

View reviewed changes

abraemer force-pushed the fix-conditions-for-bzl-package branch from 65d2995 to 1051aa6 Compare April 7, 2025 07:51

AyanSinhaMahapatra reviewed Apr 16, 2025

View reviewed changes

src/packagedcode/build.py Outdated Show resolved Hide resolved

fix: minor changes to address review comments

658f0d8

* 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 1051aa6 to 658f0d8 Compare April 17, 2025 14:04

fix: Adapt test to include new upstream hash

454910f

As we now store the upstream hash in the field `extra_data`, we need to make the test aware of that. Signed-off-by: Adrian Braemer <[email protected]>

AyanSinhaMahapatra merged commit 98fa3fd into aboutcode-org:develop Apr 23, 2025
38 checks passed

abraemer deleted the fix-conditions-for-bzl-package branch April 25, 2025 06:08

Uh oh!

Fix and enhance support for different bazel metadata versions #4194

Fix and enhance support for different bazel metadata versions #4194

Uh oh!

Conversation

abraemer commented Mar 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tasks

Uh oh!

abraemer commented Mar 18, 2025

Uh oh!

AyanSinhaMahapatra left a comment

Choose a reason for hiding this comment

Uh oh!

AyanSinhaMahapatra Mar 19, 2025

Choose a reason for hiding this comment

Uh oh!

abraemer Mar 21, 2025

Choose a reason for hiding this comment

Uh oh!

AyanSinhaMahapatra Apr 4, 2025

Choose a reason for hiding this comment

Uh oh!

abraemer Apr 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AyanSinhaMahapatra Mar 19, 2025

Choose a reason for hiding this comment

Uh oh!

abraemer Mar 21, 2025

Choose a reason for hiding this comment

Uh oh!

abraemer commented Apr 4, 2025

Uh oh!

AyanSinhaMahapatra left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AyanSinhaMahapatra Apr 4, 2025

Choose a reason for hiding this comment

Uh oh!

abraemer commented Apr 15, 2025

Uh oh!

AyanSinhaMahapatra left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

abraemer commented Apr 17, 2025

Uh oh!

Uh oh!

AyanSinhaMahapatra commented Apr 23, 2025

Uh oh!

Uh oh!

abraemer commented Mar 17, 2025 •

edited

Loading

AyanSinhaMahapatra left a comment •

edited

Loading