* Fix acquisition error on non fully qualified version
The new, unreleased code for getting a feature band of a sdk version try catches when it tries to get the band. It will not throw if there is no band in the string which can be expected in certain cases, but it still posts an error to the event stream. Posting an error to the event stream causes the error to bubble up to the user even if it is caught. We need to take some additional action here so the user doesn't see an error popup for something that's not an error. This is blocking our release with all of the findPath features.
Resolves https://github.com/dotnet/vscode-dotnet-runtime/issues/2015
Version updates are to update the shas of the internal packages which got updated.
* Update imports
* remove extra else
* Fix incorrect PATH echo
* Add Architecture Check into Find PATH API
We had a discussion about how Mac users tend to need the arm64 runtime and often have the x64 host installed on their PATH. This was a big concern. With global installs at least we can still rely on the output from `dotnet --info` for now, so we decided to add this back in.
* Fix test, the runtime install does not have an arch printed so we cant quite use it to check properly
* Fix test
* Fix test
* undo comment out
* Fix a very terrible bug due to a typo
* code cleanup
By passing recursive:true to all mkdirSync calls we protected against
issues that may arise from parent directories not already existing.
Fix#1985
Co-authored-by: Andy Sleigh <andy.sleigh@healthhero.com>
* Permit Installing .NET via a package.json file
Extensions that rely on .NET often have a slow startup time due to our existing API.
For extensions that want the .NET runtime to be installed as soon as possible, they can now also make an API request in their package.json.
They should add a [IDotnetAcquireContext](https://github.com/dotnet/vscode-dotnet-runtime/blob/main/vscode-dotnet-runtime-library/src/IDotnetAcquireContext.ts) object in a section titled 'x-dotnet-acquire' to do so. They don't need to include the `requestingExtensionId`, since this is already in their `package.json`,
This should go at the root of a `package.json`, and not in the `contributes` section.
```
"x-dotnet-acquire": {
"version": "8.0",
"mode": "aspnetcore"
}
```
We run the install on startup and also whenever a new extension is installed via the onDidChange API. https://code.visualstudio.com/api/references/vscode-api#Extension<T>
You can easily test this by modifying the SDK extension and adding this, then packging t with `vsce`, installing it, and running the Runtime Extension under the Debug tab. If you do so, you will see that:
If you install a new extension, the request is made.
If you had already installed the extension, the request is also made.
* Respond to linter
* Add a very basic test.
* Fix typo
* Fix typo
TS Lint failed this `Unexpected `await` of a non-Promise (non-"Thenable") value`.
But the callback can be a promise type. Its just not smart enough to realize that.
* update PATH setting description to make it more clear
* add very prototype code
* add tests and get runtime check working
* Fix some bugs
* Consider that the .NET SDK can also satisfy the Runtime
* fix typo
* fix the tests
* fix
* fix another test
* Fix tests even more
* undo yarn changes
* fix the final test hopefully
* undo yarn changes
* Dont use dotnet.exe as its not platform agnostic
* use "markdownDescription" for nicer rendering in VSCode
* Tweak call to action to use VSCode mechanisms first, then our standard installation docs, and only in the last resort mention PATH munging.
* Fix URLs
* Ignore existingPath setting for SDK installs.
C# DevKit never uses the path returned by our installation. This means users would think this path would change the sdk that this extension uses but that is not the case. This path to dotnet.exe is meant to be the path for the runtime for extensions to run on, and not the SDK path. It's confusing that the setting was used for both and a misstep in a way. DevKit is the main caller of this API so we think we can change this with minimal breakage.
* Remove warning setting and fix invalid path setting
The setting must be accessed earlier. This means vscode will need to be restarted. We also update the readme and messaging a bit so its more publicly clear in all places what the setting is for.
* Fix test
---------
Co-authored-by: Chet Husk <chusk3@gmail.com>
* upgrade to the newest version of extension telemetry
* remove vscode-extension-telemetry where its not needed
* Update vscode-test as well since its a new library
* get eslint running but its failing a lot of checks
* Fix the linting after migration
* Fix many errors
* 65 errors to go
* fix almost every lint issue
* clean lint
* Remove references to tslint
* dont install eslint as it is already here
* install eslint
* update eslint
* dont empty cache because eslint is failing to register the types that exist
* try to help eslint find the files for type defs
* Set typescript resolver
* Fix imports
* Add some import plugin but still cant import vscode correctly
* install eslint globally
* skip npm installl
* add imports
* try to fix the import issue
* build before hand so the types are picked up -_____-
* Revert "build before hand so the types are picked up -_____-"
This reverts commit e5381b42df.
* rollback to eslint 8 but still run build
* Fix an issue parsing promises
We changed the function `getPromise` to not use a lock in version 2.1.2. This was a mistake. I thought it would return the unresolved `Promise` object if it was a sync function. That is not the behavior, instead it will skip that line of code and return null even if the promise exists. This causes multiple processes to try to grab the installer file handle.
When one fails to grab the file handle, it causes a chain-reaction of cascading failures for concurrent requests, because that is supposed to be blocked from happening.
In addition, the check I added to kill the sudo process once it is finished runs on all platforms despite the master sudo process only being present on Linux, which has resulted in some higher timeout rates. We need to not do this on other platforms.
* Fix tests
we need to clear the promise out after each test is done
* dont call uninstall all in sdk test, its the wrong command
* reset event stream in the test
* add function to call in extensoin.ts for offline mode acceptAlternativeVersions on acquirectx
* refactor version helper functions outside of version resolver
version resolver is a class that is based around using the internet. It probably should have been like this before.
* add code to check existing installing versions
* Update test name
* respond to linter
* Update lint
* Fix bugs and add messages
* respond to pr feedback
* Add fake MSBuild project to allow signing VSIX and JS
* Sign JS
* Add step to pipeline
* Restrict signing package version
* Add scripts to install signing tool locally
* only go up 1 directory
* fix display name
* fix whitespace -__-
* add build step for installing microbuild
* add sign type parameter
* dont produce alpha package in official drop
* add sign type parameter to default pipeline
* dont publish package in another directory as this causes other issues, instead copy the artifact
* acknowledge the existence of a document explaining the release and other maintenance processes
this information is more suited for internal repo changes.
* change internal stream to a different signing stream
* add dotnet public for notargets sdk
* reduce to 1 feed
* use a different feed
* Use dotnet team
* use explicit feed source
* add feed auth
* fix comment
* rename unsigned vsix so both can get dropped
* use the other type of slash for REN -__-
* try to fix path
* add packages folder with empty gitignore
* figure out dir
* fix rename command
* Fix rename
the 'unsigned' one is actually the signed one
* Fix JS File Sign
dist is at the root of each but this runs in a segregated folder
* produce binlogs and try to fix the path
* Publish SDK Extension Logs
* fix signing
* remove bad slash in path that gets parsed incorrectly
* remove \
* fix microbuild to be installed b5 js signing
* update gitignore
* fix gitignore again
* [REVERT THIS] Add a file A.ts to see if files are signed alphabetical or only non minified JS is signed
* [REVERT THIS] Add content to js to see if minify changes signing
* sign js after webpack
maybe we can sign it after the bundle is created and it will still be able to edit the bundle vsix internals? I thought not but perhaps extension.js is getting replaced by the webpack, so lets see if this works.
* Revert "[REVERT THIS] Add content to js to see if minify changes signing"
This reverts commit ecacc6830b.
* Revert "[REVERT THIS] Add a file A.ts to see if files are signed alphabetical or only non minified JS is signed"
This reverts commit 1ab6ea1873.
* Update name of incorrect task.
* Change 'InstallKey' terminology to 'InstallId'
VS Code has a filter for the string 'key' -- this causes our telemetry to be dropped when referencing the term 'install key' as it thinks it may be a secret. 48ba01b589/src/vs/platform/telemetry/common/telemetryUtils.ts (L336)
Id is safe. I talked with the vscode team and there is no good alternative besides turning off all telemetry safety scanning, which I didn't want to do.
Hence, 'key' --> 'id'
* rename key to id
* rename remaining instances of 'id'
* Improve names of types
* Make Command Executor return an object instead of string
This prevents us from having to run commands twice to get the status and stdout.
Caution must be taken to make sure that the assumptions here in what is returned as stdout vs stderr are correct.
Furthermore, when looking at the code, I saw something that looked just plain incorrect, so I will need to investigate that by adding the TODO before merge
* Fix Execution Type Parsing
* fix linting issue
* Fix line parsing logic
* Fix tests
* Run icacls to set execute permissions
* move to sha256
* remove unnecessary code
* Remove todo that should go in another PR
* Remove redundant trim
* rename class CommandProcessorOutput since it didnt match CommandExecutor
* Rename class file
* Report json object for failed permissions.
* Fix test to use correct folder
* fix lint
* Fix installer file deletion
* respond to linter
* wip - separate out worker paths so contexts are needed for every call, but there is less potential for things to go wrong and less code duplication
* Further isolate code, need to isolate the install tracker now
* migrate install tracker to use event stream instead of being coupled to the context
* move installtracker to a singleton and fix tests
the install tracker really only needs to exist once since the event stream and extension state are global, this will prevent errors from different states of promises and simplify our code with this design pattern to decouple the installation tracking mechanism from the context object, which will reduce code duplication for aspnet install feature additions
* Fix some issues with the test during code migration
* Fix issue with accessing instance
I forgot to ctrl s!
* Clean up logic with install key generation
* Remove duplicate source of truth for architecture
* Improve logging and error handling of install tracker
This class is super annoying to debug without better logging
* Update some broken components of the tests
* Prevent saying object object in the log output by expanding the object
* Tests working
We used to need to concat these tests results with different contexts but not any longer
* FIx uninstall all commands
* Fix the file rename
* Improve logging messages
* Add mode handling for aspnet
* Add ASP NET Support
* Add Generic -> Mode Specific Event Stream Class Hierarchy
* [wip] create events for all 'modal' events
* Dont hold a generic arg obj because it becomes too unwieldy
* Fix some issues with the test and sdk extension
* Add 'mode' to the acquire context
This will allow API callers to set the mode to call aspnet without having to write a new endpoint call. We can default to runtime for when it wasnt provided that would be the old behavior.
* Move 'installMode' into the acquire context
This allows API callers to provide the mode,
We remove it from the worker context to dedupe this and prevent 2x sources of truth.
Now this is a bit weird because some API endpoints will take the mode even though the API endpoint is only for one mode. We will just ignore the mode for that. Eventually we can try to transition all of the related calls to a single endpoint based on 'mode' which would simplify API usage.
* Code cleanup
* De-Dupe Test Code
* Add Requested Events to Republisher
* Update InstallKey pattern to include ~aspnetcore
This is a bit strange since - used to be for 'global' and ~ for architecture. There are only so many path safe characters to use, more importantly though we dont rely on the install key as the information for the install anymore, it is stored in the DotnetInstall object. ~ has logic in place to not account for a 'legacy' install, so ~ with aspnet (which would not have existed before arch was added) is also a bet that will work with pre-existing logic.
* fix build issues
* Add ASP.NET Directory Provider
* Only Report Total Success if Path exists
The path can be undefined if an error throws so its not a success
* Add ASP.NET API to sample extension
* Resolve Asp Net Runtime versions as Runtime versions
* Fix mistake on test
* Fix bug with install script param
* add source-map-support to allow build in CI
this got removed and passed earlier somehow
* respond to linter
* add source map support
* Respond to linter
* Add test for specific telemetry messages
* 2.0.7 branding
* Prevent circular import
* rewrite completion event so we can test the modaleventrepublisher
* fix linter issue
* Revert new test since the object is not mockable
* Get rid of check for event that cannot be published via mocks
* Fix bug where logging was not up to date
to prevent i/o costs the file is dumped at the end of error handling, but this means other events past error handling dont get added to the log. This fixes this.
* respond to pr feedback
* Remove 'Debug' As it's not used
* Remove 'Diff' as it's not used
* Remove 'es-abstract' as its not used
* Remove 'got' as its not used
* Remove http cache semantics from the main dependencies list
its just a sub dependency
* Remove 'p-retry' as its not used
* Remove 'public-ip' as its not used
* Remove tslint from dependencies, its not a dependency
* Increase attribution information for library dependencies
* Remove a ton of unused dependencies
* Clean out dependency tree
* Clean up sample extension dependencies
* Fix SDK Yarn.Lock
* Fix sample yarn.lock
* Reorder remaining licenses to match package.json
* add missing semver package that used to be included elsewhere by other dependencies
* Delete vscode-dotnet-runtime-extension/yarn-error.log
* Update readme.md with notices per industry standard
* Add back in ts-loader as it is required
* wip - separate out worker paths so contexts are needed for every call, but there is less potential for things to go wrong and less code duplication
* Further isolate code, need to isolate the install tracker now
* migrate install tracker to use event stream instead of being coupled to the context
* move installtracker to a singleton and fix tests
the install tracker really only needs to exist once since the event stream and extension state are global, this will prevent errors from different states of promises and simplify our code with this design pattern to decouple the installation tracking mechanism from the context object, which will reduce code duplication for aspnet install feature additions
* Fix some issues with the test during code migration
* Fix issue with accessing instance
I forgot to ctrl s!
* Clean up logic with install key generation
* Remove duplicate source of truth for architecture
* Improve logging and error handling of install tracker
This class is super annoying to debug without better logging
* Update some broken components of the tests
* Prevent saying object object in the log output by expanding the object
* Tests working
We used to need to concat these tests results with different contexts but not any longer
* FIx uninstall all commands
* Fix the file rename
* Improve logging messages
* fix indentation
* Introduce error to wrap all global sdk errors
* Wrap installation in success event to improve trackability
* Expose the original event name for property expansion
* Respond to linter
* Improve Error Message Specificity
* Update the event stream to use the right install key
* Update Errors to be EventBasedErrors that contain the Event Name
This is a mildly ugly pattern. See the PR description for more detail.
* Make legacy code throw an error instead of throwing a string
Throwing other object types up makes it harder for us to get the callstack etc
* Fix bug with install key management where it would not have -global or the architecture would be unspecified
* respond to linter
* Fix whitespace
* Fix whitespace
* WIP - Migrate Install Key strings to type to allow future flexibility with installs -- e.g. non-versioned install folder names so we can auto update. Start collecting more information during install about who installed it so we can ref count them for uninstalls. Does not migrate telemetry events or other code yet which needs to be experimented on further
* WIP - Migrate all code to use InstallKey Type & Fix Bugs
There are several big things here that might seem like small code changes.
1 - Before when we found a 'partial install' we would delete ALL local runtimes. I am astounded this exists. There are other extensions that might depend on those other runtimes and this just deletes it from underneath them. A partial install is rare, I think you need to pkill vscode or something since the install script runs out of proc. We consider an install partial if our code never got to mark it as installed after calling the install script. Its rare but does happen, at 2010 occurences out of 470k in recent days.
Renames a function which appears to handle partial installs to clarify it only manages local ones.
Gets rid of some unused functions or moves them to a better place.
Renames IInstallKey to DotnetInstall. The install key is the string representing the directory of the install. It shouldn't have two meanings.
Add logic for ref counting which extensoins own which installs.
* wip - record extension owner for installs
Fix record keeping to properly ref count extensions
Rename types to match standard better
Still need to figure out how to manage uninstalling and 'graveyard' as well as how to remove a 'legacy' install via version when it is a dotnetinstall object.
* Separate out 'graveyard' code or management of old installs to another class
* Dont pull files from dist - bad copilot, bad
* Update some more of the code to use new types
* update mock objects to get the build working
* respond to linter
* Add back in the old telemetry prooperties
I got rid of them because they are duplicated data. But TBH there are a lot of things that rely on the pre-existing property names. It would be much harder to query new and old data together if getting rid of them, so I added them back even though it causes some data duplication.
* Fix bugs with install management
* add missing semicolon lol
* Fix a bug with clearing out the installed list
The logic was missing a !.
* Fix Install Graveyard Logic
* Fix test logic for legacy installs
* Skip unnecessary preinstall local sdk check on global sdk
* Add back code to account for preinstall in sdk status check
* Remove unnecessary string checks
We never give a string to this class anymore so it's not needed
* Add event for when tracking installs messes up somehow
* Refactor classes out to many more files
* Respond to linter
* Isolate install tracking management to a separate class
* Clean up acquisition worker and remove unnecessary logic
* Port the worker to using an external install manager to isolate protected data
* Respond to linter concerns
* Remove potential deadlock with installation management
* Increase timeouts
With the change where you need to hold a lock to edit extension state to protect against timing challenges where an install is being added or deleted at the same time and can cause incorrect behavior, we had to slow down the product.
* Migrate to a singleton for install tracker so no dupliacte lock is created
* Working build for changing 'isRuntime' to mode for 'aspnet' 'sdk' or 'runtime'
* undo the singleton in test as the context needs to be per test
* remove the singleton pattern as it will result in a bug with multiple acquisition requests having a stale context
* Add Tests for Install Tracking with who owns what install
* improve timeout on test
* Revert "improve timeout on test"
This reverts commit a4168f6ec5.
* fix timeout
* Fix Test
* Fix Logic for adding pre existing installs
* Fix tests to be successful
* Code Cleanup & Remove Potential Tech Debt for Modes
remove places where we make assumptions on the existing model (sdk or runtime) to prevent future tech debt
* respond to linter
* Fix bug with context being in a different mode than a legacy installation
Mark old function as deprecated and add a comment explaining why our assumption there works
Fix possible bugs with context management where context could be a different mode than what was previously installed which could cause future bugs down the line that would be hard to track down
* Add back in removed events to prevent breaking old telemetry dashboards
* Final Code Review from Me
* Fix Test
* Remove circular dependency from test artifact
* add missing file header from refactor
* fix test
* delete directory per mode file as there is a git bug where changes in file caps is not being picked up
* Add back into directory file with proper capitilzation per git bug
* Add missing await which might cause test to fail inconsistently
* Make the lock based timeout based on the user setting for timeouts
* add logic for adding a global shared path to extension
* add tests for global path config
* update comment out code
* update mock comment
* remove commented out code in ExtensionConfigurationWorker
* update ExistingPathResolver
* add tests for global shared path
* remove empty line
* resolve duplicate import
* fix tslint
* resolve format comments
* get rid of set global path to avoid type error
* add logic for checking local paths first in ExistingPathResolver
* create set global path method
* fix test
* Fix Typo
* change the field name for global shared path; add sharedExistingDotnetPath to package.json; change get and set method for shared path for ExtensionConfigurationWorker
* fix typo
* update readme for global shared path
* Add more examples to the path for .NET
* Consolidate read.me section to one page
* Simplify Logic for Failure to set Manual Path
I thought about the direction this should go, and I think that the old logic only makes sense because we'd need to concat the existing settings. In this case we are setting the global setting, so we can just add it without having to do any additional logic.
* Consolidate setting global and local to one function for simplicity
* update sample page to use the string instead of path obj
* Refactor path setting functions to two separate ones
We need to access the configuration with the direct objects the users can use the settings json file, hence this could not be simplified to a shared object.
* update class member names to disambiguate overloaded terms
* Check whether a matching extension id extension exists before calling the global path
* Make read.me more clear
* Update read.me to show that the example is only an example
* .some may not exist in certain versions so dont use it?
* Dont activate the runtime extension under test
* dont rely on test env variable since we cant set one within vscode launch profile
* Fix ability to run extension tests under launch json
We activate the extension in the test to inject custom objects to track 'telemetry' under test and analyze the telemetry events to ensure things are going well.
We cant activate the extension twice.
In the recent past we made a change to auto activate as a necessity to check the operating system before exposing APIs to users that were not available on their OS.
When I made that change I think we actually broke the ability to run the runtime extension tests under the launch profile. This fixes that.
* Remove global setting from test
This setting impacts the return path of all tests when correctly implemented. We dont want to dupe the extension test configuration for a single test to run it with a different configuration either, so we cant test it this way.
* Add tests that dont rely on the extension
* fix typo
* improve warning messages to correctly diagnose what went wrong
* remark that you need to restart vscode for the setting to take effect
* return empty properties array instead of undefined so properties are forwarded via telemetry observer
---------
Co-authored-by: Noah Gilson <noahgilson@microsoft.com>
Co-authored-by: Noah Gilson <nagilson@umich.edu>
* Migrate pipeline to 1es
* Clean up changes made from migration tool
* Separate pipeline out into many yaml files so we can have 2 pipelines without code duplication
due to new internal requirement
* remove unncessary pipeline backup
* Remove pipeline from 1es for public as it is not going to be added to the organization in public
* Update yaml loop logic
Remove devcontainer as it is out of compliance and not used by anyone
* fix mappping by adding body to the loop with indentation:
* add a 1es repository
* Create a separate 1es pipeline
* use self to search in the correct repository
* migrate to self on 1es
* use folder name pipeline-templates over templates
* use yaml over yml
* temporarily move jobs into one os
* split pools
* Move the loop to out of the job
* move the task into steps instead of a job as u cant do a job in a job
* remove template keyword
* dont call steps template in job outside of steps
* use yaml instead of yml
* add sdl
* use unique names for jobs
* use _ instead of space
* Use azure pipelines name to conform to style
* give image to each parameter
* make it 1 os for 1 os job and use different pool
* Call pool explicyk
* try to fix names
* Use different source for mac instead of 1es
* Update get-func-name
* Update public PR pipeline
* use public pool as thats what the name param is for
* give image name instead of vmimage as it snot used
* try to switch to a better pool
* try using internal pool on public as external is not available
* Publish logs to os specific folder
* condition steps so they are skipped if not needed per os
* dont care if it succeeded
* add paran
* remove paran
* use parameter pools instead of agent os
* move to 7.0 instead of 6 bc six is broken in the cache online maybe
* switch to 1es pool
* try using public pool
* See if mac pipeline has a mirror in svc
* use image names from open
* Use vm image in the image
* Use vm image in the image
* update vm image
* try to condition the pool
* code cleanup now that everything was working
* fix whitespace
* Update vscode-test
* Update vscode-test to the new name
* respond to most pr feedback
* rename the file as I cannot change the pipeline name while its in main maybe
- The acquisition promises were tracking 2 things...
1 is the current working in progress installs.
2 is all installs that were ever done, with the assumption no outside source will ever delete the installs, which is incorrect.
When fixing 2, several tests broke. This is because they relied on the promises to be there even though nothing was installed. We dont actually want to install on the test so we dont change that, instead we add a boolean to fake the thing into thinking an install was laid onto the disk
Some errors can be thrown in contexts without a version to acquire such as the install linux dependencies command. Using ! is an anti pattern and removes type safety. We just need to make sure the install key is included everywhere we error from the acquisition context, which I do believe is the case, otherwise this fix will not work in those edge cases
The other thing to note here is how I added the version being set in the mock acquisition worker... the version is set by the extension in extension.ts but this does not happen in unit tests so we needed to add that logic now that everyone needs an acquisition context to be happy
remove extra sdk feature band check thats already done in the linux distro code
make the context optional in places where it may not actually exist, e.g. if vs code telemetry setting is changed before we call an acquire command, there would be no acquire context
- First moving the mock object import in index.ts resolved the dependency loading order problem where classes were being imported out of order
- Then, removing the getInstallKey function out of the worker prevented loading the vscode module which will always cause a failure in the library tests
- Also cleans up imports and adds whitespace
Some problems remain post refactor which I will fix next
Do this to deduplicate event stream, timeout time, extension state, proxy url, arch, and install key so we can throw errors from other places
uses a hack for the telemetry observer because it needs to be created before the acquisition context can be created
'vscode' cannot be referenced outside of code that's directly part of an extension. So wrapper classes are necessary to access the vscode ui api, or the environment, or the extension context environment. If referencing a file that references these types the build will pass but tests will fail to execute.
The utility context is where things like this will go in the future. Some of these objects must be created before the acquisition context can be ready and it is best to have some 'global' state for utilities. File Utilities could probably go there in the future as well.
It is an ugly hack but the best option. Note the runtime extension test is timing out, which I am investigating.
The process environment is for the extension host, which is separate from the vscode terminal environment. That needs to be edited as well, which we now do here.
- the shell option is needed for spawn to access windows shell commands, exec does this by default but exec does not capture detailed output streams
- fix a timeout where ms was used as seconds
- note that you need to restart for telemetry notice to change
Noah Gilson
andysleigh
Noah Gilson
JL03-Yue