Skip to content

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

Merged
agorielikov merged 13 commits into
mainfrom
feature/WIN-24578-add-rider-plugin-support
Oct 28, 2026
Merged

agorielikov merged 13 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?

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 .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.

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.

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 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

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.

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. 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) {

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.

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.

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(

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.

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.

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 {

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.

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.

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.

@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

@agorielikov
agorielikov requested a review from udif2025 October 27, 2026 08:12

@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.

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

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 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?

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.

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

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 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.

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.

Was included on accident, will be merged as separate PR with build routing changes.

standardBuilder: String,
offerDownload: Boolean
) {
val reported = synchronized(project) {

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: synchronized(project) locks on the Project object, which the platform may lock on too. putUserDataIfAbsent (or a ConcurrentHashMap.newKeySet()) avoids that.

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.

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 {

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.

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.

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.

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? {

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.

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?

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.

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 {

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.

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.

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. 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 {

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.

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.

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.

Was included on accident, will be merged as separate PR with build routing changes.

agorielikov and others added 2 commits October 28, 2026 13:37
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>
@udif2025

Copy link
Copy Markdown
Collaborator

Thanks. Splitting the IDE build routing into its own PR makes this much easier to review. The revert is clean, and making installers.yml reusable with dispatch and PR triggers is a good fix (it passed on this PR). I'll check the wiki and external download links for the old scripts/ path.

Before I approve, please reply to:

  • The awaitNoBuildRunning timeout comment. It still applies after the revert, to RustRover IDE builds and to the stop-then-start path for menu builds.
  • The three first-round threads: Rider 2026.1 compatibility range, Linux installer, and a CLion check after the cmdQuoted change.

The threads on the reverted files can be resolved. Let's pick those up in the follow-up PR.

@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.

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 CargoBuildAdapter lookup, 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 cmdQuoted change.

- 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>
@agorielikov

Copy link
Copy Markdown
Collaborator Author

While checking RustRover, issue was found that causes builds to fail is cargo is not under PATH (while building without plugin would work). This is not a regression, but latest commit fixes it.

CLion cmdQuoted sanity checks were already run as stated and Linux installer was clarified in original comment.

@agorielikov
agorielikov merged commit de05dd1 into main Oct 28, 2026
9 checks passed
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