Skip to content

Commit 954b8ed

Browse files
committed
feat(model): Add indicator nodes for cycles in the dependency graph
When during construction of the dependency graph a cycle in the dependencies is detected, add a special marker node for the package that closes the cycle that does not have any dependencies. That way, the referenced package is still visible in the graph, but the graph remains free of cycles. Signed-off-by: Oliver Heger <oliver.heger@bosch.com>
1 parent 40ef461 commit 954b8ed

3 files changed

Lines changed: 188 additions & 54 deletions

File tree

model/src/main/kotlin/DependencyGraph.kt

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,12 @@ data class DependencyGraphNode(
367367
* A list of [Issue]s that occurred handling this dependency.
368368
*/
369369
val issues: List<Issue> = emptyList()
370-
)
370+
) {
371+
/**
372+
* Test whether this node is a special marker node to indicate a cycle in the dependency graph.
373+
*/
374+
fun isCycleIndicator(): Boolean = isCycleIndicatorIndex(fragment)
375+
}
371376

372377
/**
373378
* A data class representing an edge in the dependency graph.
@@ -383,6 +388,24 @@ data class DependencyGraphEdge(
383388
val to: Int
384389
)
385390

391+
/**
392+
* Constant of a bit mask used to add information about cycles to a fragment index. A node acting as cycle indicator
393+
* has a fragment index greater than this value.
394+
*/
395+
private const val CYCLE_INDICATOR_MASK = 0xFFFF
396+
397+
/**
398+
* Check whether the given [fragmentIndex] indicates a marker node for a cycle in the dependency graph.
399+
*/
400+
fun isCycleIndicatorIndex(fragmentIndex: Int): Boolean = fragmentIndex > CYCLE_INDICATOR_MASK
401+
402+
/**
403+
* Return the plain index of the fragment referenced by the given [fragmentIndex] stripping any information about
404+
* cycles. If the passed in [fragmentIndex] is not an indicator index for cycles, it is returned as is; otherwise,
405+
* the fragment component is extracted.
406+
*/
407+
fun extractFragment(fragmentIndex: Int): Int = fragmentIndex and CYCLE_INDICATOR_MASK
408+
386409
/**
387410
* Convert this [DependencyReference] to a [DependencyGraphNode].
388411
*/

model/src/main/kotlin/utils/DependencyGraphBuilder.kt

Lines changed: 81 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ package org.ossreviewtoolkit.model.utils
2121

2222
import java.util.LinkedList
2323

24+
import org.apache.logging.log4j.kotlin.logger
25+
2426
import org.ossreviewtoolkit.model.DependencyGraph
2527
import org.ossreviewtoolkit.model.DependencyGraphEdge
2628
import org.ossreviewtoolkit.model.DependencyGraphNode
@@ -30,6 +32,8 @@ import org.ossreviewtoolkit.model.Issue
3032
import org.ossreviewtoolkit.model.Package
3133
import org.ossreviewtoolkit.model.PackageLinkage
3234
import org.ossreviewtoolkit.model.RootDependencyIndex
35+
import org.ossreviewtoolkit.model.extractFragment
36+
import org.ossreviewtoolkit.model.isCycleIndicatorIndex
3337

3438
/**
3539
* Internal class to represent the result of a search in the dependency graph. The outcome of the search
@@ -138,6 +142,12 @@ class DependencyGraphBuilder<D>(
138142
*/
139143
private val references = mutableMapOf<DependencyReference, D>()
140144

145+
/**
146+
* A map for generating fragment indices for marker nodes that are added to the graph if cycles are detected. The
147+
* key is a fragment index, the value is the number of cycles found in this fragment.
148+
*/
149+
private val cycleCounts = mutableMapOf<Int, Int>()
150+
141151
/**
142152
* Add the given [dependency] for the scope with the given [scopeName] to this builder. This function needs to be
143153
* called for all direct dependencies of all scopes. That way the builder gets sufficient information to construct
@@ -232,7 +242,8 @@ class DependencyGraphBuilder<D>(
232242
scopeName: String,
233243
dependency: D,
234244
transitive: Boolean,
235-
processed: Set<D>
245+
processed: Set<D>,
246+
inCycle: Boolean = false
236247
): DependencyReference? {
237248
val id = dependencyHandler.identifierFor(dependency)
238249
val issues = dependencyHandler.issuesFor(dependency).toMutableList()
@@ -248,13 +259,13 @@ class DependencyGraphBuilder<D>(
248259
scopeName,
249260
dependency,
250261
issues,
251-
transitive,
252-
processed
262+
processed,
263+
inCycle
253264
)
254265
}
255266

256267
is DependencyGraphSearchResult.Incompatible ->
257-
insertIntoNewFragment(id, index, scopeName, dependency, issues, transitive, processed)
268+
insertIntoNewFragment(id, index, scopeName, dependency, issues, processed, inCycle)
258269
}
259270

260271
return updateScopeMapping(scopeName, ref, transitive)
@@ -321,67 +332,85 @@ class DependencyGraphBuilder<D>(
321332
}
322333

323334
/**
324-
* Add a new fragment to the dependency graph for the [dependency] with the given [id] and [index], which may be
325-
* [transitive] and belongs to the scope with the given [scopeName]. This function is called for dependencies that
326-
* cannot be added to already existing fragments. Therefore, create a new fragment and add the [dependency] to it,
327-
* together with its own dependencies. Store the given [issues] for the dependency. Use [processed] to detect
328-
* cycles.
335+
* Add a new fragment to the dependency graph for the [dependency] with the given [id] and [index], which belongs
336+
* to the scope with the given [scopeName]. This function is called for dependencies that cannot be added to
337+
* already existing fragments. Therefore, create a new fragment and add the [dependency] to it, together with its
338+
* own dependencies. Store the given [issues] for the dependency. Use [processed] and [inCycle] to detect cycles.
329339
*/
330340
private fun insertIntoNewFragment(
331341
id: Identifier,
332342
index: Int,
333343
scopeName: String,
334344
dependency: D,
335345
issues: List<Issue>,
336-
transitive: Boolean,
337-
processed: Set<D>
346+
processed: Set<D>,
347+
inCycle: Boolean
338348
): DependencyReference? {
339349
val fragmentMapping = mutableMapOf<Int, DependencyReference>()
340350
val dependencyIndex = RootDependencyIndex(index, referenceMappings.size)
341351
referenceMappings += fragmentMapping
342352

343-
return insertIntoGraph(id, dependencyIndex, scopeName, dependency, issues, transitive, processed)
353+
return insertIntoGraph(id, dependencyIndex, scopeName, dependency, issues, processed, inCycle)
344354
}
345355

