More shell rework

docmalitt · Aug 10, 2024 · 1caa6ca · 1caa6ca
1 parent 99971ca
commit 1caa6ca
Show file tree

Hide file tree

Showing 11 changed files with 41 additions and 138 deletions.
diff --git a/app/src/main/java/io/xpipe/app/beacon/impl/ShellStartExchangeImpl.java b/app/src/main/java/io/xpipe/app/beacon/impl/ShellStartExchangeImpl.java
@@ -36,6 +36,7 @@ public Object handle(HttpExchange exchange, Request msg) {
                 .osType(control.getOsType())
                 .osName(control.getOsName())
                 .temp(control.getSystemTemporaryDirectory())
+                .ttyState(control.getTtyState())
                 .build();
     }
 }
diff --git a/app/src/main/java/io/xpipe/app/comp/store/StoreCreationMenu.java b/app/src/main/java/io/xpipe/app/comp/store/StoreCreationMenu.java
@@ -40,7 +40,7 @@ public static void addButtons(MenuButton menu) {
         menu.getItems()
                 .add(category("addTunnel", "mdi2v-vector-polyline-plus", DataStoreCreationCategory.TUNNEL, null));
 
-        // menu.getItems().add(category("addCommand", "mdi2c-code-greater-than", DataStoreCreationCategory.COMMAND, "cmd"));
+        menu.getItems().add(category("addCommand", "mdi2c-code-greater-than", DataStoreCreationCategory.COMMAND, "cmd"));
 
         menu.getItems().add(category("addSerial", "mdi2s-serial-port", DataStoreCreationCategory.SERIAL, "serial"));
 

diff --git a/app/src/main/java/io/xpipe/app/util/DataStoreFormatter.java b/app/src/main/java/io/xpipe/app/util/DataStoreFormatter.java
@@ -6,6 +6,7 @@
 import io.xpipe.core.process.ShellDialects;
 import io.xpipe.core.process.ShellStoreState;
 
+import io.xpipe.core.process.ShellTtyState;
 import javafx.beans.value.ObservableValue;
 
 public class DataStoreFormatter {
@@ -41,7 +42,8 @@ public static ObservableValue<String> shellInformation(StoreEntryWrapper w) {
                     return s.getShellDialect().getDisplayName();
                 }
 
-                return s.isRunning() ? formattedOsName(s.getOsName()) : "Connection failed";
+                var prefix = s.getTtyState() != ShellTtyState.NONE ? "[PTY] " : "";
+                return s.isRunning() ? prefix + formattedOsName(s.getOsName()) : "Connection failed";
             }
 
             return "?";

diff --git a/app/src/main/resources/io/xpipe/app/resources/misc/api.md b/app/src/main/resources/io/xpipe/app/resources/misc/api.md
@@ -1673,6 +1673,7 @@ These errors will be returned with the HTTP return code 500.
   "shellDialect": 0,
   "osType": "string",
   "osName": "string",
+  "ttyState": "string",
   "temp": "string"
 }
 ```
@@ -2969,6 +2970,7 @@ undefined
   "shellDialect": 0,
   "osType": "string",
   "osName": "string",
+  "ttyState": "string",
   "temp": "string"
 }
 
@@ -2981,6 +2983,7 @@ undefined
 |shellDialect|integer|true|none|The shell dialect|
 |osType|string|true|none|The general type of operating system|
 |osName|string|true|none|The display name of the operating system|
+|ttyState|string|false|none|Whether a tty/pty has been allocated for the connection. If allocated, input and output will be unreliable. It is not recommended to use a shell connection then.|
 |temp|string|true|none|The location of the temporary directory|
 
 <h2 id="tocS_ShellStopRequest">ShellStopRequest</h2>

diff --git a/beacon/src/main/java/io/xpipe/beacon/api/ShellStartExchange.java b/beacon/src/main/java/io/xpipe/beacon/api/ShellStartExchange.java
@@ -3,6 +3,7 @@
 import io.xpipe.beacon.BeaconInterface;
 import io.xpipe.core.process.OsType;
 import io.xpipe.core.process.ShellDialect;
+import io.xpipe.core.process.ShellTtyState;
 import io.xpipe.core.store.FilePath;
 
 import lombok.Builder;
@@ -40,6 +41,9 @@ public static class Response {
         @NonNull
         String osName;
 
+        @NonNull
+        ShellTtyState ttyState;
+
         @NonNull
         FilePath temp;
     }

diff --git a/core/src/main/java/io/xpipe/core/process/OsType.java b/core/src/main/java/io/xpipe/core/process/OsType.java
@@ -4,8 +4,6 @@
 
 import java.util.List;
 import java.util.Locale;
-import java.util.Map;
-import java.util.stream.Collectors;
 
 public interface OsType {
 
@@ -36,12 +34,6 @@ static Local getLocal() {
 
     String getName();
 
-    String getTempDirectory(ShellControl pc) throws Exception;
-
-    Map<String, String> getProperties(ShellControl pc) throws Exception;
-
-    String determineOperatingSystemName(ShellControl pc) throws Exception;
-
     sealed interface Local extends OsType permits OsType.Windows, OsType.Linux, OsType.MacOs {
 
         String getId();
@@ -87,51 +79,6 @@ public String getName() {
             return "Windows";
         }
 
-        @Override
-        public String getTempDirectory(ShellControl pc) throws Exception {
-            var def = pc.executeSimpleStringCommand(pc.getShellDialect().getPrintEnvironmentVariableCommand("TEMP"));
-            if (!def.isBlank() && pc.getShellDialect().directoryExists(pc, def).executeAndCheck()) {
-                return def;
-            }
-
-            var fallback = pc.executeSimpleStringCommand(
-                    pc.getShellDialect().getPrintEnvironmentVariableCommand("LOCALAPPDATA"));
-            if (!fallback.isBlank()
-                    && pc.getShellDialect().directoryExists(pc, fallback).executeAndCheck()) {
-                return fallback;
-            }
-
-            return def;
-        }
-
-        @Override
-        public Map<String, String> getProperties(ShellControl pc) throws Exception {
-            try (CommandControl c = pc.command("systeminfo").start()) {
-                var text = c.readStdoutOrThrow();
-                return PropertiesFormatsParser.parse(text, ":");
-            }
-        }
-
-        @Override
-        public String determineOperatingSystemName(ShellControl pc) {
-            try {
-                return pc.executeSimpleStringCommand("wmic os get Caption")
-                                .lines()
-                                .skip(1)
-                                .collect(Collectors.joining())
-                                .trim()
-                        + " "
-                        + pc.executeSimpleStringCommand("wmic os get Version")
-                                .lines()
-                                .skip(1)
-                                .collect(Collectors.joining())
-                                .trim();
-            } catch (Throwable t) {
-                // Just in case this fails somehow
-                return "Windows";
-            }
-        }
-
         @Override
         public String getId() {
             return "windows";
@@ -168,32 +115,6 @@ public String getName() {
             return "Linux";
         }
 
-        @Override
-        public String getTempDirectory(ShellControl pc) {
-            return "/tmp/";
-        }
-
-        @Override
-        public Map<String, String> getProperties(ShellControl pc) {
-            return null;
-        }
-
-        @Override
-        public String determineOperatingSystemName(ShellControl pc) throws Exception {
-            String type = "Unknown";
-            var uname = pc.command("uname -o").readStdoutIfPossible();
-            if (uname.isPresent()) {
-                type = uname.get();
-            }
-
-            String version = "?";
-            var unameR = pc.command("uname -r").readStdoutIfPossible();
-            if (unameR.isPresent()) {
-                version = unameR.get();
-            }
-
-            return type + " " + version;
-        }
     }
 
     final class Linux extends Unix implements OsType, Local, Any {
@@ -203,20 +124,6 @@ public String getId() {
             return "linux";
         }
 
-        @Override
-        public String determineOperatingSystemName(ShellControl pc) throws Exception {
-            var rel = pc.command("lsb_release -a").readStdoutIfPossible();
-            if (rel.isPresent()) {
-                return PropertiesFormatsParser.parse(rel.get(), ":").getOrDefault("Description", "Unknown");
-            }
-
-            var cat = pc.command("cat /etc/*release").readStdoutIfPossible();
-            if (cat.isPresent()) {
-                return PropertiesFormatsParser.parse(cat.get(), "=").getOrDefault("PRETTY_NAME", "Unknown");
-            }
-
-            return super.determineOperatingSystemName(pc);
-        }
     }
 
     final class Solaris extends Unix implements Any {}
@@ -265,38 +172,5 @@ public String getName() {
             return "Mac";
         }
 
-        @Override
-        public String getTempDirectory(ShellControl pc) throws Exception {
-            var found = pc.executeSimpleStringCommand(pc.getShellDialect().getPrintVariableCommand("TMPDIR"));
-
-            // This variable is not defined for root users, so manually fix it. Why? ...
-            if (found.isBlank()) {
-                return "/tmp";
-            }
-
-            return found;
-        }
-
-        @Override
-        public Map<String, String> getProperties(ShellControl pc) throws Exception {
-            try (CommandControl c = pc.command("sw_vers").start()) {
-                var text = c.readStdoutOrThrow();
-                return PropertiesFormatsParser.parse(text, ":");
-            }
-        }
-
-        @Override
-        public String determineOperatingSystemName(ShellControl pc) throws Exception {
-            var properties = getProperties(pc);
-            var name = pc.executeSimpleStringCommand(
-                    "awk '/SOFTWARE LICENSE AGREEMENT FOR macOS/' '/System/Library/CoreServices/Setup "
-                            + "Assistant.app/Contents/Resources/en.lproj/OSXSoftwareLicense.rtf' | "
-                            + "awk -F 'macOS ' '{print $NF}' | awk '{print substr($0, 0, length($0)-1)}'");
-            // For preleases and others
-            if (name.isBlank()) {
-                name = "?";
-            }
-            return properties.get("ProductName") + " " + name + " " + properties.get("ProductVersion");
-        }
     }
 }
diff --git a/core/src/main/java/io/xpipe/core/process/ShellDialect.java b/core/src/main/java/io/xpipe/core/process/ShellDialect.java
@@ -9,6 +9,7 @@
 import java.nio.charset.Charset;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.UUID;
 import java.util.stream.Stream;
 
@@ -120,6 +121,8 @@ default String getConcatenationOperator() {
 
     String getPrintStartEchoCommand(String prefix);
 
+    Optional<String> executeRobustBootstrapOutputCommand(ShellControl shellControl, String original) throws Exception;
+
     String getPrintExitCodeCommand(String id, String prefix, String suffix);
 
     int assignMissingExitCode();

diff --git a/core/src/main/java/io/xpipe/core/process/ShellTtyState.java b/core/src/main/java/io/xpipe/core/process/ShellTtyState.java
@@ -7,19 +7,23 @@
 public enum ShellTtyState {
 
     @JsonProperty("none")
-    NONE(true, false, false),
+    NONE(true, false, false, true, true),
     @JsonProperty("merged")
-    MERGED_STDERR(false, false, false),
+    MERGED_STDERR(false, false, false, false, true),
     @JsonProperty("pty")
-    PTY_ALLOCATED(false, true, true);
+    PTY_ALLOCATED(false, true, true, false, false);
 
     private final boolean hasSeparateStreams;
     private final boolean hasAnsiEscapes;
     private final boolean echoesAllInput;
+    private final boolean supportsInput;
+    private final boolean preservesOutput;
 
-    ShellTtyState(boolean hasSeparateStreams, boolean hasAnsiEscapes, boolean echoesAllInput) {
+    ShellTtyState(boolean hasSeparateStreams, boolean hasAnsiEscapes, boolean echoesAllInput, boolean supportsInput, boolean preservesOutput) {
         this.hasSeparateStreams = hasSeparateStreams;
         this.hasAnsiEscapes = hasAnsiEscapes;
         this.echoesAllInput = echoesAllInput;
+        this.supportsInput = supportsInput;
+        this.preservesOutput = preservesOutput;
     }
 }
diff --git a/dist/changelogs/11.0.md b/dist/changelogs/11.0.md
@@ -1,3 +1,11 @@
+## TTYs and PTYs
+
+Up until now, if you added a connection that always allocated pty, XPipe would complain about a missing stderr.
+In XPipe 11, there has been a ground up rework of the shell initialization code which will in theory allow for better handling of these cases.
+They are not fully supported yet and have some issues, but should work better.
+
+The main concern here is to verify that the existing normal shell implementation still works as before and there were no bugs introduced by this rework.
+
 ## Profiles
 
 You can now create multiple user profiles in the settings menu.

diff --git a/ext/base/src/main/java/io/xpipe/ext/base/store/ShellStoreProvider.java b/ext/base/src/main/java/io/xpipe/ext/base/store/ShellStoreProvider.java
@@ -4,7 +4,6 @@
 import io.xpipe.app.comp.base.OsLogoComp;
 import io.xpipe.app.comp.base.SystemStateComp;
 import io.xpipe.app.comp.base.TtyWarningComp;
-import io.xpipe.app.comp.store.StoreEntryComp;
 import io.xpipe.app.comp.store.StoreEntryWrapper;
 import io.xpipe.app.comp.store.StoreSection;
 import io.xpipe.app.ext.ActionProvider;
@@ -13,13 +12,15 @@
 import io.xpipe.app.fxcomps.Comp;
 import io.xpipe.app.storage.DataStorage;
 import io.xpipe.app.storage.DataStoreEntry;
+import io.xpipe.app.util.DataStoreFormatter;
 import io.xpipe.app.util.TerminalLauncher;
 import io.xpipe.core.process.ShellStoreState;
 import io.xpipe.core.process.ShellTtyState;
 import io.xpipe.core.store.ShellStore;
 import io.xpipe.ext.base.script.ScriptStore;
 import javafx.beans.binding.Bindings;
 import javafx.beans.property.BooleanProperty;
+import javafx.beans.value.ObservableValue;
 
 public interface ShellStoreProvider extends DataStoreProvider {
 
@@ -32,11 +33,6 @@ default Comp<?> createTtyWarning(StoreEntryWrapper w) {
                 w.getPersistentState()));
     }
 
-    @Override
-    default StoreEntryComp customEntryComp(StoreSection s, boolean preferLarge) {
-        return StoreEntryComp.create(s, createTtyWarning(s.getWrapper()), preferLarge);
-    }
-
     @Override
     default ActionProvider.Action launchAction(DataStoreEntry entry) {
         return new ActionProvider.Action() {
@@ -66,4 +62,9 @@ default Comp<?> stateDisplay(StoreEntryWrapper w) {
     default DataStoreUsageCategory getUsageCategory() {
         return DataStoreUsageCategory.SHELL;
     }
+
+    @Override
+    default ObservableValue<String> informationString(StoreSection section) {
+        return DataStoreFormatter.shellInformation(section.getWrapper());
+    }
 }
diff --git a/openapi.yaml b/openapi.yaml
@@ -627,6 +627,9 @@ components:
         osName:
           type: string
           description: The display name of the operating system
+        ttyState:
+          type: string
+          description: Whether a tty/pty has been allocated for the connection. If allocated, input and output will be unreliable. It is not recommended to use a shell connection then.
         temp:
           type: string
           description: The location of the temporary directory