Add functionality to track LibraryVersion dependencies #1524

brianjp93 · 2024-12-06T00:06:41Z

This pr adds:

A new M2M field on LibraryVersion to track dependencies between libraries
A GitHub workflow that generates dependency reports using boostdep
Management command to update LibraryVersion dependencies from GH artifacts
A daily task to check for the boostdep artifact and update dependencies
Extended GithubAPIClient to fetch artifacts from GitHub Actions
Tests for the new dependency parsing functionality
fixes Save dependencies for each LibraryVersion #1402

The workflow analyzes each Boost release tag of the form "boost-x.x.0" to generate a dependency report which is then used to populate the dependencies M2M between LibraryVersion and Library models.

brianjp93 · 2024-12-06T00:36:09Z

Github action runs on push to master or manually.

sdarwin · 2024-12-06T15:20:28Z

Near the beginning of the github actions it's checking out the whole repository and then discarding it. Here are more efficient alternatives.

Simply download the file in a one-liner without checking out the repo. (if this works it seems preferable).

curl -OL https://github.com/${GITHUB_REPOSITORY}/blob/${GITHUB_SHA}/.github/workflows/scripts/get_deps.sh?raw=true

or

Switch to sparse mode: https://github.com/actions/checkout

# Fetch only a single file
- uses: actions/checkout@v4
  with:
    sparse-checkout: |
      .github/workflows/scripts/get_deps.sh
    sparse-checkout-cone-mode: false

sdarwin · 2024-12-06T17:20:30Z

the PR is ok with me, maybe Greg can review the django code.

This commit adds: - A new M2M field on LibraryVersion to track dependencies between libraries - A GitHub workflow that generates dependency reports using boostdep - Management command to update LibraryVersion dependencies from GH artifacts - Extended GithubAPIClient to fetch artifacts from GitHub Actions - Tests for the new dependency parsing functionality - fixes boostorg#1402 The workflow analyzes each Boost release tag of the form "boost-x.x.0" to generate a dependency report which is then used to populate the dependencies field on LibraryVersion models.

GregKaleka

Nice work. Handful of small changes suggested.

GregKaleka · 2024-12-06T18:00:22Z

core/githubhelper.py

+    def get_artifacts(self, owner=None, repo=None, name=None):
+        """Return a list of artifacts from the GH api.
+
+        Filter results by the name of the artifact by supplying name.
+        """
+        owner = owner or self.owner
+        repo = repo or self.repo_slug
+        url = f"https://api.github.com/repos/{owner}/{repo}/actions/artifacts"


I think sticking to strings and tweaking the parameter name make this a bit clearer.

Suggested change

def get_artifacts(self, owner=None, repo=None, name=None):

"""Return a list of artifacts from the GH api.

Filter results by the name of the artifact by supplying name.

"""

owner = owner or self.owner

repo = repo or self.repo_slug

url = f"https://api.github.com/repos/{owner}/{repo}/actions/artifacts"

def get_artifacts(self, owner="", repo_slug="", name=""):

"""Return a list of artifacts from the GH api.

Filter results by the name of the artifact by supplying name.

"""

owner = owner or self.owner

repo = repo_slug or self.repo_slug

url = f"https://api.github.com/repos/{owner}/{repo_slug}/actions/artifacts"

GregKaleka · 2024-12-06T18:05:23Z

libraries/github.py

