Repository navigation
WIN-24578 :: Add Rider plugin support for .net projects - #19
Conversation
udif2025
left a comment
There was a problem hiding this comment.
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.
| # 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") |
There was a problem hiding this comment.
Nit: is there a Linux installer script that needs Rider added too, or is this .ps1 the only one?
There was a problem hiding this comment.
The .ps1 is the only installer. There's no Linux installer script in the repo, and Linux users install from the Marketplace or with "Install plugin from disk", which covers Rider like any other IDE. Rider is already in the Windows installer: install-common.ps1 targets RR, IU, CL and RD, and both the .exe and the standalone script are built from it.
| dependencies { | ||
| intellijPlatform { | ||
| // Same 2026.2 (262) line as the root module's other compile targets. | ||
| rider("2026.2") { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Good call. I added the 2026.1 IDEs (CLion, RustRover, Rider) to the Plugin Verifier alongside 2026.2, so the whole declared 261–262 range is now verified. Rider 2026.1 comes back compatible: nothing the Rider module uses is missing there. But it surfaced a real, pre-existing break in RustRover/CLion 2026.1: the IDE-build runner called CargoBuildAdapter.attachToProcessHandler, which only exists from 2026.2, so accelerated Build Project would throw NoSuchMethodError there (shipped since 1.0.3). Fixed by resolving the adapter API for whichever Rust plugin version is loaded, matching that version's own CargoBuildManager. All six IDEs are now compatible. README and CHANGELOG updated.
| } | ||
| val exitCode = processHandler.exitCode | ||
| val (content, type) = when { | ||
| stopRequested -> "${presentation.contentName} was stopped." to NotificationType.INFORMATION |
There was a problem hiding this comment.
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") |
There was a problem hiding this comment.
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\" -- ") |
There was a problem hiding this comment.
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 = "&|<>^()" |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Thanks. Sanity-checked in CLion: CMake Build and Rebuild through Incredibuild both work with the new quoting. The rules are also covered by unit tests in IncredibuildRunnerTest: plain tokens stay unquoted, values with spaces or & | < > ^ ( ) and empty values get quoted, % is escaped, and trailing backslashes are doubled so they can't escape the closing quote. For the usual cmake --build arguments nothing changes, since none of them contain those characters. Only paths or target names that do, such as a build directory under C:\R&D, now get quoted correctly instead of being split by cmd.exe.
|
|
||
| internal object DotNetBuildRunner { | ||
|
|
||
| fun build(project: Project, request: DotNetBuildRequest, actionType: String) { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Handled. Every Incredibuild build, whether .NET, Rust or CMake, goes through a one-build-at-a-time guard in IncredibuildRunner.run(). If one is running, the user is offered to stop it. On confirmation it's stopped gracefully, and the new build starts only after the old process has exited. Cancel starts nothing. So two MSBuilds never run in the same tree.
| * 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( |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Handled in 7384109. projectsBuild now builds through the solution with one per-project target per selected project (e.g. -t:src\App;Lib), so the solution maps every project and its references to their own configuration|platform, and the whole selection builds in one parallel invocation. Target names come from the solution file (nested solution folders and MSBuild's name cleansing, for .sln and .slnx), with unit tests. I've run it against real MSBuild 17.14, where only the selected projects and their references build.
| */ | ||
| internal object MsBuildLocator { | ||
|
|
||
| fun locate(): MsBuildLocation { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Handled in 272389e. MsBuildLocator caches the detected MSBuild for the session, and the MSBuild setting's setter clears that cache on any change, including clearing it back to auto-detect. A configured path skips vswhere entirely. It also re-detects if the cached executable has disappeared, and never caches "not found", so installing MSBuild takes effect without restarting the IDE.
udif2025
left a comment
There was a problem hiding this comment.
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", |
There was a problem hiding this comment.
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.)
There was a problem hiding this comment.
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.
| @@ -129,7 +131,7 @@ if ($anyRunning) { | |||
| } | |||
|
|
|||
| $pluginZipBase64 = @' | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Scoped to Rider now.
| request.kind, | ||
| request.configuration, | ||
| request.platform, | ||
| restore = needsRestore(readSolutionProjectFiles(request.solutionFile)) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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\" -- ") |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 { |
There was a problem hiding this comment.
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 byisBuildRunning(), so they can still overlap with a tool-window build.
There was a problem hiding this comment.
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 } |
There was a problem hiding this comment.
Nit: with many projects selected, this makes the banner and progress title very long. Consider capping it, e.g. App, Lib +12 more.
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
|
||
| ### Added | ||
|
|
||
| - CLion support for CMake projects: **Build**, **Rebuild** and **Build All** in the **Incredibuild** menu run |
There was a problem hiding this comment.
plugin.xml already described CLion support before this PR. Was this entry already released? If so, it shouldn't be under [Unreleased].
There was a problem hiding this comment.
With new version all of it was moved out of unreleased and under 1.1.0
udif2025
left a comment
There was a problem hiding this comment.
Thanks for working through the last round - the version bump, CI-built installers, per-IDE Verifier runs, the .slnf restore fix and the Build Cache flag all look good.
This push also adds IDE build interception for CLion and Rider (license check, CMake task runner, Rider action wrappers and before-run providers), which roughly doubles the PR's scope. Please update the PR description and test notes to cover it, or split it into a follow-up if that's easier.
Inline comments below - the caching, license-semantics and possible-hang ones are what I'd like answered before merge. Also still open from the first round: the Rider 2026.1 compatibility range, the Linux installer question, and a CLion sanity run after the cmdQuoted change (more relevant now that CLion IDE builds use the same path).
| # committed. Windows-only: the .exe needs csc.exe and 7-Zip, and the standalone script is run | ||
| # with Windows PowerShell, which is also what assembles it here (see installer/README.md). | ||
| installers: | ||
| name: Build Installers |
There was a problem hiding this comment.
This job only runs when a release is published, so CI hasn't exercised it yet. Does the uploaded asset really match intellij-ib-plugin.zip (and not a versioned name)? Consider adding workflow_dispatch so it can be tried before 1.1.0. Also, does anything (README, wiki, download links) still point to the deleted scripts/install-intellij-ib-plugin.ps1?
There was a problem hiding this comment.
Asset name: yes. build.gradle.kts pins buildPlugin's output to intellij-ib-plugin.zip, unversioned, and "Upload Release Asset" uploads build/distributions/*. A signed archive, if signing is configured, gets its own name, so the exact-name download still picks the plain zip.
Trying it early: I moved the job into a reusable installers.yml, which release.yml calls with the tag, so release behaviour is unchanged. It also has workflow_dispatch: without a tag it builds from the branch and uploads the installers as an artifact only; with a tag it re-runs the release step for that release. Since dispatch only appears once the file is on main, it also runs on PRs touching installer/** or the workflow, which exercises it on this PR.
Links: nothing in the repo, on any branch, points to the old scripts/ path. I couldn't check the wiki or external download pages from here; could you check those? I also added a README line pointing to the installers on the release page.
| /** The project to build for [configuration]/[task], or null for the whole solution. */ | ||
| protected abstract fun projectFile(configuration: RunConfiguration, task: T): String? | ||
|
|
||
| override fun getId(): Key<T> = rider.id |
There was a problem hiding this comment.
This depends on the platform picking the first provider registered for a key when running a step, and the last one when loading a saved step - neither is a documented guarantee. A short manual check on Rider 2026.2 would help: an existing run configuration with "Build Project" with the setting on and off, adding a new before-run step, and editing an existing one.
There was a problem hiding this comment.
Was included on accident, will be merged as separate PR with build routing changes.
| standardBuilder: String, | ||
| offerDownload: Boolean | ||
| ) { | ||
| val reported = synchronized(project) { |
There was a problem hiding this comment.
Nit: synchronized(project) locks on the Project object, which the platform may lock on too. putUserDataIfAbsent (or a ConcurrentHashMap.newKeySet()) avoids that.
There was a problem hiding this comment.
Was included on accident, will be merged as separate PR with build routing changes.
| * Locates and validates the installation for [flavor]. Blocking (registry query, file reads | ||
| * and a short-lived BuildConsole process), so never call it on the EDT. | ||
| */ | ||
| fun check(flavor: BuildFlavor): InstallationStatus { |
There was a problem hiding this comment.
check() spawns BuildConsole /QueryLicense (up to 5s) on every IDE build in all three IDEs, and in Rider it runs inside a non-cancellable modal progress (prepareForIdeAction) together with MsBuildLocator.locate()'s vswhere call (up to 15s). That makes every Rider Build noticeably slower. Please cache the installation status and the MSBuild location per session, and clear them on Re-check or when settings change - this also covers my earlier vswhere caching comment.
There was a problem hiding this comment.
Was included on accident, will be merged as separate PR with build routing changes.
| * (`InitiatorRegType`) or CI (`CIInitiatorRegType`) initiator registration is anything but | ||
| * "None" - a machine can start builds with either. Null if neither line is present. | ||
| */ | ||
| internal fun parseLicenseQuery(output: String): Boolean? { |
There was a problem hiding this comment.
Does InitiatorRegType None reliably mean the machine can't start a build? With floating licenses, can a machine show None and still get a license allocated when it builds? If so, those users would silently fall back to the IDE's own build. Could someone from the Windows side confirm what that value means?
There was a problem hiding this comment.
Was included on accident, will be merged as separate PR with build routing changes.
| * rather than be called back. Blocks until nothing is running and returns true, or returns | ||
| * false if the user chose not to stop the running build. | ||
| */ | ||
| internal fun awaitNoBuildRunning(project: Project): Boolean { |
There was a problem hiding this comment.
Possible permanent hang: if the running process never exits after it's stopped, this waits forever (waitFor() has no timeout), and in whenNoBuildRunning neither start nor onDeclined is ever called. CLion's runner (outcome.get()) and Rider's before-run step would then block forever and the IDE's build never completes. Please add a timeout and report the build as aborted when it expires.
There was a problem hiding this comment.
Good catch. Both wait paths now have a 2-minute stop timeout (STOP_TIMEOUT_MS). It's generous because a graceful Incredibuild cancel lets the build system wind down on its own, but it's finite.
whenNoBuildRunning: a scheduled timeout competes with the "old build ended" callback, and whichever fires first wins. On timeout the new build isn't started, onNotStarted runs, and a warning notification explains why.
awaitNoBuildRunning: waitFor(STOP_TIMEOUT_MS). On timeout it notifies and returns false, which the RustRover runner reports to the IDE as ABORTED.
The CLion runner and Rider before-run step have moved to the separate IDE-build-routing PR, but they go through the same run(), so on timeout they get NotStarted and report an aborted build rather than hanging. The old process isn't killed on timeout, so stopping stays graceful.
| * interface is internal API the Plugin Verifier rejects; nothing can be built before a project is | ||
| * open anyway. Idempotent, so later projects find the wrappers already in place. | ||
| */ | ||
| class DotNetIdeBuildActionInstaller : ProjectActivity { |
There was a problem hiding this comment.
replaceAction on Rider's own actions is never undone. If the plugin is unloaded or updated without a restart, the wrappers stay and point at classes that are gone. Either restore the originals on unload (e.g. a DynamicPluginListener) or mark the plugin as requiring a restart. Also, was it checked that the toolbar button, Build menu and keyboard shortcuts actually reach the wrapper? Menu groups can keep the old action instance.
There was a problem hiding this comment.
Was included on accident, will be merged as separate PR with build routing changes.
736e44f accidentally included the "route the IDE's own Build/Rebuild through Incredibuild" work (CLion/Rider IDE-build interception, installation check and settings row, fallback notifications). It will come in a separate PR; the review fixes from that commit are kept. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks. Splitting the IDE build routing into its own PR makes this much easier to review. The revert is clean, and making Before I approve, please reply to:
The threads on the reverted files can be resolved. Let's pick those up in the follow-up PR. |
udif2025
left a comment
There was a problem hiding this comment.
Thanks. The stop timeout fixes the hang, and verifying 2026.1 as well was worth it: it found the RustRover 2026.1 NoSuchMethodError. Rider 2026.1 passing the Verifier answers my compatibility question.
Approving. A few non-blocking follow-ups:
- Since the Verifier can't see the reflective
CargoBuildAdapterlookup, please run one IDE Build Project in RustRover 2026.1 with "Accelerate the IDE's own build actions" on, to confirm the fallback constructor path works. - The two first-round threads: Linux installer, and a CLion check after the
cmdQuotedchange.
- disambiguatedLabel moved to WorkspaceLabels.kt and only checks the other
workspace when both the Rust and CMake plugins are loaded; RustRover (no CMake
plugin) threw NoClassDefFoundError for CMakeWorkspace on every menu update.
- Cargo builds put the folder of RustRover's configured cargo first on PATH, so
the bare "cargo" command is found even when it isn't on PATH already
("'cargo' is not recognized").
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
While checking RustRover, issue was found that causes builds to fail is CLion cmdQuoted sanity checks were already run as stated and Linux installer was clarified in original comment. |
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.