Refactor Subprocess Structure #438

RichiCoder1 · 2017-05-14T07:19:35Z

This PR accomplishes two major things:

It removes the last vestiges of the old style way of how we were doing a number of things in Gui behind the scenes. Also cleans up our async primitives to use the seriously high quality VS.Threading package.
Replaces the buggy and ill convinced WAMP IPC protocol with a much more solid WCF IPC layer. Mostly for my santity, but it has had the nice side effect of crashing less and being ever so slightly faster.

Other things of note:

I redid how we're copying Subprocess into Gui's output. It's much more sane now, (and gets rid of an app.config issue that was there previously). It also means that Main no longer references Subprocess, but instead a shared project to get Dtos.
ChocolateyGui (main) no longer has a dependency on chocolatey.lib (ha). All the choco specific bits have been abstracted and contained in Subprocess.

Lastly, one thing that @ferventcoder and I will both appreciate, you can now start ChocolateyGui.Subprocess before, and independent of, ChocolateyGui. This makes it muuuuch easier to debug Subprocess as you can just set both projects as startup projects in VS (with Subprocess starting first), allowing you to simultaneously attach to both.

Moves models into a shared project, and copys ChocolateyGui.Subprocess using copy rather than the previous (error prone) reference method.

RichiCoder1 · 2017-05-14T07:53:39Z

AppVeyor is angry w/ me because of cake-contrib/Cake.Recipe#97 (of which the short term fix will like be to just stop using a shared project and switch to class lib)

ferventcoder · 2017-05-17T23:02:53Z

I'm all about fixing stuff. Let me take a looksy

ferventcoder · 2017-05-17T23:05:19Z

Source/ChocolateyGui.Shared/Models/Package.cs

        public bool IsInstalled { get; set; }

+        [DataMember]
+


looks like extra lines inserted here. Issues with flipping this away or to UTF8 usually causes this.

ferventcoder · 2017-05-17T23:06:12Z

Source/ChocolateyGui.Shared/Models/PackageOperationResult.cs

        public bool Successful { get; set; }

-        public IReadOnlyList<string> Messages { get; set; } = new string[0];
+        [DataMember]
+        public string[] Messages { get; set; } = new string[0];


So it has to be an array? Can't be a List<string>? (I'm pretty sure IList won't work here).

I honestly don't know haha. It was yelling at me for other things. Will try again.

ferventcoder · 2017-05-17T23:07:50Z

Source/ChocolateyGui.Shared/Models/SemanticVersion.cs

+            return hashCode;
+        }
+    }
+}


Why did you need to implement this? I'd prefer using it directly out of chocolatey.dll.

Unless it is being internalized, which I think we fixed a long time ago.

Nope, this was because I pulled chocolatey.lib out of chocolateygui (so it's only the in the subprocess). However, there's 0 reason I can't just put it back, especially since I'm just using a model.

ferventcoder · 2017-05-17T23:09:01Z

Source/ChocolateyGui.Subprocess/ChocolateyGui.Subprocess.csproj

-    <Reference Include="Nito.AsyncEx, Version=4.0.1.0, Culture=neutral, processorArchitecture=MSIL">
-      <HintPath>..\packages\Nito.AsyncEx.4.0.1\lib\net45\Nito.AsyncEx.dll</HintPath>
+    <Reference Include="Microsoft.VisualStudio.Threading, Version=15.3.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a, processorArchitecture=MSIL">
+      <HintPath>..\packages\Microsoft.VisualStudio.Threading.15.3.20\lib\net45\Microsoft.VisualStudio.Threading.dll</HintPath>


Weird name...

Suprisingly, it's well names. It the Microsoft Visual Studio Team's threading library. It's actually really awesome (hence why I swapped Nito.AsyncEx with it.)

ferventcoder · 2017-05-17T23:10:56Z

Source/ChocolateyGui.Subprocess/packages.config

-  <package id="System.ValueTuple" version="4.3.1" targetFramework="net452" />
-  <package id="WampSharp" version="1.2.5.27-beta" targetFramework="net452" />
-  <package id="WampSharp.Fleck" version="1.2.5.27-beta" targetFramework="net452" />
-  <package id="WampSharp.NewtonsoftJson" version="1.2.5.27-beta" targetFramework="net452" />


That really reduced some dependencies!!

That was 90% of the reason for this change. WampSharp just had some many superflous dependencies and issues.

ferventcoder · 2017-05-17T23:13:26Z

Source/ChocolateyGui/Services/ChocolateyRemotePackageService.cs

-                            }
-
-                            _progressService.WriteMessage(message.Message, powerShellLineType);
-                        });


