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

migrate fails if metadata process fails for one of the packages #20329

Open
1 of 4 tasks
gkamperis opened this issue Nov 18, 2023 · 4 comments
Open
1 of 4 tasks

migrate fails if metadata process fails for one of the packages #20329

gkamperis opened this issue Nov 18, 2023 · 4 comments
Assignees
Labels
scope: core core nx functionality type: bug

Comments

@gkamperis
Copy link

gkamperis commented Nov 18, 2023

Current Behavior

Trying to migrate from 16.5.1 to 17.1.2.

nx migrate latest (atm 17.1.2) fails on our repo because "fetching metadata" (as the console suggests) fails on esbuild.

This is because esbuild has a postinstall script which runs the esbuild.exe.

Note that we are using a private package registry and in a corporate environment our "temp" folder does not have permissions for code execution.

This is why the postinstall of esbuild fails - as it is trying to execute in the temp directory.
This causes the whole migration process to fail.

According to esbuild this is not vital for the correct function of the lib.
evanw/esbuild#1621
However, this is not important because the "fetching metadata" phase of the migration does not really install/use the package itself.
For whatever reason the migration process fetches the packages it deems required in a temp location, does whatever it needs to do and then these are discarded.

There is a lot of redundancy going on in the migration process that causes it to be slow and cumbersome.

  • The migration process does not check if I already have the packages installed. It could/should get the metadata it needs directly with no extra installation if I already have the package installed.
  • The fetched packages are discarded. I will have to install them again in the repo to continue my migration anyway.
  • In the console I can see packages are fetched twice

So, the easiest fix I guess is to prevent failure of the "fetching metadata" steps/stage and allow the user to handle correct installation of the packages when the migration really happens in the repo itself.

The best overall improvement will be to get the metadata from the local repo if they are there, and then go to the registry if not there.
There are a lot of cases where I can install the packages in my repo without issue because I have permissions on certain directories and I will be able to save a lot of time vs fetching packages again and again.

Another solution, the one I used to workaround the issue, is to allow us to pass config to the tmp package you are using to create temp directories.
https://www.npmjs.com/package/tmp
I edited your js to set the config to the directory I had exec permissions.
This fetched and executed the esbuild postinstall without failure.

Not sure what the best way of configuring this would be.
Seems like these have to be passed from the migrate command all the way down which kind of does not make total sense

EDIT: inspired by the comments below and existing setup for other flows, an environment variable that allows to set the temp path location would solve the problem too.

Expected Behavior

I would expect the migration process to not fail completely if "metadata" cannot be fetched for any package.

Would be great if the metadata can be fulfilled from the local repo packages to speed things up and make it more resilient.

GitHub Repo

No response

Steps to Reproduce

  1. Remove execution permissions from the temp dir location
  2. migration where esbuild is required (ie angular project) fails as postinstall fails

Nx Report

Nx 17.1.2

Failure Logs

No response

Package Manager Version

No response

Operating System

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

Additional Information

No response

@AgentEnder AgentEnder added the scope: core core nx functionality label Nov 20, 2023
@elizww
Copy link

elizww commented Nov 21, 2023

I have the same issue, I think this is what we had before #17588, it wasn't resolved there as well,

They suggested NX_MIGRATE_SKIP_INSTALL=true - didn't work for me

@gkamperis
Copy link
Author

gkamperis commented Nov 22, 2023

I tried that too but it did not work. But for my particular scenario an env-variable that specifies the temp directory location would resolve the issue

@gkamperis
Copy link
Author

anyone from the core team watching?

@gkamperis
Copy link
Author

still an issue...

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

4 participants