Skip to content

Conversation

moritzx22
Copy link

@moritzx22 moritzx22 commented Sep 21, 2025

This Pull request will reflect that dynamic dependencies may change the graph during recomputeDirty of nodes. A newly added dyndep output may be dirty and can cause other targets to be dirty as well. This shall take effect if those nodes has been visited before. A new algorithm to iterate over the graph to make nodes dirty has been added.
For a detailed problem description, see issue#2635

This commit will reflect that dynamic dependencies may change the graph during recomputeDirty of nodes.
A newly added dyndep output may be dirty and can cause other targets to be dirty as well. This shall take effect if those nodes has been visited before.
A new algorithm to iterate over the graph to make nodes dirty has been added.
@moritzx22
Copy link
Author

moritzx22 commented Sep 21, 2025

Test Bench

Example build file for testing:

rule cp
  command = cp $in $out

rule cp_dynB
  command = cp $in $out && touch $stamp && touch $output
  dyndep = $stamp.dd

rule mkdyn
  command = printf 'ninja_dyndep_version = 1\nbuild  $st_out | $dyn_out: dyndep | $dyn_in\n' > $stamp.dd

build b1: cp  b
build b2: cp  b1
build b3: cp  b2
build b4: cp  b3
build b5: cp  b4

build a1: cp  a
build a2: cp  a1
build a3: cp_dynB a2 || stamp-a.dd
  stamp = stamp-a
  output = b
build a4: cp  a3
build a5: cp  a4

build stamp-a.dd: mkdyn
  stamp = stamp-a
  dyn_out = b
  st_out = a3

now do the following commands:

$ touch a b && ninja
$ touch a && ninja b5 a5
$ touch a && ninja b5 a5

In the master branch are issues, not the required processes are always started. The solution of this Pull Request works well.

Analysis

the next figure reports the dependency graph without dyndep[noDyndep]:
noDyndep
[noDyndep]

The next figure reports the dependency graph with dyndep[complete], using this Pull Request, and target a is touched:
complete
[complete]

The red means dirty, blue means clean.

The next figure reports the dependency graph using master branch, and target a is touched:
master
[master]

It can be easily seen that targets with b are not considered to be dirty by the master branch.

Scanning Algorithm Master branch

  1. First target b5 will be added. There are still 2 independent graphs a and b, see figure [noDyndep]. Nothing in graph b is dirty, and nothing is detected.
  2. Second target a5 is added. Now the dyndep is added, the graph dependency changes. Only nodes with an a are visited and are marked as dirty. The targets with an b are not marked as dirty.
  3. no further action, as b5 has already been added, any b is missed to be marked as dirty.

The algorithm does not reflect that the graph may change. This issue occurs if a dyndep adds an output target (here b) and if this target is dirty and has other outputs.

Proposed Scanning Algorithm enhancement

if the dyndep is applied, there is an additional iterating algorithm across the graph following the outputs. The outputs are only followed if they have been visited before. Therefore a node can now be dirty, clean and spare. This does prevent to mark nodes as dirty which are not requested to be built.
The command $ touch a && ninja a5 would not follow the b targets.

The additional iterating over the graph starts here: iterating

If dyndep applied, the algorithm will check any input instead of only one. Dyndep may have changed it.
@moritzx22
Copy link
Author

The recently pushed commit will address issue#2641, it is a simular dyndep issue as #2653.

Test Bench

Similar as reported in the issue, just one target was added abc

rule dyndep
  command = printf 'ninja_dyndep_version = 1\nbuild stamp | other: dyndep\n' > $out

rule build_with_dyndep
  command = touch $out other
  dyndep = $out.dd

rule depends_on_dyndep
  command = printf '%s: other\n' $out > $out.d && cp other $out
  depfile = $out.d
  
rule cp
  command = cp $in $out

build stamp.dd: dyndep
build stamp: build_with_dyndep || stamp.dd
build abc: cp stamp
build copy: depends_on_dyndep || abc

Following graph with proposed solution
a
Following graph without proposed solution (copy is not dirty and therefore not build)
a

blue is not build, red is build.
The following commands has been used to generate the graphics.

$ ninja -v -d explain
adding spaces to the build_with_dyndep command
$ ninja -v -d explain

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.

1 participant