Rust Integration#1827
Conversation
Coverage Report for CI Build 24905160743Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Coverage decreased (-0.1%) to 70.538%Details
Uncovered ChangesNo uncovered changes found. Coverage Regressions9 previously-covered lines in 2 files lost coverage.
Coverage Stats💛 - Coveralls |
MarvMa
left a comment
There was a problem hiding this comment.
Nice work 🥳 Rust will be a nice addition to IDEasy.
|
I tried to install rust on MacOS ARM64 and got the following error message: |
|
I was able to install it on linux with no issues and it worked perfectly fine. Great work! |
hohwille
left a comment
There was a problem hiding this comment.
@tmukh thanks for your PR and bringing rust support to IDEasy. 👍
Since you already left the team, my suggestions is that we merge this PR and test it after the merge when URL updater did its job.
We can then also create a cleanup issue to address my review comments.
| protected void performToolInstallation(ToolInstallRequest request, Path installationPath) { | ||
|
|
||
| VersionIdentifier resolvedVersion = request.getRequested().getResolvedVersion(); | ||
| FileAccess fileAccess = this.context.getFileAccess(); | ||
|
|
||
| if (Files.isDirectory(installationPath)) { | ||
| fileAccess.backup(installationPath); | ||
| } | ||
| fileAccess.mkdirs(installationPath); | ||
|
|
||
| Path cargoHome = installationPath.resolve(".cargo"); | ||
| Path rustupHome = installationPath.resolve(".rustup"); | ||
| fileAccess.mkdirs(cargoHome); | ||
| fileAccess.mkdirs(rustupHome); | ||
|
|
||
| Path installerScript = downloadTool(request.getRequested().getEdition().edition(), resolvedVersion); | ||
| if (Files.isDirectory(installerScript)) { | ||
| // ToolRepositoryMock may provide an unpacked folder instead of a single download file. | ||
| installerScript = installerScript.resolve("content.sh"); | ||
| } | ||
|
|
||
| List<String> installerArgs = getRustupInstallerArgs(resolvedVersion); | ||
| ProcessContext process = request.getProcessContext().createChild().errorHandling(ProcessErrorHandling.THROW_CLI).directory(installationPath) | ||
| .withEnvVar("CARGO_HOME", cargoHome.toString()).withEnvVar("RUSTUP_HOME", rustupHome.toString()); | ||
| if (isWindowsExeInstaller(installerScript)) { | ||
| Path installerExecutable = installerScript; | ||
| String fileName = installerScript.getFileName().toString(); | ||
| if (!WINDOWS_RUSTUP_INIT_EXE.equalsIgnoreCase(fileName)) { | ||
| Path canonicalInstaller = installerScript.resolveSibling(WINDOWS_RUSTUP_INIT_EXE); | ||
|
|
||
| // Handle corrupted installations where rustup-init.exe might exist as a file or directory | ||
| if (Files.exists(canonicalInstaller, LinkOption.NOFOLLOW_LINKS)) { | ||
| LOG.info("Found existing installer at {}, checking type", canonicalInstaller); | ||
| boolean isDirectory = Files.isDirectory(canonicalInstaller, LinkOption.NOFOLLOW_LINKS); | ||
| LOG.info("Existing installer is {} (directory: {}), deleting it", canonicalInstaller, isDirectory); | ||
|
|
||
| try { | ||
| // First attempt: delete using fileAccess which handles files, directories, and symlinks | ||
| fileAccess.delete(canonicalInstaller); | ||
| LOG.debug("Successfully deleted existing installer at {}", canonicalInstaller); | ||
| } catch (IllegalStateException e) { | ||
| LOG.warn("First deletion attempt failed for {}, retrying: {}", canonicalInstaller, e.getMessage()); | ||
| // Retry in case of transient lock issues | ||
| try { | ||
| fileAccess.delete(canonicalInstaller); | ||
| LOG.debug("Successfully deleted existing installer on retry at {}", canonicalInstaller); | ||
| } catch (IllegalStateException e2) { | ||
| LOG.error("Failed to delete {} after retry: {}", canonicalInstaller, e2.getMessage(), e2); | ||
| throw new IllegalStateException("Failed to clean up corrupted installer at " + canonicalInstaller | ||
| + " (may be locked by another process or permission denied)", e2); | ||
| } | ||
| } | ||
|
|
||
| // Verify deletion was successful | ||
| if (Files.exists(canonicalInstaller, LinkOption.NOFOLLOW_LINKS)) { | ||
| boolean stillIsDirectory = Files.isDirectory(canonicalInstaller, LinkOption.NOFOLLOW_LINKS); | ||
| throw new IllegalStateException("Failed to delete corrupted installer at " + canonicalInstaller | ||
| + " (is directory: " + stillIsDirectory + ", may be locked by another process)"); | ||
| } | ||
| } | ||
|
|
||
| fileAccess.copy(installerScript, canonicalInstaller, FileCopyMode.COPY_FILE_TO_TARGET_OVERRIDE); | ||
| installerExecutable = canonicalInstaller; | ||
| } | ||
| process.executable(installerExecutable).addArgs(installerArgs); | ||
| } else { | ||
| String installerScriptArg = installerScript.toAbsolutePath().toString(); | ||
| process.executable(this.context.findBashRequired()).addArgs(installerScriptArg).addArgs(installerArgs); | ||
| } | ||
| process.run(); | ||
|
|
||
| Path cargoBin = cargoHome.resolve("bin"); | ||
| Path toolBin = installationPath.resolve("bin"); | ||
| if (Files.exists(toolBin, LinkOption.NOFOLLOW_LINKS)) { | ||
| fileAccess.delete(toolBin); | ||
| } | ||
| if (Files.isDirectory(cargoBin)) { | ||
| fileAccess.symlink(cargoBin, toolBin); | ||
| } | ||
|
|
||
| this.context.writeVersionFile(resolvedVersion, installationPath); | ||
| LOG.debug("Installed {} in version {} at {}", this.tool, resolvedVersion, installationPath); | ||
| } |
There was a problem hiding this comment.
This looks like a lot of copy&paste from the overridden method.
IMHO it is not a good idea to implement it this way.
What if we change the logic in the super-class?
The general process with downloading, optional extraction, post-extract-hook logic, writing version file, etc. should not be duplicated.
Whenever we had such situations we added some flexibility to the super-class so we can hook into smaller special aspects in sub-class to customise what we needed.
Co-authored-by: Jörg Hohwiller <hohwille@users.noreply.github.com>
|
Cant be merged cause of compile error: Collided with #1050. |
|
|
||
| // Skip heavyweight MSVC installer execution in tests. | ||
| } | ||
| } |
There was a problem hiding this comment.
We should not do things like this but test the original commandlet.
My initial suggestion was that we add a dependencies.json and add a dependency to visual-studio-build-tools.
Then we create a VisualStudioBuildTools commandlet with that name that performs that installation on Windows and does nothing on any other OS.
Then we could even add the same dependency to GraalVM and can build native images without manually installing the VS BuildTools as currently described in https://github.com/devonfw/IDEasy/blob/main/documentation/contributing/graalvm-build-guide.adoc
This PR Fixes #1719
ide install rust1.1.0) instead of always resolving to the defaultrelease-0.7.Checklist for this PR
Make sure everything is checked before merging this PR. For further info please also see our DoD.
mvn clean testlocally all tests pass and build is successful#«issue-id»: «brief summary»(e.g.#921: fixed setup.bat). If no issue ID exists, title only.In Progressand assigned to you or there is no issue (might happen for very small PRs)internalChecklist for tool commandletsHave you added a new
«tool»as commandlet? There are the following additional checks:«tool»«TOOL»_VERSIONand«TOOL»_EDITIONare honored by your commandlet