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

Buidling code on master results in error when executing any task #522

Closed
daniel-lohausen-consilica opened this issue Jun 9, 2023 · 3 comments
Closed

Buidling code on master results in error when executing any task #522

daniel-lohausen-consilica opened this issue Jun 9, 2023 · 3 comments
Assignees
@rbbarad

Comments

@daniel-lohausen-consilica
Copy link
Contributor

daniel-lohausen-consilica commented Jun 9, 2023 •
edited

Describe the bug
Building code on master results in error when executing any task.

To reproduce

  1. Checkout master branch
  2. Run npm install && npm run fullBuild
  3. Run node ./package/Tasks/AWSCLI/AWSCLI.js (or any other task)

The following error occurs:

internal/modules/cjs/loader.js:638
    throw err;
    ^

Error: Cannot find module '/mnt/c/Users/UI725000/git/aws-toolkit-azure-devops/build/npmcache/package/Tasks/AWSCLI/AWSCLI.js'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:636:15)
    at Function.Module._load (internal/modules/cjs/loader.js:562:25)
    at Function.Module.runMain (internal/modules/cjs/loader.js:831:12)
    at startup (internal/bootstrap/node.js:283:19)
    at bootstrapNodeJSCore (internal/bootstrap/node.js:623:3)

Same applies when publishing the module and trying in a 'real' environment.
Tested with multiple node/npm versions

Expected behavior

  • Task is running in real enviorment
  • Task will start, but then complain about missing parameters when running locally

Additional context
@daniel-lohausen-consilica
Copy link
Contributor Author

@stevejroberts, @hunterwerlla - As the main collaborators, can you help me here? I would like to open a PR for #511 but am currently blocked by this issue. Could also be a misunderstanding on my end...Would be great if you could assist.

@stevejroberts
Copy link
Contributor

@daniel-lohausen-consilica It's been several years since I last looked at or worked with this code, so I'd probably be more of a hindrance than a help at this stage. I've alerted my former colleagues in the teams that now look after this toolkit to your message, hopefully we can get you some traction.

Merged
1 task
@rbbarad rbbarad self-assigned this Jan 5, 2024
@rbbarad rbbarad mentioned this issue Jan 6, 2024
Merged
5 tasks
@rknechtel rknechtel mentioned this issue Jan 10, 2024
Closed
@rbbarad rbbarad mentioned this issue Jan 22, 2024
Merged
5 tasks
rbbarad added a commit that referenced this issue Jan 22, 2024
@rbbarad
Revert azure-pipelines-task-lib to 2.12.2 to mitigate shelljs bundlin…
51f51d3 
…g issue (#539)

### Background

This is a follow-up pr to fix the issue in #522. 

[Mark shelljs as external for esbuild bundling](#537)  attempted to address this issue, but was not a successful solution for all of our tasks, as some of our tasks need the commands present in ShellJS.

### Problem

This issue emerged with the bump of `azure-pipelines-task-lib` dependency in: #473.

Problem detailed in the open issue on `azure-pipelines-task-lib` repo: [Cannot use JS bundler because of shelljs dependency](microsoft/azure-pipelines-task-lib#942).

Essentially, the toolkit takes a dependency on `azure-pipelines-task-lib` which takes an additional dependency on ShellJS. Our toolkit uses the esbuild JS Bundler to build the extension. This fails to bundle ShellJS because it uses dynamic require statements ([problematic ShellJS code](https://github.com/shelljs/shelljs/blob/a6d1e49f180a495d83bcf67bd85782c626aae029/shell.js#L23-L26)) instead of explicit require statements, which prevents bundlers from correctly analyzing the dependencies.
-  In short, require('./a') works but require('./' + 'a') doesn't.

[ShellJS has no plans to accommodate these explicit requires statements](shelljs/shelljs#962 (comment))

### Solution

Before making any larger scale changes, let's get `azure-pipelines-task-lib` back to its last known healthy state (working version we used for last release). This PR addresses that.

### Future `azure-pipelines-task-lib` updates
Whenever we next bump the `azure-pipelines-task-lib` major version, we'll need to keep an eye on updates to microsoft/azure-pipelines-task-lib#942 for any new fixes or workarounds. Current known workarounds involve forking the ShellJS repository, making the necessary changes to ShellJS, and overriding the ShellJS dependency in `package.json` with the forked version.
- [Example of forked ShellJS with hardcoded explicit `requires` statements](Everspace/shelljs@4e2ea23)
- [Example of creating a bundler plugin with a fixed ShellJS file at compile time](https://github.com/tjosepo/azure-pipelines/blob/main/packages/esbuild-plugin-azure-pipelines-task-lib-fix/src/index.js)
@rbbarad
Copy link
Contributor

rbbarad commented Feb 1, 2024

Thanks for bringing this issue to our attention! This has been fixed in #539 - resolving this issue

@rbbarad rbbarad closed this as completed Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rbbarad rbbarad

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@stevejroberts @rbbarad @daniel-lohausen-consilica