nrwl / nx

Smart Monorepos · Fast CI
https://nx.dev
MIT License
23.65k stars 2.36k forks source link

"nx migrate" mangles binary files #22788

Closed potch closed 6 months ago

potch commented 7 months ago

Current Behavior

When running nx migrate and applying migrations, the migration scripts changed hundreds of binary files across our monorepo, corrupting them

Expected Behavior

nx migrate operates on an allowList principle, and does not modify files it does not understand.

GitHub Repo

No response

Steps to Reproduce

  1. starting with an nx@15 repository containing binary files: extensions include .als, .pkg, .npy, .plist
  2. run nx migrate latest, and apply recommended migrations
  3. observe changes to some binary files

Nx Report

Node   : 18.19.0
OS     : darwin-arm64
npm    : 8.19.4

nx                 : 18.2.3
@nx/js             : 18.2.3
@nx/jest           : 18.2.3
@nx/linter         : 18.2.3
@nx/eslint         : 18.2.3
@nx/workspace      : 18.2.3
@nx/angular        : 18.2.3
@nx/cypress        : 18.2.3
@nx/devkit         : 18.2.3
@nx/eslint-plugin  : 18.2.3
@nx/express        : 18.2.3
@nx/node           : 18.2.3
@nx/plugin         : 18.2.3
@nx/storybook      : 18.2.3
@nrwl/tao          : 18.2.3
@nx/web            : 18.2.3
@nx/webpack        : 18.2.3
typescript         : 4.9.5
---------------------------------------
Registered Plugins:
@nx-go/nx-go
---------------------------------------
Community plugins:
@compodoc/compodoc          : 1.1.19
@ngneat/spectator           : 14.0.0
@ngrx/effects               : 15.3.0
@ngrx/entity                : 15.3.0
@ngrx/router-store          : 15.3.0
@ngrx/schematics            : 15.3.0
@ngrx/store                 : 15.3.0
@ngrx/store-devtools        : 15.3.0
@nguniversal/builders       : 15.2.1
@nguniversal/common         : 15.2.1
@nguniversal/express-engine : 15.2.1
@nx-go/nx-go                : 3.0.0
@storybook/angular          : 7.6.17
apollo-angular              : 4.2.1
ng-mocks                    : 14.12.1
nx-stylelint                : 13.4.0
---------------------------------------
Local workspace plugins:
     workspace-plugin

Failure Logs

No response

Package Manager Version

No response

Operating System

Additional Information

happy to provide before/after examples of binary files if needed.

AgentEnder commented 7 months ago

Would you like to submit a PR? The changes should be done here: https://github.com/nrwl/nx/blob/master/packages/devkit/src/utils/binary-extensions.ts

potch commented 6 months ago

Would you like to submit a PR? The changes should be done here: https://github.com/nrwl/nx/blob/master/packages/devkit/src/utils/binary-extensions.ts

I'm happy to submit additions to the extension list, but I'd much prefer the logic be fully inverted, and nx only act on file extensions on an allowlist vs the current "list of files to leave alone"- for instance, we have binaries checked in without extensions.

AgentEnder commented 6 months ago

Hey @potch! I'm going to go ahead and close this out with the added binary extensions being merged. Most migrations shouldn't hit every arbitrary file, so this was likely a one-time fix and we will re-evaluate if we need to do a similar mass update in the future.

github-actions[bot] commented 5 months ago

This issue has been closed for more than 30 days. If this issue is still occuring, please open a new issue with more recent context.