You don't need any of this now? I'm guessing you have a logger set up.

ferventcoder · 2017-05-17T23:13:54Z

Source/ChocolateyGui/Services/ChocolateyRemotePackageService.cs

+            {
+                Logger.Fatal(ex, "Failed to create client channel to server.");
+                throw;
+            }


ferventcoder · 2017-05-17T23:14:19Z

Source/ChocolateyGui/Services/ServiceCallbackHandler.cs

+            }
+
+            _progressService.WriteMessage(message.Message, powerShellLineType);
+        }


And there it is.

ferventcoder · 2017-05-17T23:14:53Z

Source/ChocolateyGui/ViewModels/SettingsViewModel.cs

+            foreach (var feature in features)
+            {
+                Features.Add(feature);
+            }


Optimization?

Actually could be optimized further. I don't what library I pulled in had it, but ObservableCollection (which backs Features, Settings, and Sources on this page) doesn't have an AddRange, so I had to fall back to good old fashion add.

ferventcoder

I'm mostly 👍 - if you've played with this and everything works appropriately, then I'm all 👍 . Once this is in I can add in the fixes for the background service checks.

RichiCoder1 · 2017-05-18T16:25:34Z

I'll address the interface issues in a later PR. Should be good to go.

Richard Simpson added 3 commits May 13, 2017 21:47

Consolidate models and change subprocess xcopy

1e835e4

Moves models into a shared project, and copys ChocolateyGui.Subprocess using copy rather than the previous (error prone) reference method.

Switch to VS Threading from Nito Async

3b2aa51

Switch to WCF model for IPC.

847180f

RichiCoder1 requested review from ferventcoder and gep13 May 14, 2017 07:19

Appease the angry build gods (and use C#6, not 7)

c36583d

RichiCoder1 mentioned this pull request May 14, 2017

Remove AlphaFS dependency #434

Closed

ferventcoder reviewed May 17, 2017

View reviewed changes

ferventcoder approved these changes May 17, 2017

View reviewed changes

ferventcoder mentioned this pull request May 18, 2017

Add check for background service #382

Closed

Move to ClassLib and remove semanticversion.

1f5ebce

RichiCoder1 merged commit 6bc3ab1 into develop May 18, 2017

RichiCoder1 deleted the feature/Refactor_Process_Structure branch May 18, 2017 16:25

ferventcoder mentioned this pull request Nov 27, 2017

Chocolatey GUI doesn't elevate with a standard Windows user account #511

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor Subprocess Structure #438

Refactor Subprocess Structure #438

RichiCoder1 commented May 14, 2017

RichiCoder1 commented May 14, 2017 •

edited

ferventcoder commented May 17, 2017

ferventcoder May 17, 2017

ferventcoder May 17, 2017

RichiCoder1 May 18, 2017

ferventcoder May 17, 2017

ferventcoder May 17, 2017

RichiCoder1 May 18, 2017

ferventcoder May 17, 2017

RichiCoder1 May 18, 2017

ferventcoder May 17, 2017

RichiCoder1 May 18, 2017

ferventcoder May 17, 2017

ferventcoder May 17, 2017

ferventcoder May 17, 2017

ferventcoder May 17, 2017

RichiCoder1 May 18, 2017

ferventcoder left a comment

RichiCoder1 commented May 18, 2017

Refactor Subprocess Structure #438

Refactor Subprocess Structure #438

Conversation

RichiCoder1 commented May 14, 2017

RichiCoder1 commented May 14, 2017 • edited

ferventcoder commented May 17, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ferventcoder left a comment

Choose a reason for hiding this comment

RichiCoder1 commented May 18, 2017

RichiCoder1 commented May 14, 2017 •

edited