@@ -587,3 +586,44 @@ def update_commit_author_github_data(self, obj=None, email=None, overwrite=False
                if gh_author["html_url"]:
                    author.github_profile_url = gh_author["html_url"]
                author.save(update_fields=["avatar_url", "github_profile_url"])
+
+    def fetch_most_recent_boost_dep_artifact_content(self, owner=None):


Is owner really optional? GH docs claim it's a required argument. If it is, probably should still use an empty string instead of None.

It's not optional to github, but get_artifacts falls back to self.owner if the kwarg is not supplied. I can set it to an empty string by default, though.

libraries/github.py

GregKaleka · 2024-12-06T18:07:02Z

libraries/github.py

+            return
+        return self.client.get_artifact_content(artifact["archive_download_url"])
+
+    def update_library_version_dependencies(self, owner=None, clean=False):


Same question about owner here.

GregKaleka · 2024-12-06T18:08:46Z

libraries/github.py

+            with transaction.atomic():
+                if clean:
+                    library_version.dependencies.clear()
+                library_version.dependencies.add(*dependencies)


I think this is a bit simpler.

Suggested change

with transaction.atomic():

if clean:

library_version.dependencies.clear()

library_version.dependencies.add(*dependencies)

if clean:

library_version.dependencies.set(*dependencies)

else:

library_version.dependencies.add(*dependencies)

libraries/management/commands/update_library_version_dependencies.py

GregKaleka · 2024-12-06T18:12:25Z

libraries/models.py

@@ -378,6 +378,12 @@ class LibraryVersion(models.Model):
    deletions = models.IntegerField(default=0)
    files_changed = models.IntegerField(default=0)
    cpp_standard_minimum = models.CharField(max_length=50, blank=True, null=True)
+    dependencies = models.ManyToManyField(
+        "Library",


Above, the FK to library uses libraries.Library. Doesn't really matter, but we should probably be consistent.

GregKaleka · 2024-12-06T18:16:38Z

libraries/tests/fixtures.py

+    return """Dependencies for version boost-1.33.0
+Dependencies for version boost-1.34.0
+Dependencies for version boost-1.35.0
+algorithm -> concept_check config detail logic numeric~conversion
+Dependencies for version boost-1.85.0
+algorithm -> array assert bind concept_check config core
+numeric~conversion -> array
+callable_traits ->"""


You can use textwrap.dedent to make this a bit nicer.

Suggested change

return """Dependencies for version boost-1.33.0

Dependencies for version boost-1.34.0

Dependencies for version boost-1.35.0

algorithm -> concept_check config detail logic numeric~conversion

Dependencies for version boost-1.85.0

algorithm -> array assert bind concept_check config core

numeric~conversion -> array

callable_traits ->"""

return dedent(

"""\

Dependencies for version boost-1.33.0

Dependencies for version boost-1.34.0

Dependencies for version boost-1.35.0

algorithm -> concept_check config detail logic numeric~conversion

Dependencies for version boost-1.85.0

algorithm -> array assert bind concept_check config core

numeric~conversion -> array

callable_traits ->"""

)

Never used that before. Good to know.

GregKaleka · 2024-12-06T18:33:02Z

libraries/tests/test_github.py

+    assert lv.dependencies.all().count() == 5
+    lv = LibraryVersion.objects.get(
+        library__key="algorithm", version__name="boost-1.85.0"
+    )
+    assert lv.dependencies.all().count() == 6
+    # callable traits is in the file but has no dependencies
+    lv = LibraryVersion.objects.get(
+        library__key="callable_traits", version__name="boost-1.85.0"
+    )
+    assert lv.dependencies.all().count() == 0
+    lv = LibraryVersion.objects.get(
+        library__key="numeric/conversion", version__name="boost-1.85.0"
+    )
+    assert lv.dependencies.all().count() == 1


Suggested change

assert lv.dependencies.all().count() == 5

lv = LibraryVersion.objects.get(

library__key="algorithm", version__name="boost-1.85.0"

)

assert lv.dependencies.all().count() == 6

# callable traits is in the file but has no dependencies

lv = LibraryVersion.objects.get(

library__key="callable_traits", version__name="boost-1.85.0"

)

assert lv.dependencies.all().count() == 0

lv = LibraryVersion.objects.get(

library__key="numeric/conversion", version__name="boost-1.85.0"

)

assert lv.dependencies.all().count() == 1

assert lv.dependencies.count() == 5

lv = LibraryVersion.objects.get(

library__key="algorithm", version__name="boost-1.85.0"

)

assert lv.dependencies.count() == 6

# callable traits is in the file but has no dependencies

lv = LibraryVersion.objects.get(

library__key="callable_traits", version__name="boost-1.85.0"

)

assert lv.dependencies.count() == 0

lv = LibraryVersion.objects.get(

library__key="numeric/conversion", version__name="boost-1.85.0"

)

assert lv.dependencies.count() == 1

libraries/utils.py

GregKaleka

LGTM

brianjp93 force-pushed the 1402-deps branch 3 times, most recently from 24a1bee to 8b11c8f Compare December 6, 2024 00:29

brianjp93 marked this pull request as ready for review December 6, 2024 00:36

brianjp93 requested review from rbbeeston, GregKaleka and sdarwin December 6, 2024 00:36

brianjp93 force-pushed the 1402-deps branch 3 times, most recently from 1cd25a7 to d369068 Compare December 6, 2024 17:14

brianjp93 force-pushed the 1402-deps branch from d369068 to 7359b00 Compare December 6, 2024 17:30

GregKaleka reviewed Dec 6, 2024

View reviewed changes

Apply requested PR fixes

e211d5d

GregKaleka approved these changes Dec 13, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add functionality to track LibraryVersion dependencies #1524

Add functionality to track LibraryVersion dependencies #1524

brianjp93 commented Dec 6, 2024 •

edited

Loading

brianjp93 commented Dec 6, 2024

sdarwin commented Dec 6, 2024 •

edited

Loading

sdarwin commented Dec 6, 2024

GregKaleka left a comment

GregKaleka Dec 6, 2024

GregKaleka Dec 6, 2024

brianjp93 Dec 9, 2024

GregKaleka Dec 6, 2024

GregKaleka Dec 6, 2024

GregKaleka Dec 6, 2024

GregKaleka Dec 6, 2024

brianjp93 Dec 9, 2024

GregKaleka Dec 6, 2024

GregKaleka left a comment

Add functionality to track LibraryVersion dependencies #1524

Are you sure you want to change the base?

Add functionality to track LibraryVersion dependencies #1524

Conversation

brianjp93 commented Dec 6, 2024 • edited Loading

brianjp93 commented Dec 6, 2024

sdarwin commented Dec 6, 2024 • edited Loading

sdarwin commented Dec 6, 2024

GregKaleka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GregKaleka left a comment

Choose a reason for hiding this comment

brianjp93 commented Dec 6, 2024 •

edited

Loading

sdarwin commented Dec 6, 2024 •

edited

Loading