Skip to content

WIN-24578 :: Add Rider plugin support for .net projects - #19

Open
agorielikov wants to merge 8 commits into
mainfrom
feature/WIN-24578-add-rider-plugin-support
Open

agorielikov wants to merge 8 commits into
mainfrom
feature/WIN-24578-add-rider-plugin-support

Conversation

@agorielikov

Copy link
Copy Markdown
Collaborator

Add Rider plugin support, which still installs from same zip file as CLion and RustRover.
Distribution works as expected, tested on a build with 100 sub-project libraries.
Caching doesn't work as .net is not supported yet.

@agorielikov
agorielikov requested a review from udif2025 October 9, 2026 14:32

@udif2025 udif2025 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solid, well-documented PR - the NativeBuild refactor is clean and the -graph / -nodeReuse:false / UseSharedCompilation=false reasoning is great. No blockers; the one I'd like addressed before merge is how Build Selected Projects handles referenced projects' configuration/platform (comment on projectsBuild). The rest are smaller points and nits.

Comment thread installer/install-common.ps1 Outdated
# RR = RustRover, IU = IntelliJ IDEA Ultimate, RD = Rider (the .NET actions -
# see rider-support.xml). Community editions (IC) don't carry
# com.intellij.modules.ultimate, which the Rust plugin requires there.
$targetProductCodes = @("RR", "IU", "RD")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: is there a Linux installer script that needs Rider added too, or is this .ps1 the only one?