346356
/**
347357
* Insert the [dependency] with the given [id] and [RootDependencyIndex][index], which belongs to the scope with
348-
* the given [scopeName] and may be [transitive] into the dependency graph. Insert the dependencies of this
349-
* [dependency] recursively. Create a new [DependencyReference] for the dependency and initialize it with the list
350-
* of [issues]. Use the given [processed] set to figure out cycles in the dependency graph. If such a cycle is
351-
* detected, stop processing of further dependencies and return *null*.
358+
* the given [scopeName] into the dependency graph. Insert the dependencies of this [dependency] recursively.
359+
* Create a new [DependencyReference] for the dependency and initialize it with the list of [issues]. Use the given
360+
* [processed] set and the [inCycle] flag to deal with cycles in the dependency graph. If such a cycle is detected,
361+
* add a marker node for the affected package (with a special fragment index) that does not have further
362+
* dependencies.
352363
*/
353364
private fun insertIntoGraph(
354365
id: Identifier,
355366
index: RootDependencyIndex,
356367
scopeName: String,
357368
dependency: D,
358369
issues: List<Issue>,
359-
transitive: Boolean,
360-
processed: Set<D>
370+
processed: Set<D>,
371+
inCycle: Boolean
361372
): DependencyReference? {
362373
val nextProcessed = processed + dependency
363-
val transitiveDependencies = dependencyHandler.dependenciesFor(dependency)
364-
.mapNotNullTo(mutableSetOf()) { dep ->
365-
dep.takeUnless { it in nextProcessed }?.let {
366-
addDependencyToGraph(scopeName, dep, transitive = true, nextProcessed + dep)
374+
val transitiveDependencies = if (inCycle) {
375+
emptySet()
376+
} else {
377+
dependencyHandler.dependenciesFor(dependency)
378+
.mapNotNullTo(mutableSetOf()) { dep ->
379+
addDependencyToGraph(
380+
scopeName,
381+
dep,
382+
transitive = true,
383+
nextProcessed + dep,
384+
inCycle = dep in nextProcessed
385+
)
367386
}
387+
}
388+
389+
val fragmentIndex = if (inCycle) {
390+
fragmentIndexForCycle(index.fragment).also {
391+
logger.info { "Cycle detected in dependency graph for package '${id.toCoordinates()}'." }
392+
logger.info { "Adding marker node with fragment index $it." }
368393
}
394+
} else {
395+
index.fragment
396+
}
369397

370398
val fragmentMapping = referenceMappings[index.fragment]
371-
if (index.root in fragmentMapping) {
372-
// If this point is reached, the package has already been inserted when processing its dependencies.
373-
// This means that there is a cyclic dependency. To handle this case correctly, the insert operation has
374-
// to be started anew.
375-
return addDependencyToGraph(scopeName, dependency, transitive, nextProcessed)
399+
if (index.root in fragmentMapping && !startOfCycle(index)) {
400+
// If this point is reached, the package has already been inserted when processing its dependencies, but
401+
// no cycle has been found. To handle this case correctly, the insert operation has to be restarted; this
402+
// should then find the already inserted node in the graph.
403+
return addDependencyToGraph(scopeName, dependency, transitive = true, processed + dependency)
376404
}
377405

378406
val ref = DependencyReference(
379407
pkg = index.root,
380-
fragment = index.fragment,
408+
fragment = fragmentIndex,
381409
dependencies = transitiveDependencies,
382410
linkage = dependencyHandler.linkageFor(dependency),
383411
issues = issues
384412
)
413+
385414
fragmentMapping[index.root] = ref
386415
references[ref] = dependency
387416

@@ -392,6 +421,16 @@ class DependencyGraphBuilder<D>(
392421
return ref
393422
}
394423

424+
/**
425+
* Check whether the given [index] refers to a node that is the starting point of a cycle. This is the case if
426+
* there is a corresponding cycle indicator node.
427+
*/
428+
private fun startOfCycle(index: RootDependencyIndex): Boolean =
429+
references.keys.any {
430+
it.pkg == index.root && isCycleIndicatorIndex(it.fragment) &&
431+
extractFragment(it.fragment) == index.fragment
432+
}
433+
395434
/**
396435
* Construct a [Package] for the given [id] that corresponds to the given [dependency]. If the package is already
397436
* available, nothing has to be done. Otherwise, create a new one and add it to the set managed by this object. If
@@ -419,6 +458,20 @@ class DependencyGraphBuilder<D>(
419458

420459
return ref
421460
}
461+
462+
/**
463+
* Generate a special fragment index for a node that indicates a cycle in the dependency graph. The combination of
464+
* package ID and fragment index must be unique for all nodes in the graph. Therefore, marker nodes to indicate
465+
* cycles require some special values here. Cycle indicator nodes have a fragment index greater than 65536 where
466+
* the upper bits are just a counter to generate a unique value. Use the provided original [fragmentIndex] to
467+
* obtain the corresponding counter. To find the fragment index of the original node (that starts the cycle), the
468+
* upper 16 bits just have to be masked out.
469+
*/
470+
private fun fragmentIndexForCycle(fragmentIndex: Int): Int {
471+
val counter = cycleCounts.getOrDefault(fragmentIndex, 0) + 1
472+
cycleCounts[fragmentIndex] = counter
473+
return fragmentIndex + (counter shl 16)
474+
}
422475
}
423476

424477
/**

0 commit comments

Comments
 (0)