Skip to content

Conversation

rezib
Copy link
Collaborator

@rezib rezib commented Sep 9, 2025

This commit introduces the graph modules with class to handle a graph of reverse dependencies between packages in a Rift project.

When a package is built, especially when its version has changed, it is important to verify that it does not break build and tests of other packages that depends on it (aka. reverse dependencies).

With this new feature, as soon as dependency_tracking option is enabled in project configuration, the graph of reverse dependencies is automatically created with rift build and validate actions, unless --skip-deps option is set on command line. Dependencies are either found in packages info.yaml metadata file or by parsing subpackages (including their provides) and build requirements out of RPM spec files as a fallback.

Rift then solves the reverse dependencies of the packages provided by users with build and validate actions to determine the packages that should be rebuilt or validated consequently.

Unit tests are added to validate the code introduced for this feature.

This commit introduces the graph modules with class to handle a graph of
reverse dependencies between packages in a Rift project.

When a package is built, especially when its version has changed, it is
important to verify that it does not break build and tests of other
packages that depends on it (aka. reverse dependencies).

With this new feature, as soon as dependency_tracking option is enabled
in project configuration, the graph of reverse dependencies is
automatically created with rift build and validate actions, unless
--skip-deps option is set on command line. Dependencies are either found
in packages info.yaml metadata file or by parsing subpackages (including
their provides) and build requirements out of RPM spec files as a
fallback.

Rift then solves the reverse dependencies of the packages provided by
users with build and validate actions to determine the packages that
should be rebuilt or validated consequently.

Unit tests are added to validate the code introduced for this feature.
@rezib rezib requested review from qa-cea and valeriyoann and removed request for qa-cea September 9, 2025 09:46
@rezib rezib requested a review from qa-cea September 23, 2025 09:03
Comment on lines +1110 to +1111
Return the first index in result of packages in provided build
requirements list.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggest: Return the index of the first package in result that appear as a build requirement in new_build_requirements

@@ -0,0 +1,256 @@
#
# Copyright (C) 2024 CEA
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: 2025

Comment on lines +55 to +61
# Parse buildrequires string in spec file to discard explicit versions
# enforcement.
self.build_requires = [
value.group(1)
for value
in re.finditer(r"(\S+)( (>|>=|=|<=|<) \S+)?", spec.buildrequires)
]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: why discard these ? If a package requires another package to be under a certain version, and it gets updated, the build will break right ? So detecting that at update seems a good idea ?

Comment on lines +102 to +103
logging.info(" requires: %s", node.build_requires)
logging.info(" subpackages: %s", str(node.subpackages))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggest: switch the two, and replace subpackages with provides

logging.info(" requires: %s", node.build_requires)
logging.info(" subpackages: %s", str(node.subpackages))
logging.info(
" rdeps: %s", str([rdep.package.name for rdep in node.rdeps])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggest: replace rdeps: %s with is required by: %s

self.config, self.staff, self.modules, args
)
self.assertEqual(
[pkg.name for pkg in pkgs], ['libone', 'my-software']
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: how did the my-software pkg get here ? It isn't in the list of packages on line 1110

# return an empty list.
self.assertEqual(pkgs, [])

def test_get_packages_to_build_package_order(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggest: add another test to verify cyclic dependencies

@@ -0,0 +1,350 @@
#
# Copyright (C) 2020 CEA
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: 2025

try:
package.load()
except FileNotFoundError as err:
logging.warning("Skipping package %s unable to load: %s",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggest: add single quotes around the package name to make it easier to read

""" Test graph dump """
pkg_name = 'fake'
self.make_pkg(name=pkg_name)
package = Package(pkg_name, self.config, self.staff, self.modules)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: where is this package going to ? The variable is not used in this function, so is the test succeeding because of leftovers from other tests ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants