Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#25: implement tool commandlet for intellij #297

Open
wants to merge 55 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
13a2b75
#25: implemented install and plugins
ndemirca Feb 23, 2024
b8292b6
Merge branch 'main' into feature/25-Implement-ToolCommandlet-for-Inte…
ndemirca Mar 7, 2024
bcf0e4b
added first windows unittests for intellij
ndemirca Mar 8, 2024
1194c04
tested intellij installation
ndemirca Mar 8, 2024
2213057
adapted test data for intellij commandlet
ndemirca Mar 13, 2024
4329115
Adapted intellij tests
ndemirca Mar 13, 2024
b1f7e9c
Merge branch 'main' into feature/25-Implement-ToolCommandlet-for-Inte…
ndemirca Apr 17, 2024
a1ae18a
adjusted test data
ndemirca Apr 17, 2024
3402cc7
changed test data for mac
ndemirca Apr 17, 2024
2d39454
fixed intellij Unittests
ndemirca Apr 17, 2024
0bd0b42
expanden intellij tool commandlet
ndemirca Apr 17, 2024
1599acb
added javadoc
ndemirca Apr 18, 2024
2dbd602
Merge branch 'main' into feature/25-Implement-ToolCommandlet-for-Inte…
ndemirca Apr 23, 2024
c4ae3cf
added improvements to intellij and optimized tests
ndemirca Apr 23, 2024
e26ce93
cleaned up test data
ndemirca Apr 23, 2024
7b63574
imroved tests
ndemirca Apr 23, 2024
93265c9
fixed mac implementation
ndemirca Apr 24, 2024
dd5b979
added file mode to extractDmg to fix a bug
ndemirca Apr 24, 2024
ab2bedd
improved mac implementation
ndemirca Apr 24, 2024
5f5b2a9
refactoring
ndemirca Apr 24, 2024
a87dde9
adapted tests
ndemirca Apr 24, 2024
ca10f69
fixed unittests for mac
ndemirca Apr 25, 2024
6e46659
fixed unittest
ndemirca Apr 25, 2024
053faed
Merge branch 'main' into feature/25-Implement-ToolCommandlet-for-Inte…
ndemirca Apr 25, 2024
35012f0
small improvements
ndemirca Apr 25, 2024
b9af487
moved open into bin folder
ndemirca Apr 25, 2024
13c526f
Merge branch 'main' into feature/25-Implement-ToolCommandlet-for-Inte…
ndemirca Apr 29, 2024
c9af2b5
fix git merge conflict leftover
ndemirca Apr 29, 2024
7733bfc
Merge branch 'feature/25-Implement-ToolCommandlet-for-Intellij' of ht…
ndemirca Apr 29, 2024
4bbb95b
fixed gitattributes
ndemirca Apr 29, 2024
2c8820a
Merge branch 'main' into feature/25-Implement-ToolCommandlet-for-Inte…
ndemirca Apr 30, 2024
9ed18b4
added gitkeep into gitignore
ndemirca Apr 30, 2024
789f5c7
restover from uninstall commandlet
ndemirca Apr 30, 2024
5463998
refactoring
ndemirca Apr 30, 2024
a3cb929
added CR improvements
ndemirca Apr 30, 2024
1c25fb4
added improvements
ndemirca May 3, 2024
02ce32c
added line endings
ndemirca May 7, 2024
1589f88
Merge branch 'main' into feature/25-Implement-ToolCommandlet-for-Inte…
ndemirca May 7, 2024
9f5d2f7
fixed mac implementation
ndemirca May 7, 2024
1664414
fixed and cleaned up tests
ndemirca May 8, 2024
548357d
manipulated jmc test file
ndemirca May 8, 2024
35c7d63
reverted test step for jmc tests
ndemirca May 8, 2024
480a472
added node_modules into gittignore
ndemirca May 8, 2024
408b8e7
Merge branch 'main' into feature/25-Implement-ToolCommandlet-for-Inte…
ndemirca May 8, 2024
30e4048
Merge branch 'main' into feature/25-Implement-ToolCommandlet-for-Inte…
ndemirca May 17, 2024
20e8710
fixed copy bug in extractdmg
ndemirca May 23, 2024
a45b108
changed binarypath for mac
ndemirca May 27, 2024
bf7fa79
fixed copy method within extractdmg
ndemirca May 27, 2024
26e902f
adapted mac binary path
ndemirca May 27, 2024
3382e54
#347: fixed copy
jan-vcapgemini May 27, 2024
40eb875
Merge branch 'feature/347-Change-copy-method' into feature/25-Impleme…
ndemirca May 27, 2024
30062f3
fixed copy leftover and refactoring
ndemirca May 28, 2024
ad6f549
reverted refactoring
ndemirca May 28, 2024
63793c8
fixed mac tests
ndemirca May 28, 2024
9ca87e3
Merge branch 'main' into feature/25-Implement-ToolCommandlet-for-Inte…
hohwille Jun 13, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,6 @@
*.tar binary
*.bz2 binary
*.gz binary

# special handling for test files
cli/src/test/resources/ide-projects/intellij/repository/intellij/intellij/default/windows/bin/studio64.exe text
Copy link
Member

Choose a reason for hiding this comment

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

is this full path really required? Assuming we do some small refactoring and the path changes nobody will remember to also fix this path here?

Suggested change
cli/src/test/resources/ide-projects/intellij/repository/intellij/intellij/default/windows/bin/studio64.exe text
studio64.exe text

2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*.bak
.*
!.gitignore
!.gitkeep
!.ide.software.version
!.devon.software.version
!.ide
Expand All @@ -15,6 +16,7 @@
target/
eclipse-target/
generated/
node_modules

# Package Files #
*.jar
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -662,7 +662,8 @@ public void extractDmg(Path file, Path targetDir) {
if (appPath == null) {
throw new IllegalStateException("Failed to unpack DMG as no MacOS *.app was found in file " + file);
}
copy(appPath, targetDir);

copy(appPath, targetDir, FileCopyMode.COPY_TREE_OVERRIDE_TREE);
pc.addArgs("detach", "-force", mountPath);
pc.run();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,33 @@
package com.devonfw.tools.ide.tool.intellij;

import com.devonfw.tools.ide.cli.CliArgument;
import com.devonfw.tools.ide.common.Tag;
import com.devonfw.tools.ide.context.IdeContext;
import com.devonfw.tools.ide.io.FileAccessImpl;
import com.devonfw.tools.ide.process.ProcessMode;
import com.devonfw.tools.ide.tool.ide.IdeToolCommandlet;
import com.devonfw.tools.ide.tool.ide.PluginDescriptor;
import com.devonfw.tools.ide.tool.java.Java;
import com.devonfw.tools.ide.version.VersionIdentifier;

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.PosixFilePermission;
import java.nio.file.attribute.PosixFilePermissions;
import java.util.Set;

/**
* {@link IdeToolCommandlet} for <a href="https://www.jetbrains.com/idea/">IntelliJ</a>.
*/
public class Intellij extends IdeToolCommandlet {

private static final String IDEA = "idea";

private static final String IDEA64_EXE = IDEA + "64.exe";

private static final String IDEA_BASH_SCRIPT = IDEA + ".sh";

/**
* The constructor.
*
Expand All @@ -23,6 +38,27 @@ public Intellij(IdeContext context) {
super(context, "intellij", Set.of(Tag.INTELLIJ));
}

@Override
public void runTool(ProcessMode processMode, VersionIdentifier toolVersion, String... args) {

install(true);
args = CliArgument.prepend(args, this.context.getWorkspacePath().toString());
super.runTool(processMode, toolVersion, args);
}

@Override
protected String getBinaryName() {

Path toolBinPath = getToolBinPath();
if (this.context.getSystemInfo().isWindows()) {
return toolBinPath.resolve(IDEA64_EXE).toString();
} else if (this.context.getSystemInfo().isLinux()) {
return toolBinPath.resolve(IDEA_BASH_SCRIPT).toString();
} else {
return getToolPath().resolve("IntelliJ IDEA CE.app").resolve("Contents").resolve("MacOS").resolve(IDEA).toString();
Copy link
Member

@hohwille hohwille Jun 13, 2024

Choose a reason for hiding this comment

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

IMHO this is still wrong as we discussed in our meeting.
I already merged the PR #359 and resolved the conflicts into this PR.
If we did everything correct there is no such thing anymore than IntelliJ IDEA CA.app inside the software folder.
Also please be aware the different editions of intelliJ will have a different *.app name so your code would never work with the ultimate edition used by many projects (CE stands for Community Edition).
I hope it was already clarified meanwhile why we need the generic MacOS workaround and avoid such ugly OS-switches here and handling of MacOS apps in individual commandlets.
Ideally like in devonfw-ide after installation we can just call idea on any plattform to launch IntelliJ.
This PR therefore still needs some reworking...

}
}

@Override
public boolean install(boolean silent) {

Expand All @@ -31,10 +67,31 @@ public boolean install(boolean silent) {
}

@Override
public void installPlugin(PluginDescriptor plugin) {
protected void postInstall() {

super.postInstall();
if (this.context.getSystemInfo().isMac()) {
setMacOsFilePermissions(getToolPath().resolve("IntelliJ IDEA CE.app").resolve("Contents").resolve("MacOS").resolve(IDEA));
}
Comment on lines +73 to +75
Copy link
Member

Choose a reason for hiding this comment

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

same here...

}

// TODO Auto-generated method stub
private static void setMacOsFilePermissions(Path binaryFile) {

if (Files.exists(binaryFile)) {
String permissionStr = FileAccessImpl.generatePermissionString(493);
Copy link
Member

Choose a reason for hiding this comment

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

Seems I missed to discuss on this API.
IMHO this is very bad design:

  • numeric file permissions must always be given in octal notation or nobody can understand what 493 actually is. If you write it octal as 0755 at least some bash/unix experienced user will understand what this.
  • We should avoid direct usage of FileAccessImpl in commandlets. That is why we have FileAccess interface.
  • Why do we provide a method in FileAccessImpl that converts a number to a String and then we again convert the String to a Set of PosixFilePermissions in order to then natively set this via Files.setPosixFilePermissions? The idea of FileAccess is to create a higher level API and abstraction to such lower-level Java NIO operations and make our live easier. So if we believe that using numeric (octal) notation is the most readable way why dont we create a method in FileAccess to set file permissions to a given file with a given numeric value? However, it may be even simpler and more readable to simply define constants for the combinations that we need like this:
/** {@link PosixFilePermission}s for "rwxr-xr-x" or 0755. */
public static Set<PosixFilePermission> RWX_RX_RX = Set.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE, PosixFilePermission.OWNER_EXECUTE, PosixFilePermission.GROUP_READ, PosixFilePermission.GROUP_EXECUTE, PosixFilePermission.OTHERS_READ, PosixFilePermission.OTHERS_EXECUTE);
  • My real suggestion would rather be to add a method FileAccess.makeExecutable(Path) that will simply implement what chmod a+x «path» will do and could also be implemented to work on Windows (see also here).

Set<PosixFilePermission> permissions = PosixFilePermissions.fromString(permissionStr);
try {
Files.setPosixFilePermissions(binaryFile, permissions);
} catch (IOException e) {
throw new RuntimeException(e);
} finally {
return;
Comment on lines +87 to +88
Copy link
Member

Choose a reason for hiding this comment

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

This does not seem to add any value and is IMHO only causing confusion.

Suggested change
} finally {
return;

}
}
}

}
@Override
public void installPlugin(PluginDescriptor plugin) {
Copy link
Member

Choose a reason for hiding this comment

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

This is not implemented so this PR does not entirely close #25. If you do something like this on purpose, then create a new issue for IntelliJ plugin support and leave a comment here like TODO https://github.com/devonfw/IDEasy/issues/«id». Otherwise we still end up in the same situation that we have currently that some comandlets seem to be present but are incomplete and if we have no issue about the problem, we might forget about it and currently merging this PR would automatically close #25.


}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
package com.devonfw.tools.ide.tool.intellij;