Comment thread rider/build.gradle.kts
dependencies {
intellijPlatform {
// Same 2026.2 (262) line as the root module's other compile targets.
rider("2026.2") {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rider module is compiled against 2026.2 only, and the README says Rider 2026.2. If the plugin's since-build still allows 261, Rider 2026.1 will load rider-support.xml and may fail on Rider-internal APIs (isExistingSolution, solutionFilterPath, ...). Either add Rider 2026.1 to the Verifier IDEs or make sure the compatibility range / installer keeps it out.

}
val exitCode = processHandler.exitCode
val (content, type) = when {
stopRequested -> "${presentation.contentName} was stopped." to NotificationType.INFORMATION

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stopRequested is only set when the build is cancelled from this status-bar progress. Stopping it via the Stop Build action or the tool window ends up here as "failed (exit code -1)" with an ERROR balloon. Consider checking whether the handler was destroyed / sharing a flag with Stop Build.

}
cmdLines.append("\r\n")
if (index < build.commands.lastIndex) {
cmdLines.append("if errorlevel 1 exit /b %ERRORLEVEL%\r\n")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if errorlevel 1 only catches exit codes >= 1, so a negative one (e.g. a crashed dotnet host) lets the chain carry on. if not "%ERRORLEVEL%"=="0" exit /b %ERRORLEVEL% (or || exit /b) is safer.

.append(" --caption ").append(shellSingleQuoted(project.name))
script.append("exec ").append(shellSingleQuoted(consolePath))
.append(" --caption ").append(shellSingleQuoted(caption))
.append(" --ib-quiet --build-cache-local-shared --build-cache-basedir=\"\$PWD\" -- ")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this still passes --build-cache-local-shared --build-cache-basedir for MSBuild builds, though caching isn't supported for .NET yet. Harmless, or should they be dropped for MSBuild?

return "\"" + escaped + "\\".repeat(trailingBackslashes) + "\""
}

private const val CMD_SPECIAL_CHARACTERS = "&|<>^()"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: cmdQuoted now also quotes &|<>^() and doubles trailing backslashes. That changes the CMake/CLion command line too - looks correct, but worth a quick CLion sanity run.


internal object DotNetBuildRunner {

fun build(project: Project, request: DotNetBuildRequest, actionType: String) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: nothing stops a second Build from starting while one is still running on the same solution - two MSBuilds in one tree will hit file-lock failures. Fine if Rust/CMake behave the same today.

* One MSBuild invocation per project, run one after the other: MSBuild only accepts a single
* project per invocation. Each one is still parallel within itself (see [COMMON_MSBUILD_ARGUMENTS]).
*/
internal fun projectsBuild(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Building each project directly with -p:Configuration=... -p:Platform=... means those global properties also flow to its ProjectReferences (outside a solution build MSBuild doesn't unset them). So every referenced project is built with the selected project's mapped config|platform rather than its own solution mapping - e.g. App -> Release|x64, Lib -> Release|AnyCPU: Lib gets built as x64 into a different bin\ than Rider's build, and can bring back the plugin-vs-Rider rebuild churn e02c494 fixed.

Suggestion: build through the solution with project targets instead, e.g. App.sln -t:"Folder\App;Lib" (solution-folder path is available from Rider's entity tree or the .sln). The solution then maps every project correctly, and multiple selected projects build in one parallel invocation instead of sequentially. If you prefer the current approach, at least worth testing on a mixed-platform solution.

*/
internal object MsBuildLocator {

fun locate(): MsBuildLocation {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: locate() runs vswhere on every build (up to a 15s timeout). Could cache the result per session and reset it when the setting changes.

@agorielikov
agorielikov requested a review from udif2025 October 19, 2026 07:33

@udif2025 udif2025 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall the design is solid: the Rider adapter only exposes platform types, the MSBuild flag choices are well reasoned, vswhere requires the .NET SDK component, cmdQuoted handles operator characters and trailing backslashes, and stopped builds are reported as stopped. CI is green.

The main issue is the first inline comment: -graph most likely breaks Build Selected Projects, and nothing here tests it against a real MSBuild. The version/embedded-zip mismatch should also be fixed before merge. The rest are smaller suggestions.

Review drafted with Claude Code.

* only affect this one command-line invocation.
*/
internal val COMMON_MSBUILD_ARGUMENTS = listOf(
"-graph",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likely bug: -graph breaks Build Selected Projects. projectsBuild passes the solution's per-project targets (-t:src\App;My_Lib), but in static-graph mode MSBuild doesn't build the solution metaproject. It expands the .sln into entry points for every project and applies the requested targets to each project directly. So App.csproj would be asked for a target named src\App and fail with MSB4057, and it wouldn't be limited to the selection either.

The PR description only mentions whole-solution testing. Could you run this action on a real solution with a project inside a solution folder? If it fails, I'd drop -graph for projectsBuild, since the graph-mode rationale above is about whole-solution builds anyway.

(I couldn't run MSBuild to confirm, so this is from how graph builds handle .sln entry points.)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ran it on a real solution (App → My.Lib in a src solution folder, plus an unrelated root Other project) with both dotnet msbuild 17.14.43 and VS 2022 MSBuild 17.14.40, using the exact projectsBuild arguments including -graph. -t:src\App, -t:src\My_Lib and -t:src\My_Lib;Other all exit 0 and build only the selected projects plus their references (Other is untouched unless selected). The static graph does load every project in the .sln, but graph mode then maps the solution's per-project targets to those projects rather than passing them down, so no MSB4057. Keeping -graph for projectsBuild.

Comment thread scripts/install-intellij-ib-plugin.ps1 Outdated
@@ -129,7 +131,7 @@ if ($anyRunning) {
}

$pluginZipBase64 = @'

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The version isn't bumped but the embedded build changed. gradle.properties is still version = 1.0.3, while this blob now contains a different intellij-ib-plugin-1.0.3.jar (I decoded it and it includes the Rider/dotnet classes). That leaves two different builds labelled 1.0.3 in the field. Please bump the version and move the CHANGELOG [Unreleased] entries under it.

Also, a ~200 KB locally built blob can't be verified in review or reproduced from CI. Consider generating it in the release workflow instead of committing it.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bumped the version to 1.1.0.

  • gradle.properties is now 1.1.0. I chose a minor bump because this release adds CLion and Rider support; switch it to 1.0.4 if you'd rather.
  • In CHANGELOG.md, the former Unreleased entries now sit under ## [1.1.0], with an empty ## [Unreleased] above.
  • A fresh build produces intellij-ib-plugin-1.1.0.jar, so two different builds no longer share the 1.0.3 label.

// org.rust only loads through rust-support.xml. Confirmed by the Verifier's own "Missing
// dependencies: com.jetbrains.rust (optional): Unavailable" against Rider. (The CMake lines above
// don't apply to Rider: it bundles com.intellij.cmake itself.)
com.incredibuild.ibplugin:Package 'org\.rust' is not found

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ignore is global, so it also hides real org.rust resolution problems when verifying against RustRover/IDEA, not just Rider. Can it be scoped to the Rider target? If not, a RustRover-only verification run without this line would keep that coverage.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scoped to Rider now.

request.kind,
request.configuration,
request.platform,
restore = needsRestore(readSolutionProjectFiles(request.solutionFile))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When a .slnf is open, entryPoint is the filter but this reads the full .sln. One never-restored project that the filter excludes then adds -restore to every filtered build, which is the restore churn COMMON_MSBUILD_ARGUMENTS says it avoids. Suggest reading the filter's project list when entryPoint is a .slnf.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The restore check now reads only the filter's own projects when a .slnf is open, so a project the filter leaves out can no longer add -restore to every filtered build.

.append(" --caption ").append(shellSingleQuoted(project.name))
script.append("exec ").append(shellSingleQuoted(consolePath))
.append(" --caption ").append(shellSingleQuoted(caption))
.append(" --ib-quiet --build-cache-local-shared --build-cache-basedir=\"\$PWD\" -- ")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that this path is shared with MSBuild, the build-cache flags go to .NET builds on Linux too. The PR says caching doesn't support .NET yet. Should these be left off for MSBuild builds, or have you confirmed they're harmless?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, and it applied on Windows too: the shared native path was also sending /BUILDCACHELOCAL=ON to MSBuild builds. I hadn't verified the cache was harmless for .NET, so rather than rely on that, NativeBuild now has a buildCache flag. It stays on for CMake and is off for both MSBuild builders, so .NET builds get no --build-cache-* options on Linux and no /BUILDCACHELOCAL on Windows. Tests cover both wrappers and both builders. We can turn it back on once Build Cache supports .NET.

* two quick clicks can't both get past it. A Rebuild's chained build is unaffected: its clean
* has already ended by the time the build starts.
*/
private fun confirmNoBuildRunning(project: Project): Boolean {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This guard now applies to the Rust and CMake builds too, which is a behaviour change worth a CHANGELOG line. Two smaller points:

  • After "Stop Running Build" the new build still isn't started, so the user has to click again. Consider starting it once the stop completes.
  • Builds accelerated through IncredibuildProjectTaskRunner (the IDE build pipeline) aren't tracked by isBuildRunning(), so they can still overlap with a tool-window build.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, all three addressed:

CHANGELOG: Added under 1.1.0 → Changed. Only one Incredibuild build runs at a time, in every IDE. Starting a second one offers to stop the running build, and before this change two builds could run side by side with the second replacing the first's tool-window output.

Start after stop: "Stop Running Build" now starts the new build automatically once the old process has actually exited, never while it's still winding down. run() now reports through onStarted / onNotStarted callbacks instead of returning the handler, since the start can now happen later. When the old process ends, the guard runs again before starting, in case something else started in between. Cancel behaves as before: the build doesn't start, and an IDE-pipeline build reports ABORTED. The dialog now reads "Stop it and start this one once it has stopped?". The menu builds, the CLion/Rider IDE builds and Rebuild's chained cargo clean all go through this unchanged.

Pipeline overlap: Half right. Pipeline builds were already tracked, because IncredibuildProjectTaskRunner creates its process through createProcessHandler, which registers it as the active handler. So isBuildRunning() and Stop Build saw it, and a menu build started during a pipeline build was already caught. The gap was the other direction: the pipeline never checked before starting, so it could start on top of a running menu build. It now calls a new blocking awaitNoBuildRunning() before each build, and before Rebuild's cargo clean. That shows the same stop prompt, waits for the stop to complete, and reports ABORTED if the user cancels.

}
val target = when (request.kind) {
DotNetBuildKind.BUILD_PROJECTS ->
request.projectFiles.joinToString(", ") { File(it).nameWithoutExtension }

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: with many projects selected, this makes the banner and progress title very long. Consider capping it, e.g. App, Lib +12 more.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. The banner now shows at most two names plus a count, e.g. App, Lib +12 more. Up to three are still listed in full, since "+1 more" wouldn't save anything. The progress title wasn't affected: it shows the solution name, not the selection. Covered by a unit test.

return
}

val selection = RiderSolutionModel.selection(e.dataContext)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: the selection is computed for Build/Rebuild Solution outside the context menu too, where it isn't used. You could compute it only for BUILD_PROJECTS or isFromContextMenu.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. update() now reads the selection only in the context-menu branch and for Build Selected Projects. On the menu path it sits after the kind != BUILD_PROJECTS || check, so Build/Rebuild Solution skip it entirely. No behaviour change.

Comment thread CHANGELOG.md

### Added

- CLion support for CMake projects: **Build**, **Rebuild** and **Build All** in the **Incredibuild** menu run

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

plugin.xml already described CLion support before this PR. Was this entry already released? If so, it shouldn't be under [Unreleased].

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With new version all of it was moved out of unreleased and under 1.1.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants