CLI #37

felix-b · 2017-03-29T13:34:47Z

This PR is for task #35. It will include the skeleton of the CLI program and implementation of the service command.

felix-b · 2017-03-29T13:38:04Z

first run of nwheels service -h

…d ICommandContext to allow unit testing ; added unit test project NWheels.Cli.UnitTests.

…ything named 'Publish').

…plemented 'publish' and 'run' commands

- Run determines module assembly search directories - BootConfiguration extended with list of assembly search directories - MicroserviceHost looks up assemblies in search directories if specified in BootConfiguration

…Bin" convention.

…(replaced AssemblyDirectories added earlier).

…all to IWebHost.Run() was blocking, replaced with IWebHost.Start().

…used deadlock, because output buffer was not read while waiting for process to finish.

…han per every module.

…launchSettings.json (project debug settings) doesn't have to change per developer.

felix-b · 2017-04-05T05:50:51Z

Done. Ready for review/merge. See https://github.com/felix-b/NWheels/issues/35#issuecomment-291737360 for final info.

ILyaIL83

So as I see it, bitness is critical, the rest is minor or should be kept in mind and/or covered by integration tests.
What do you think?
I haven't tested Cli in depth, but it looks very useful.

ILyaIL83 · 2017-04-05T12:22:59Z

Source/NWheels.Cli/Publish/PublishCommand.cs

+                ref _environmentFilePath, requireValue: false, 
+                help: "path to environment XML file to use");
+
+            syntax.DefineOption("n|no-cli", 


Why to call publish in development mode at all?(I see that RunCommand calls publish with no-cli)

It is a shortcut. Instead of calling nwheels publish on source folder, and then nwheels run on published folder, you can call nwheels run on source folder, and it will do both things in one command.

ILyaIL83 · 2017-04-05T13:20:46Z

Source/NWheels.Cli/Run/RunCommand.cs

+                return false;
+            }
+
+            var projectElement = XElement.Parse(File.ReadAllText(projectFilePath));


This is very fragile path. Nobody can guarantee that in the future project file's structure won't be changed.

As I mentioned in https://github.com/felix-b/NWheels/issues/35#issuecomment-291737360, in the Current Limitations section, we are waiting for Roslyn project system to be fully ported and consumable from .NET Core applications. Then we can use Roslyn to inspect solution and project files. Roslyn project system provides such classes as this: http://source.roslyn.io/#Microsoft.CodeAnalysis.Workspaces.Desktop/Workspace/MSBuild/MSBuildWorkspace.cs

ILyaIL83 · 2017-04-05T13:43:38Z

Source/NWheels.Cli/Run/RunCommand.cs

+                }
+                else
+                {
+                    LogWarning($"Assembly directory pair could not be parsed: {line}");


When I run Cli project, I've got little bit strange log decoration, despite service went up.
StrangeOutput.txt

This is unexpected output from dotnet msbuid command. I expect to get lines in the format "assembly.dll=path/to/assmebly.dll", but instead I get "Welcome to .NET Core!". Something goes wrong. Can you write me the command you ran, and what was the current folder?

Since we weren't able to consistently reproduce it, let's see when it pops up again. If it will, we will open a bug,

ILyaIL83 · 2017-04-05T13:57:41Z

Source/NWheels.Cli/Run/RunCommand.cs

@@ -0,0 +1,396 @@
+using NWheels.Cli.Publish;


Solution configuration: Debug
Solution platform: Any CPU
Application arguments: run NWheels.Samples.FirstHappyPath --no-publish
I've started Cli with Ctrl+F5(not in debug) and it runs in 32 bit(C:\Windows\SysWOW64\cmd.exe).
While 64 bit default cmd is from C:\Windows\System32.
So, the question is can we run in 64 and why it's in 32?

Good point. I didn't take that into account. I think we can stick to 64bit. Probably define Platform=x64 for NWheels.Cli project. I'll check that.

I checked -- as I thought, it depends on Platform setting of NWheels.Cli.

Visual Studio is a 32 bit process, and it will run programs in 32 bit if their platform is "x86" or "Any CPU". If the platform is "x64", it will run the program in 64 bit with both F5 and Ctrl+F5.

I have set the platform of NWheels.Cli to x64.

Let's track it in #39 -- we have to discuss it.

ILyaIL83 · 2017-04-05T14:16:44Z

Source/NWheels.Implementation/Microservices/MicroserviceHost.cs

+
+            foreach (var directoryProbe in _assemblyMap.Directories)
+            {
+                var filePathProbes = new[] {


Why do we need *.exe and *.so?

Good point. We probably don't.

In order to get .exe, the project needs to be published as a self-contained application. Our code runs on folder where the project compiles to, Compile and publish are two different operations in .NET Core. Compile never produces .exe. So, looking for .exe is redundant.

Regarding .so, these are assemblies specific to Linux. They belong to runtime-specific assemblies, which I cannot reliably map anyways. I've put this comment in code:

//TODO: skip assemblies that come from runtimes/<RID>/... (RID = Runtime IDentifier, e.g. win7-x64, debian-x64) // since such assemblies are listed multiple times (per each RID), // and we don't filter by any specific RID, it is unreliable anyway // + those assemblies don't seem to ever arrive to AssemblyLoadContext.Resolving

Thus, looking for .so is redundant.

I will remove relevant bits of code.

ILyaIL83 · 2017-04-05T14:19:18Z

Source/NWheels.Platform.Messaging.Implementation/KestrelLifecycleListenerComponent.cs

@@ -36,7 +36,7 @@ public override void MicroserviceActivating()
                })
                .Build();

-            _host.Run();


With Run command thread was blocked as I understand. Thanks for fix.

You're welcome :)

ILyaIL83 · 2017-04-05T14:23:27Z

Source/NWheels.sln

@@ -1,7 +1,7 @@

 Microsoft Visual Studio Solution File, Format Version 12.00
 # Visual Studio 15
-VisualStudioVersion = 15.0.26228.4
+VisualStudioVersion = 15.0.26228.9


I have 15.0.26228.10 version of VS :) I think you also, so does it mean they haven't changed format?

It means I didn't install last VS2017 update on my home desktop :) Will do that today.

ILyaIL83 · 2017-04-05T14:28:57Z

Source/NWheels.Cli/Program.cs

@@ -0,0 +1,81 @@
+using System;


When I run Cli without any parameters it crashed with:
"error: missing command
Press any key to continue . . ."
It seems Cli itself should gives/takes -h|--help command when there is no any.

Yeah. You're right. Will fix that.

We'll track it in #38. The fix will be part of PR #33.

CLI: initial skeleton

62377ac

felix-b self-assigned this Mar 29, 2017

felix-b requested a review from ILyaIL83 March 29, 2017 13:39

felix-b added 3 commits March 30, 2017 07:13

CLI: separated 'service' command into 'publish' and 'run' ; introduce…

4ea9ab7

…d ICommandContext to allow unit testing ; added unit test project NWheels.Cli.UnitTests.

CLI: added missing files (apparently, .gitignore interfered with ever…

9f4959c

…ything named 'Publish').

CLI: got rid of NWheels.Cli.UnitTests in favor of system testing ; im…

18196a7

…plemented 'publish' and 'run' commands

felix-b mentioned this pull request Mar 31, 2017

Command-line interface (CLI) #35

Closed

felix-b and others added 9 commits April 2, 2017 04:35

Kernel/Extensions: added StringExtensions.DefaultIfNullOrEmpty

ee15288

Kernel/Microservices + CLI: progress with Publish and Run commands

2d240df

- Run determines module assembly search directories - BootConfiguration extended with list of assembly search directories - MicroserviceHost looks up assemblies in search directories if specified in BootConfiguration

CLI: temporarily(?) removing "all projects compile to solution-level …

6c360c3

…Bin" convention.

Kernel/Microservices: added AssemblyLocationMap to BootConfiguration …

2dddb25

…(replaced AssemblyDirectories added earlier).

CLI: mostly complete implementation - in debugging phase.

1d3b7d0

Platform/Messaging: bugfix in KestrelLifecycleListenerComponent - a c…

32a5eac

…all to IWebHost.Run() was blocking, replaced with IWebHost.Start().

CLI: bugfix in CommandBase - output interception in ExecuteProgram ca…

924ee42

…used deadlock, because output buffer was not read while waiting for process to finish.

CLI/run: performance improvement - invoke MSBuild only once, rather t…

ac6f6cf

…han per every module.

CLI: fixed/added support for relative paths in the command line; now …

b6d137a

…launchSettings.json (project debug settings) doesn't have to change per developer.

felix-b changed the title ~~WIP - CLI~~ CLI Apr 5, 2017

ILyaIL83 reviewed Apr 5, 2017

View reviewed changes

ILyaIL83 approved these changes Apr 5, 2017

View reviewed changes

felix-b merged commit d1fdf04 into master Apr 6, 2017

felix-b deleted the pr-cli branch April 6, 2017 05:27

felix-b added the gen1 label Mar 29, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CLI #37

CLI #37

felix-b commented Mar 29, 2017 •

edited

Loading

felix-b commented Mar 29, 2017 •

edited

Loading

felix-b commented Apr 5, 2017 •

edited

Loading

ILyaIL83 left a comment

ILyaIL83 Apr 5, 2017

felix-b Apr 5, 2017

ILyaIL83 Apr 5, 2017

felix-b Apr 5, 2017

ILyaIL83 Apr 5, 2017

felix-b Apr 5, 2017

felix-b Apr 6, 2017

ILyaIL83 Apr 5, 2017

felix-b Apr 5, 2017

felix-b Apr 5, 2017

felix-b Apr 6, 2017

ILyaIL83 Apr 5, 2017

felix-b Apr 5, 2017 •

edited

Loading

ILyaIL83 Apr 5, 2017

felix-b Apr 5, 2017

ILyaIL83 Apr 5, 2017

felix-b Apr 5, 2017

ILyaIL83 Apr 5, 2017

felix-b Apr 5, 2017

felix-b Apr 6, 2017

CLI #37

CLI #37

Conversation

felix-b commented Mar 29, 2017 • edited Loading

felix-b commented Mar 29, 2017 • edited Loading

felix-b commented Apr 5, 2017 • edited Loading

ILyaIL83 left a 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

felix-b Apr 5, 2017 • edited Loading

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

felix-b commented Mar 29, 2017 •

edited

Loading

felix-b commented Mar 29, 2017 •

edited

Loading

felix-b commented Apr 5, 2017 •

edited

Loading

felix-b Apr 5, 2017 •

edited

Loading