import com.devonfw.tools.ide.context.AbstractIdeContextTest;
import com.devonfw.tools.ide.context.IdeTestContext;
import com.devonfw.tools.ide.log.IdeLogLevel;
import com.devonfw.tools.ide.os.SystemInfo;
import com.devonfw.tools.ide.os.SystemInfoMock;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;

import java.nio.file.Path;

/**
* Integration test of {@link Intellij}.
*/
public class IntellijTest extends AbstractIdeContextTest {

private static final String PROJECT_INTELLIJ = "intellij";

private final IdeTestContext context = newContext(PROJECT_INTELLIJ);

/**
* Tests if the {@link Intellij} can be installed properly.
*
* @param os String of the OS to use.
*/
@ParameterizedTest
@ValueSource(strings = { "windows", "mac", "linux" })
public void testIntellijInstall(String os) {

// arrange
SystemInfo systemInfo = SystemInfoMock.of(os);
context.setSystemInfo(systemInfo);
Intellij commandlet = new Intellij(context);

// act
commandlet.install();

// assert
checkInstallation(context);

//if tool already installed
commandlet.install();
assertLogMessage(context, IdeLogLevel.DEBUG, "Version 2023.3.3 of tool intellij is already installed");
}

/**
* Tests if {@link Intellij IntelliJ IDE} can be run.
*
* @param os String of the OS to use.
*/
@ParameterizedTest
@ValueSource(strings = { "windows", "mac", "linux" })
public void testIntellijRun(String os) {
// arrange
SystemInfo systemInfo = SystemInfoMock.of(os);
context.setSystemInfo(systemInfo);
Intellij commandlet = new Intellij(context);

// act
commandlet.run();

// assert
SystemInfo currentSystemInfo = context.getSystemInfo();
Path workspacePath = this.context.getWorkspacePath();

if (currentSystemInfo.isMac()) {
assertLogMessage(context, IdeLogLevel.INFO, "intellij mac " + workspacePath);
} else if (currentSystemInfo.isLinux()) {
assertLogMessage(context, IdeLogLevel.INFO, "intellij linux " + workspacePath);
} else if (currentSystemInfo.isWindows()) {
assertLogMessage(context, IdeLogLevel.INFO, "intellij windows " + workspacePath);
}
checkInstallation(context);
}

private void checkInstallation(IdeTestContext context) {

assertThat(context.getSoftwarePath().resolve("intellij/.ide.software.version")).exists().hasContent("2023.3.3");
assertLogMessage(context, IdeLogLevel.SUCCESS, "Successfully installed java in version 17.0.10_7");
assertLogMessage(context, IdeLogLevel.SUCCESS, "Successfully installed intellij in version 2023.3.3");
}
}

This file was deleted.

This file was deleted.

This file was deleted.

Binary file not shown.