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.

Sign up for GitHub

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

Jump to bottom

[BUG](arborist) Lifecycle Scripts of file: dependencies do not run #4277

Closed
nVitius opened this issue Aug 3, 2021 · 1 comment · Fixed by #4875
Closed

[BUG](arborist) Lifecycle Scripts of file: dependencies do not run #4277

nVitius opened this issue Aug 3, 2021 · 1 comment · Fixed by #4875
Assignees
@ruyadorno
Labels
Bug thing that needs fixing Priority 2 secondary priority issue ws:arborist Related to the arborist workspace

Comments

@nVitius
Copy link

nVitius commented Aug 3, 2021

What / Why

Arborist skips the lifecycle scripts of packages installed from a local path

When

  • Running npm install (npm v7.2.0/arborist v2.7.1)

Where

  • n/a

How

Current Behavior

Arborist runs the lifecycle scripts for a package in the _build step of reify.
I spent some time stepping through the debugger to see where the issue was happening and found this:
https://github.com/npm/arborist/blob/c5e1618a0059b37ee878b73553d7dc47bf3fa976/lib/arborist/reify.js#L973
I'm not sure what target.fsTop is here or how it is calculated, but it seems that any packages installed with a local path, are skipped from the rebuild because of this condition.

Removing the condition allows the scripts to run correctly.

I will gladly work on this if someone could point me in the right direction.

Steps to Reproduce

  • install a package with a local path (e.g. "dep-postinstall-test": "file:../dep-postinstall-test")
  • ensure the package (dep-postinstall-test) has a postinstall script
  • run npm i --foreground-scripts (to verify the output of the postinstall)

Expected Behavior

  • postinstall scripts of all dependencies should run

Who

  • n/a

References

  • n/a
@fritzy fritzy transferred this issue from npm/arborist Jan 20, 2022
@fritzy fritzy added Needs Triage needs review for next steps ws:arborist Related to the arborist workspace labels Jan 20, 2022
@fritzy fritzy changed the title [BUG] Lifecycle Scripts of file: dependencies do not run [BUG](arborist) Lifecycle Scripts of file: dependencies do not run Jan 20, 2022
@ruyadorno ruyadorno added Priority 2 secondary priority issue Bug thing that needs fixing and removed Needs Triage needs review for next steps labels May 9, 2022
@ruyadorno ruyadorno self-assigned this May 10, 2022
ruyadorno added a commit to ruyadorno/cli that referenced this issue May 10, 2022
@ruyadorno
fix(arborist): link deps lifecycle scripts
0d95e7b 
- Fixes running proper lifecycle scripts for linked deps and
  workspaces.
- Added test to validate lifecycle scripts don't run twice
  for linked deps
- Tweaked "reify workspaces bin files" test to also validate
  proper lifecycle scripts ran before check for linked bins.
- Tweaked reify test running lifecycle scripts of unchanged link
  nodes to also validate that the install lifecycle scripts are
  also called.

Fixes: npm#4277
Fixes: npm#4552
Fixes: npm/statusboard#439
Relates to: npm#2905
ruyadorno added a commit to ruyadorno/cli that referenced this issue May 10, 2022
@ruyadorno
fix(arborist): link deps lifecycle scripts
0f8f932 
- Fixes running proper lifecycle scripts for linked deps and
  workspaces.
- Added test to validate lifecycle scripts don't run twice
  for linked deps
- Tweaked "reify workspaces bin files" test to also validate
  proper lifecycle scripts ran before check for linked bins.
- Tweaked reify test running lifecycle scripts of unchanged link
  nodes to also validate that the install lifecycle scripts are
  also called.

Fixes: npm#4277
Fixes: npm#4552
Fixes: npm/statusboard#439
Relates to: npm#2905
@ruyadorno ruyadorno mentioned this issue May 10, 2022
Merged
wraithgar pushed a commit that referenced this issue May 10, 2022
@ruyadorno
fix(arborist): link deps lifecycle scripts (#4875)
5a50762 
- Fixes running proper lifecycle scripts for linked deps and
  workspaces.
- Added test to validate lifecycle scripts don't run twice
  for linked deps
- Tweaked "reify workspaces bin files" test to also validate
  proper lifecycle scripts ran before check for linked bins.
- Tweaked reify test running lifecycle scripts of unchanged link
  nodes to also validate that the install lifecycle scripts are
  also called.

Fixes: #4277
Fixes: #4552
Fixes: npm/statusboard#439
Relates to: #2905
@ruyadorno
Copy link
Contributor

should be fixed in npm@8.10.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ruyadorno ruyadorno

Labels
Bug thing that needs fixing Priority 2 secondary priority issue ws:arborist Related to the arborist workspace
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(arborist): link deps lifecycle scripts ruyadorno/cli
3 participants
@fritzy @ruyadorno @nVitius