Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

regression in 19.8.4 regarding task execution order #28380

Closed
2 of 4 tasks
mikonse opened this issue Oct 9, 2024 · 4 comments
Closed
2 of 4 tasks

regression in 19.8.4 regarding task execution order #28380

mikonse opened this issue Oct 9, 2024 · 4 comments
Assignees
Labels
scope: core core nx functionality type: bug

Comments

@mikonse
Copy link

mikonse commented Oct 9, 2024

Current Behavior

When describing task dependencies via nx.json we'd expect those task dependencies to be honored (i.e. executed one after another) when running with nx run-many and parallel set to a value greater than one in the nx.json

With nx versions 20.0.0 as well as 19.8.4 and 19.5.8 (almost certainly introduced by #28227) task dependency order is not properly followed by nx.

E.g. with an nx.json like

{
  "targetDefaults": {
    "pretest": {
      "cache": true,
      "dependsOn": ["^prebuild", "prebuild"]
    },
    "test": {
      "dependsOn": ["pretest"],
      "cache": true,
    }
  }
}

And two projects (p1 and p2), where p1 defines a prebuild task and p2 depends on p1

Running nx run-many --target pretest --parallel 10 can now result in a situation where p2:pretest and p1:prebuild are executed at the same time which might lead to errors if p1:prebuild is some sort of code-generation step upon which p2 depends.

I haven't been able to come up wth a minimal example to reproduce this but the issue was introduced in all nx releases which included #28227. That PR apparently reworked the way the project graph is built quite a bit and maybe introduced some inconsistencies

Expected Behavior

The task dependency order should be adhered to as expected. Tasks which depend upon one another MUST not be executed in parallel when running via nx run-many.

Steps to Reproduce

  1. Have task dependencies between multiple projects
  2. Run via nx run-many and a large enough parallel setting
  3. Observe tasks which are dependent upon one another be executed at the same time.

Nx Report

Node           : 20.18.0
OS             : linux-x64
Native Target  : x86_64-linux
npm            : 10.8.2

nx                 : 20.0.0
@nx/js             : 20.0.0
@nx/jest           : 20.0.0
@nx/eslint         : 20.0.0
@nx/workspace      : 20.0.0
@nrwl/workspace    : 17.3.2
@nx/devkit         : 20.0.0
@nx/eslint-plugin  : 20.0.0
@nx/node           : 20.0.0
@nx/plugin         : 20.0.0
@nx/react          : 20.0.0
@nx/rollup         : 20.0.0
@nrwl/tao          : 17.3.2
@nx/web            : 20.0.0
@nx/webpack        : 20.0.0
typescript         : 5.6.2
---------------------------------------
Registered Plugins:
@nx/jest/plugin
@rs-box-apps/workspace-plugin
@nx/eslint/plugin
---------------------------------------
Community plugins:
@jscutlery/semver   : 5.3.1
@rs-temaf/nx.gitlab : 0.5.0
@rs-temaf/nx.rs-run : 0.2.0
@rscocdip/nx        : 1.1.1
---------------------------------------
Local workspace plugins:
         @rs-box-apps/workspace-plugin
---------------------------------------
The following packages should match the installed version of nx
  - @nrwl/workspace@17.3.2
  - @nrwl/tao@17.3.2

Operating System

  • macOS
  • Linux
  • Windows
  • Other (Please specify)
@jaysoo jaysoo added the scope: core core nx functionality label Oct 14, 2024
@hschmidtchen
Copy link

We are facing the same issue attempting to upgrade form 19.5.7 to 19.8.4.

Running a set of validations with yarn nx run-many --all --target=validate --parallel=1 causes a crucial build dependency task to be executed as the very last step instead of before the validation targets.

If I execute the validation targets one by one without run-many, execution does not fail.

Nx Report

Node           : 20.16.0
OS             : linux-x64
Native Target  : x86_64-linux
yarn           : 4.4.0

nx                 : 19.8.4
@nx/js             : 19.8.4
@nx/jest           : 19.8.4
@nx/linter         : 19.8.4
@nx/eslint         : 19.8.4
@nx/workspace      : 19.8.4
@nx/angular        : 19.8.4
@nx/cypress        : 19.8.4
@nx/devkit         : 19.8.4
@nx/eslint-plugin  : 19.8.4
@nx/express        : 19.8.4
@nx/nest           : 19.8.4
@nx/node           : 19.8.4
@nrwl/tao          : 19.8.4
@nx/web            : 19.8.4
@nx/webpack        : 19.8.4
typescript         : 5.4.5
---------------------------------------
Registered Plugins:
@nxlv/python
---------------------------------------
Community plugins:
@ionic/angular         : 8.3.2
@ionic/angular-toolkit : 11.0.1
@luzmo/ngx-embed       : 6.3.4
@ngneat/spectator      : 18.0.2
@ngrx/component        : 17.2.0
@ngrx/component-store  : 17.2.0
@ngrx/effects          : 17.2.0
@ngrx/entity           : 17.2.0
@ngrx/router-store     : 17.2.0
@ngrx/store            : 17.2.0
@ngrx/store-devtools   : 17.2.0
@nxlv/python           : 19.1.3
ngx-toastr             : 19.0.0
ngx-ui-loader          : 13.0.0

@xiongemi
Copy link
Collaborator

i suspect it might be related to this github issue #26929

@xiongemi
Copy link
Collaborator

also might to relate to github issue #28599

xiongemi added a commit that referenced this issue Nov 1, 2024
<!-- Please make sure you have read the submission guidelines before
posting an PR -->
<!--
https://github.com/nrwl/nx/blob/master/CONTRIBUTING.md#-submitting-a-pr
-->

<!-- Please make sure that your commit message follows our format -->
<!-- Example: `fix(nx): must begin with lowercase` -->

<!-- If this is a particularly complex change or feature addition, you
can request a dedicated Nx release for this pull request branch. Mention
someone from the Nx team or the `@nrwl/nx-pipelines-reviewers` and they
will confirm if the PR warrants its own release for testing purposes,
and generate it for you if appropriate. -->

## Current Behavior
<!-- This is the behavior we have today -->
- we introduce a feature to allow circular project dependencies to
execute tasks in pr https://github.com/nrwl/nx/pull/28227/files
- for this feature, we introduce the concept of dummy task as a
temporary placeholder so we can still generate a task graph even if
dependencies contain circular dependencies
- however, it does not handle a task graph of multiple dummy task deps.
for example:
lib1 -> lib2 -> lib3 -> lib4
if we got task graph like:
lib1:build -> lib2:dummy
lib2:dummy -> lib3:dummy
lib3:dummy -> lib4:rebuild
currently, it does not create a dependency between
lib1:build->lib4:prebuild

## Expected Behavior
<!-- This is the behavior we should expect with the changes in this PR
-->
it should link the dependency when it contains multiple level of
depending on dummy tasks

## Related Issue(s)
<!-- Please link the issue being fixed so it gets closed when this is
merged. -->

Fixes #28599 and
#28380
@xiongemi
Copy link
Collaborator

xiongemi commented Nov 1, 2024

fixed in pr #28669

@xiongemi xiongemi closed this as completed Nov 1, 2024
jaysoo pushed a commit that referenced this issue Nov 1, 2024
<!-- Please make sure you have read the submission guidelines before
posting an PR -->
<!--
https://github.com/nrwl/nx/blob/master/CONTRIBUTING.md#-submitting-a-pr
-->

<!-- Please make sure that your commit message follows our format -->
<!-- Example: `fix(nx): must begin with lowercase` -->

<!-- If this is a particularly complex change or feature addition, you
can request a dedicated Nx release for this pull request branch. Mention
someone from the Nx team or the `@nrwl/nx-pipelines-reviewers` and they
will confirm if the PR warrants its own release for testing purposes,
and generate it for you if appropriate. -->

## Current Behavior
<!-- This is the behavior we have today -->
- we introduce a feature to allow circular project dependencies to
execute tasks in pr https://github.com/nrwl/nx/pull/28227/files
- for this feature, we introduce the concept of dummy task as a
temporary placeholder so we can still generate a task graph even if
dependencies contain circular dependencies
- however, it does not handle a task graph of multiple dummy task deps.
for example:
lib1 -> lib2 -> lib3 -> lib4
if we got task graph like:
lib1:build -> lib2:dummy
lib2:dummy -> lib3:dummy
lib3:dummy -> lib4:rebuild
currently, it does not create a dependency between
lib1:build->lib4:prebuild

## Expected Behavior
<!-- This is the behavior we should expect with the changes in this PR
-->
it should link the dependency when it contains multiple level of
depending on dummy tasks

## Related Issue(s)
<!-- Please link the issue being fixed so it gets closed when this is
merged. -->

Fixes #28599 and
#28380
jaysoo pushed a commit that referenced this issue Nov 1, 2024
<!-- Please make sure you have read the submission guidelines before
posting an PR -->
<!--
https://github.com/nrwl/nx/blob/master/CONTRIBUTING.md#-submitting-a-pr
-->

<!-- Please make sure that your commit message follows our format -->
<!-- Example: `fix(nx): must begin with lowercase` -->

<!-- If this is a particularly complex change or feature addition, you
can request a dedicated Nx release for this pull request branch. Mention
someone from the Nx team or the `@nrwl/nx-pipelines-reviewers` and they
will confirm if the PR warrants its own release for testing purposes,
and generate it for you if appropriate. -->

## Current Behavior
<!-- This is the behavior we have today -->
- we introduce a feature to allow circular project dependencies to
execute tasks in pr https://github.com/nrwl/nx/pull/28227/files
- for this feature, we introduce the concept of dummy task as a
temporary placeholder so we can still generate a task graph even if
dependencies contain circular dependencies
- however, it does not handle a task graph of multiple dummy task deps.
for example:
lib1 -> lib2 -> lib3 -> lib4
if we got task graph like:
lib1:build -> lib2:dummy
lib2:dummy -> lib3:dummy
lib3:dummy -> lib4:rebuild
currently, it does not create a dependency between
lib1:build->lib4:prebuild

## Expected Behavior
<!-- This is the behavior we should expect with the changes in this PR
-->
it should link the dependency when it contains multiple level of
depending on dummy tasks

## Related Issue(s)
<!-- Please link the issue being fixed so it gets closed when this is
merged. -->

Fixes #28599 and
#28380
jaysoo pushed a commit that referenced this issue Nov 1, 2024
<!-- Please make sure you have read the submission guidelines before
posting an PR -->
<!--
https://github.com/nrwl/nx/blob/master/CONTRIBUTING.md#-submitting-a-pr
-->

<!-- Please make sure that your commit message follows our format -->
<!-- Example: `fix(nx): must begin with lowercase` -->

<!-- If this is a particularly complex change or feature addition, you
can request a dedicated Nx release for this pull request branch. Mention
someone from the Nx team or the `@nrwl/nx-pipelines-reviewers` and they
will confirm if the PR warrants its own release for testing purposes,
and generate it for you if appropriate. -->

<!-- This is the behavior we have today -->
- we introduce a feature to allow circular project dependencies to
execute tasks in pr https://github.com/nrwl/nx/pull/28227/files
- for this feature, we introduce the concept of dummy task as a
temporary placeholder so we can still generate a task graph even if
dependencies contain circular dependencies
- however, it does not handle a task graph of multiple dummy task deps.
for example:
lib1 -> lib2 -> lib3 -> lib4
if we got task graph like:
lib1:build -> lib2:dummy
lib2:dummy -> lib3:dummy
lib3:dummy -> lib4:rebuild
currently, it does not create a dependency between
lib1:build->lib4:prebuild

<!-- This is the behavior we should expect with the changes in this PR
-->
it should link the dependency when it contains multiple level of
depending on dummy tasks

<!-- Please link the issue being fixed so it gets closed when this is
merged. -->

Fixes #28599 and
#28380
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: core core nx functionality type: bug
Projects
None yet
Development

No branches or pull requests

5 participants