Switch Razor to using clasp as a source package #10139
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| #!/usr/bin/env bash | ||
|
|
||
| # This works around an issue where .editorconfigs in nuget packages are not respected if the NUGET_PACKAGES | ||
| # environment variable does not end with a slash. We copy the logic for setting the NUGET_PACKAGES variable from the eng/common/tools.sh | ||
| # script as we cannot modify the script itself (its arcade managed). | ||
| # This workaround is only required for non-windows builds as the powershell versions of the arcade scripts already ensure a trailing slash is present. | ||
| # Tracking https://github.com/dotnet/roslyn/issues/72657 | ||
| ci=false | ||
| while [[ $# > 0 ]]; do | ||
| opt="$(echo "${1/#--/-}" | tr "[:upper:]" "[:lower:]")" | ||
| case "$opt" in | ||
| -ci) | ||
| ci=true | ||
| break | ||
| ;; | ||
| *) | ||
| shift | ||
| ;; | ||
| esac | ||
| done | ||
| if [[ "$ci" == true ]]; then | ||
| if [[ -z ${NUGET_PACKAGES:-} ]]; then | ||
| if [[ "$ci" == true ]]; then | ||
| export NUGET_PACKAGES="$HOME/.nuget/packages/" | ||
| else | ||
| export NUGET_PACKAGES="$repo_root/.packages/" | ||
| export RESTORENOCACHE=true | ||
| fi | ||
| fi | ||
| fi |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,8 @@ | ||
| // Copyright (c) .NET Foundation. All rights reserved. | ||
| // Licensed under the MIT license. See License.txt in the project root for license information. | ||
|
|
||
| extern alias LegacyClasp; | ||
|
|
||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Collections.Immutable; | ||
|
|
@@ -240,8 +242,27 @@ public static ApplyFormatEditsHandler New( | |
| BufferManager bufferManager, | ||
| ILspLogger logger) | ||
| { | ||
| var instance = CreateInstance(Type, textBufferFactoryService, bufferManager.Instance, new LegacyClaspILspLogger(logger)); | ||
| return new(instance); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Wraps the razor logger (from the clasp source package) into the binary clasp logger that webtools uses. | ||
| /// </summary> | ||
| /// <param name="logger"></param> | ||
| private class LegacyClaspILspLogger(ILspLogger logger) : LegacyClasp.Microsoft.CommonLanguageServerProtocol.Framework.ILspLogger | ||
|
There was a problem hiding this comment. I'm guessing that this is necessary until Web Tools is updated and then we pull in new Web Tools assemblies for tests? If so, could you please be sure to file an issue for us so we remember to get back to this? There was a problem hiding this comment. Yeah we can remove the code that ships the legacy clasp package. I filed #10160 for that. However I expect this particular bit of code will actually need to change to a reflection instantiation of the ILspLogger from the webtools package (since the razor source package type isn't the same type as the webtools source package type). Its unfortunate that the clasp type is exposed across boundaries here - maybe there is a possibility on the webtools side to retrieve the logger in another way (that Razor wouldn't need to know about). There was a problem hiding this comment. Yeah, that makes sense. Of course, we use so much reflection for the WebTools code here that's really just business as usual. 😄 |
||
| { | ||
| public void LogEndContext(string message, params object[] @params) => logger.LogEndContext(message, @params); | ||
|
|
||
| public void LogError(string message, params object[] @params) => logger.LogError(message, @params); | ||
|
|
||
| public void LogException(Exception exception, string? message = null, params object[] @params) => logger.LogException(exception, message, @params); | ||
|
|
||
| public void LogInformation(string message, params object[] @params) => logger.LogInformation(message, @params); | ||
|
|
||
| public void LogStartContext(string message, params object[] @params) => logger.LogStartContext(message, @params); | ||
|
|
||
| public void LogWarning(string message, params object[] @params) => logger.LogWarning(message, @params); | ||
| } | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Firstly, assume I know nothing about our infra, but do we want to change things so that this is no longer automatically updated by our darc subscriptions? Seems like if we don't do that, you're just as likely to break us if you make changes to things.
Having said that, I've no idea if these ever actually automatically update, as I've just been doing it manually when I need to :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no idea if roslyn ide packages can be upgraded by darc subscriptions. But either way you don't have any subscriptions in Razor from Roslyn for any of your normal branches, so I don't know why you have versions in version.details.xml
From darc:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated the version details file as well since CI was complaining. I assume there must be some other reason for having the versions listed there too, but there aren't any subscriptions using it as far as I can tell.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I have no idea either, I just always update both files 🤷♂️