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

"nx migrate" mangles binary files #22788

Closed
1 of 4 tasks
potch opened this issue Apr 11, 2024 · 3 comments
Closed
1 of 4 tasks

"nx migrate" mangles binary files #22788

potch opened this issue Apr 11, 2024 · 3 comments
Assignees
Labels

Comments

@potch
Copy link

potch commented Apr 11, 2024

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

  • macOS
  • Linux
  • Windows
  • Other (Please specify)

Additional Information

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

@AgentEnder
Copy link
Member

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
Copy link
Author

potch commented Apr 22, 2024

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
Copy link
Member

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.

FrozenPandaz pushed a commit that referenced this issue May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants