moonrepo / moon

A build system and monorepo management tool for the web ecosystem, written in Rust.
https://moonrepo.dev/moon
MIT License
2.93k stars 160 forks source link

[bug] Breaking change for dependent targets in v1.30.0 #1736

Open dudicoco opened 15 hours ago

dudicoco commented 15 hours ago

Describe the bug

The behavior of moon ci triggering of affected targets was changed in v1.30.0.

There are two issues here:

  1. While this is partially a desired change in my view as it resolves https://github.com/moonrepo/moon/issues/1624, it is nonetheless a breaking change and should have been introduced behind a flag or mentioned in the release notes.
  2. It introduced a side effect of affected dependencies of a target triggering their own dependents, which is undesired in my view.

Steps to reproduce

Full reproduction is available in this branch

  1. Use moon v1.29.4
  2. Make a change to apps/backend/app-01/package.json
  3. Commit
  4. Run moon ci --base HEAD~1:
    
    $ moon --version        
    moon 1.29.4

$ moon ci --base HEAD~1 ▪▪▪▪ Gathering touched files apps/backend/app-01/package.json ▪▪▪▪ Gathering potential targets app-01:install app-02:install ▪▪▪▪ Generating action graph Target count: 2 Action count: 6 ▪▪▪▪ Running tasks installing ▪▪▪▪ root:install ▪▪▪▪ root:install (11ms) app-01:install | test ▪▪▪▪ app-01:install (cached, 5aba2e13)

SUMMARY

pass SyncWorkspace (46ms) skip SetupToolchain(system) (skipped, 5ms) pass SyncProject(system, root) (5ms) pass SyncProject(system, app-01) (5ms) pass RunTask(root:install) (15ms) pass RunTask(app-01:install) (cached, 40ms, 5aba2e13)

STATS

Actions: 5 completed (1 cached), 1 skipped Time: 113ms

5. Switch to moon v1.30.0
6. Run `moon ci --base HEAD~1`:
```sh
$ moon ci --base HEAD~1
▪▪▪▪ Gathering touched files
  apps/backend/app-01/package.json
▪▪▪▪ Gathering potential targets
  app-01:install
  app-02:install
▪▪▪▪ Generating action graph
Target count: 2
Action count: 8
▪▪▪▪ Running tasks
▪▪▪▪ root:install
  root:install | installing
▪▪▪▪ root:install (9ms)
app-01:install | test
▪▪▪▪ app-01:install (cached, 5aba2e13)
app-02:install | test
▪▪▪▪ app-02:install (cached, 009f8d7a)

 SUMMARY 

pass SyncWorkspace (33ms)
skip SetupToolchain(system) (skipped)
pass SyncProject(system, root) 
pass SyncProject(system, app-02) 
pass SyncProject(system, app-01) 
pass RunTask(root:install) (11ms)
pass RunTask(app-01:install) (cached, 30ms, 5aba2e13)
pass RunTask(app-02:install) (cached, 39ms, 009f8d7a)

 STATS 

Actions: 7 completed (2 cached), 1 skipped
   Time: 86ms

As you can see in the second run with moon v1.30.0, the root:install task which was triggered because it's a dependency of the app-01:install task, triggered its own dependents - app-02:install.

Expected behavior

I would want a way to control if a dependency triggered by upstream tasks wii trigger its own dependents.

milesj commented 4 hours ago

@dudicoco We don't run dependents for the dependencies of a task. The logic is here: https://github.com/moonrepo/moon/blob/master/crates/action-graph/src/action_graph_builder.rs#L392

We create a new RunRequirements struct with default values, which sets the dependents field to false.

I think something else may be going on, can you paste the --log trace output? Can you also paste the output of moon task-graph app-01:install --dot?