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

remove npx in the node 10 directory; remove npm, npx, corepack, and related node modules in the node 16 directory #3897

Merged
merged 4 commits into from
Jul 11, 2022
Merged

remove npx in the node 10 directory; remove npm, npx, corepack, and related node modules in the node 16 directory #3897

merged 4 commits into from
Jul 11, 2022

Conversation

DenisRumyantsev
Copy link
Contributor

@DenisRumyantsev DenisRumyantsev commented Jul 6, 2022 •
edited
Loading

Npm and related modules were removed in this pull request.
Npx is a broken symlink. Npx is not used anywhere in the agent. We need to remove it as well.
Tested that applied changes work as expected: Linux agent was built, configured, and launched fine.

npm, npx, corepack, and related node_modules in the externals/node16 directory also were removed.

Risks analysis checklist:

  • There are no risky dependency updates
  • Changes have been tested
  • Enough test coverage for changes and current test coverage for the agent doesn't look poor
  • We understand how the agent is working, how changes affect agent behavior
  • There are no breaking changes
  • There are no other concerns
  • I have not discovered any new uncovered test/use cases

@DenisRumyantsev DenisRumyantsev added the misc Miscellaneous Changes label Jul 6, 2022
@DenisRumyantsev DenisRumyantsev self-assigned this Jul 6, 2022
@DenisRumyantsev DenisRumyantsev marked this pull request as ready for review July 6, 2022 16:12
@DenisRumyantsev DenisRumyantsev requested a review from a team July 6, 2022 16:25
@duyork
Copy link

duyork commented Jul 6, 2022

This is incorrect. We need the NPX and Node_Modules as they are used for ADO Agent installation on Linux, at least for the current processes we're using.

Has the process for installing the agent on Linux changed?

@alexander-smolyakov
Copy link
Contributor

This is incorrect. We need the NPX and Node_Modules as they are used for ADO Agent installation on Linux, at least for the current processes we're using.

Has the process for installing the agent on Linux changed?

@duyork could you please clarify how exactly you are using NPX and Node_Modules to install the pipeline agent?

@duyork
Copy link

duyork commented Jul 7, 2022

This is incorrect. We need the NPX and Node_Modules as they are used for ADO Agent installation on Linux, at least for the current processes we're using.
Has the process for installing the agent on Linux changed?

@duyork could you please clarify how exactly you are using NPX and Node_Modules to install the pipeline agent?

OK after digging into the error we're seeing; it looks like it was just due to copying the NPX symlink to a file that doesn't exist is the actual root cause here.

This PR looks good to me now, thank you.

@DenisRumyantsev DenisRumyantsev changed the title remove npx remove npx in the node 10 directory; remove npm, npx, corepack, and related node modules in the node 16 directory Jul 7, 2022
@DenisRumyantsev
Copy link
Contributor Author

The find . -xtype l command did not detect broken symlinks in the agent build.

@AndreyIvanov42
Copy link
Contributor

Tested, working fine

@DenisRumyantsev DenisRumyantsev merged commit 362dfd2 into master Jul 11, 2022
alexander-smolyakov pushed a commit that referenced this pull request Jul 11, 2022
@DenisRumyantsev @AndreyIvanov42 @alexander-smolyakov
remove npx in the node 10 directory; remove npm, npx, corepack, and r…
68ff736 
…elated node modules in the node 16 directory (#3897)

* remove npx symlink from node10

* remove npm, npx, and corepack from node16

* add a comment

Co-authored-by: AndreyIvanov42 <93121155+AndreyIvanov42@users.noreply.github.com>
@alexander-smolyakov alexander-smolyakov mentioned this pull request Jul 11, 2022
Merged
alexander-smolyakov added a commit that referenced this pull request Jul 12, 2022
@alexander-smolyakov @AndreyIvanov42 @DenisRumyantsev
Patch for Agent v2.206.0 - Node 16 execution handler & Remove unuse…
ef1261a 
…d folders from Node distribution (#3899)

* Added node16 execution handler (#3861)
* remove npx in the node 10 directory; remove npm, npx, corepack, and related node modules in the node 16 directory (#3897)
* Update dotnet scripts (#3895)
* Agent Release 2.206.1

Co-authored-by: AndreyIvanov42 <93121155+AndreyIvanov42@users.noreply.github.com>
Co-authored-by: Anatolii Bolshakov (Akvelon INC) <v-anbols@microsoft.com>
Co-authored-by: Andrey Ivanov <v-andivanov@microsoft.com>
Co-authored-by: Denis Rumyantsev <mr.denis.rumyantsev@gmail.com>
Co-authored-by: Lilia Sabitova <lilia.sabitova@ua.akvelon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AndreyIvanov42 AndreyIvanov42 AndreyIvanov42 approved these changes

@wojgasior wojgasior wojgasior approved these changes

@max-zaytsev max-zaytsev max-zaytsev approved these changes

@alexander-smolyakov alexander-smolyakov alexander-smolyakov approved these changes

Assignees

@DenisRumyantsev DenisRumyantsev

Labels
misc Miscellaneous Changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@DenisRumyantsev @duyork @alexander-smolyakov @AndreyIvanov42 @max-zaytsev @wojgasior