Build automation window for faster test iteration with device #87

jevertt · 2016-07-01T19:54:01Z

This is a first time checkin of a Build & Deploy window. It supports
one-button clicks for building the SLN file, building the APPX,
uninstalling & installing on device, grabbing the log file (among other
things). Also has a single click build SLN, APPX, and install option.

This is a first time checkin of a Build & Deploy window. It supports one-button clicks for building the SLN file, building the APPX, uninstalling & installing on device, grabbing the log file (among other things). Also has a single click build SLN, APPX, and install option.

msftclas · 2016-07-01T19:54:05Z

Hi @jevertt, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla.microsoft.com.

TTYL, MSBOT;

jevertt · 2016-07-01T19:56:55Z

Here's a screenshot

david-c-kline · 2016-07-01T19:59:45Z

Assets/HoloToolkit/Build/Editor/BuildDeployPortal.cs

+        public const int kTimeoutMS = (int)(kTimeOut * 1000.0f);
+        public const float kMaxWaitTime = 20.0f;
+
+        public static readonly string kAPI_ProcessQuery = @"http://{0}/api/resourcemanager/processes";


http only works when connected over USB (localhost:10080) or if the user explicitly disables HTTPS in the Windows Device Portal via the browser.

Requiring the user to disable HTTPS is generally not a good idea. Using HTTPS will require your code to get / validate the server certificate from the device.

As long as we specify in the ReadMe or Info to remind people to keep their device connected.

Yes, it currently requires people to first...

pair the device.

disabling HTTPS requirement in the device portal (or always connect local).

Could you please update the ReadMe.md with information about this component?

NeerajW · 2016-07-01T20:34:44Z

@jevertt this is absolutely EPIC! Will make so many developers happy :)

jwittner · 2016-07-04T19:27:31Z

Right now the HoloToolkit menu has a Build subsection with easy output of the Sln and an option to open the Sln. I'm thinking we should add a button here for outputting the Sln in the same manner that the build menu currently does, and then drop that subsection from the menu.

Also, to note: Resolves - #49

jwittner · 2016-07-04T21:18:06Z

Assets/HoloToolkit/Build/Editor/BuildDeployPortal.cs

+    public class BuildDeployPortal
+    {
+        // Consts
+        public const float kTimeOut = 6.0f;


Curious about the 'k' prefix on these. @NeerajW Do we have a coding standard? Or just shooting for consistency within a file?

We recommend and follow general C# coding conventions. Also match the style in files for sure.

Personally, I can do without the 'k' prefix :)

riverar · 2016-07-05T16:14:43Z

Cool stuff, definitely better than my PowerShell script, but not sure this method should be legitimized further given:

it has a dependency on the optional Windows Device Portal
future WinAppDeployCmd tooling (Anniversary SDK) works with HoloLens

riverar · 2016-07-05T16:19:57Z

Assets/HoloToolkit/Build/Editor/BuildDeployPortal.cs

+    {
+        // Consts
+        public const float kTimeOut = 6.0f;
+        public const int kTimeoutMS = (int)(kTimeOut * 1000.0f);


Nit: Use the TimeSpan class to clean up manual arithmetic

david-c-kline · 2016-07-05T16:34:06Z

@riverar,
What is your concern regarding 'legitimizing' use of the Windows Device Portal?

Windows Device Portal seems to be the most appropriate solution for this feature. Since the target is developers building applications for HoloLens, it is reasonable to expect that the device is in developer mode.

riverar · 2016-07-05T16:42:22Z

@davidkline-ms

Developer mode does not turn on the Windows Device Portal by default
Some developers may not have a need for Device Portal (Anniversary SDK insider builds or higher fixed WinAppDeployCmd, which is far simpler)
This iteration doesn't support HTTPS
Having two methods to deploy apps seems confusing (winappdeploycmd, portal). Which is the team officially supporting?

NeerajW · 2016-07-05T16:58:40Z

@riverar Windows Device Portal is the supported form of communicating with Windows devices on all different platforms like HoloLens, Phone, Xbox etc.

It's reasonable to expect developers to have this enabled for their HoloLens.
Not supporting HTTPS in the first iteration is not a blocker to enable this awesome feature.

Let me know if I can help answer any other questions.

david-c-kline · 2016-07-05T17:00:32Z

@riverar,
To add on to @NeerajW's comment, developers are asking for a 'one button' build and deploy. This is the awesomeness that this proposal delivers.

Thanks!

riverar · 2016-07-05T17:05:38Z

And how do you plan on migrating users back onto HTTPS? I think landing that in v1 is critical for security, agree on everything else!

david-c-kline · 2016-07-05T17:13:47Z

We could limit the connection, in this iteration, to USB and then add support for specifying an ip address (using https) in a future update.

jevertt · 2016-07-05T18:29:39Z

Personally, I'd rather give the user the ability to make the choice (rather than making the choice for them) - only works wired or disabling HTTPS and it works both wired and remote. This is an option that is exposed to people in the Device Portal. And I will work towards adding remote HTTPS support.

riverar · 2016-07-05T18:46:56Z

Users will turn off HTTPS and not turn it back on when its support is eventually rolled out, leaving their devices in an insecure state.

Also consider the HoloLens Academy scenario, in which devices are remote controlled and are deployment targets via public IP not localhost. (HA at BUILD in fact stressed wireless deployment!) In this scenario, malicious actors on the network can sniff out the portal Basic auth credentials... a non-starter, really.

I like @davidkline-ms's suggestion for v1. If I get overruled here, a commitment to break HTTP support in the next push, forcing users to reconfigure the Device Portal, would be strongly recommended.

jevertt · 2016-07-05T21:11:48Z

What about this. When HTTPS support is added, also add a checkbox "Don't allow non-secure connections" that throws an exception if non-secure connections are set on the target device (and default it to true)? Or maybe a dialog box that says - non-secure connections are currently allowed on device, would you like to disable this setting? (and do it for them)

riverar · 2016-07-05T21:50:29Z

Assets/HoloToolkit/Build/Editor/BuildDeployWindow.cs

+
+                // Username/Password (and save seeings, if changed)
+                string newUsername = EditorGUILayout.TextField(kGUIHorizSpacer + "Username", deviceUser);
+                string newPassword = EditorGUILayout.TextField(kGUIHorizSpacer + "Password", devicePassword);


Switch to EditorGUILayout.PasswordField so we get masked input in editor

david-c-kline · 2016-07-05T23:06:41Z

@jevertt,
I like the idea of checkbox that allows stopping if the device is not in secure mode.

We could definitely check the device and, if indicated change the settings for them. In my experience, setting the device to HTTP does not preclude it from talking via HTTPS, so they can safely say "no" and we could still use HTTPS.

Removed k in constants & made password field hidden

riverar · 2016-07-06T00:57:05Z

I don't understand the strong desire to ship support for an insecure configuration.

I cloned the proposed commits and wired up a ServerCertificateValidationCallback that ignores (only) CRL issues (as the HoloLens root cert doesn't have a CDP specified). I then dropped the certificate into %AppData%\Roaming.mono\certs\Trust and things mostly worked until I hit what appears to be old TLS bugs in Mono, perhaps related to self-signature, Subject Alternative Name extension, or something else entirely.

I'm worried HTTPS support is far away now, perhaps restricting this to loopback use only is in order.

jwittner · 2016-07-06T02:11:05Z

Assets/HoloToolkit/Build/Editor/BuildDeployWindow.cs

+                    if (GUILayout.Button("Open SLN", GUILayout.Width(buttonWidth_Quarter)))
+                    {
+                        // Open SLN
+                        FileInfo slnFile = new FileInfo(Path.Combine(buildDirectory, PlayerSettings.productName + ".sln"));


Acts funky if no solution exists. Maybe disable in that case? Current Build menu asks if you'd like to build one in this case.

Win32Exception: The system cannot find the file specified.

System.Diagnostics.Process.Start_shell (System.Diagnostics.ProcessStartInfo startInfo, System.Diagnostics.Process process)
System.Diagnostics.Process.Start_common (System.Diagnostics.ProcessStartInfo startInfo, System.Diagnostics.Process process)
System.Diagnostics.Process.Start (System.Diagnostics.ProcessStartInfo startInfo)
System.Diagnostics.Process.Start (System.String fileName)
HoloToolkit.Unity.BuildDeployWindow.OnGUI () (at Assets/HoloToolkit/Build/Editor/BuildDeployWindow.cs:128)
System.Reflection.MonoMethod.Invoke (System.Object obj, BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) (at /Users/builduser/buildslave/mono/build/mcs/class/corlib/System.Reflection/MonoMethod.cs:222)

good catch - I'll take a look a this

jwittner · 2016-07-06T15:03:41Z

Should we drop the current Build menu in this PR so there's no period of overlap?

…ith HTTPS default setting) & removed redundant build sub-menu

jwittner · 2016-07-06T19:28:50Z

Assets/HoloToolkit/Build/Editor/BuildDeployWindow.cs

+                        }
+                        else
+                        {
+                            Debug.LogError("Solution file does not exist (" + slnFilename + ")");


Maybe something like the below from the old BuildMenu? Might be more clear than an error in the logs.

bool buildNow = EditorUtility.DisplayDialog("Solution Not Found", "We couldn't find the solution. Would you like to Build?", "Yes, Build", "No");

Alternatively you could check for the filepath before adding the button and disable the button if it doesn't exist.

I like it - in now.

… isn't found (per feedback)

NeerajW · 2016-07-06T19:45:58Z

Thanks for the updates @jevertt. I think we are ready to merge.
Current drop will only support local IPOverUSB.

riverar · 2016-07-06T20:37:43Z

Thanks for tolerating my moaning, I think we ultimately went down the right road here. Cool stuff @jevertt!

david-c-kline · 2016-07-06T20:45:55Z

@riverar,
I detected no moaning, just good thorough community review and feedback :)

msftclas added the cla-required label Jul 1, 2016

david-c-kline reviewed Jul 1, 2016
View reviewed changes

added -build component to the readme

cf6924f

jwittner mentioned this pull request Jul 4, 2016

BuildCommands.BuildForHololens is undiscoverable from the command line. #85

Closed

jwittner reviewed Jul 4, 2016
View reviewed changes

jwittner mentioned this pull request Jul 4, 2016

Build sub menu is not documented #74

Closed

riverar reviewed Jul 5, 2016
View reviewed changes

Feedback from community

26dbfb5

Removed k in constants & made password field hidden

jwittner mentioned this pull request Jul 6, 2016

There's no way to configure where builds from the Build menu are output to. #84

Closed

jwittner reviewed Jul 6, 2016
View reviewed changes

Changed to localIP support only (as this is the only one that works w…

84b6cc0

…ith HTTPS default setting) & removed redundant build sub-menu

jwittner reviewed Jul 6, 2016
View reviewed changes

Added an option to build SLN, if openDialog is clicked & the SLN file…

3e87036

… isn't found (per feedback)

NeerajW merged commit bef9230 into microsoft:master Jul 6, 2016

jwittner mentioned this pull request Jul 7, 2016

Deploying to device is cumbersome and not available directly from Unity #49

Closed

riverar mentioned this pull request Jul 8, 2016

Ability to configure HoloLens IP address in the Build Window #105

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Build automation window for faster test iteration with device #87

Build automation window for faster test iteration with device #87

jevertt commented Jul 1, 2016

msftclas commented Jul 1, 2016

jevertt commented Jul 1, 2016

david-c-kline Jul 1, 2016

NeerajW Jul 1, 2016

jevertt Jul 1, 2016

NeerajW Jul 1, 2016

NeerajW commented Jul 1, 2016

jwittner commented Jul 4, 2016 •

edited

Loading

jwittner Jul 4, 2016

NeerajW Jul 5, 2016

riverar commented Jul 5, 2016

riverar Jul 5, 2016

david-c-kline commented Jul 5, 2016

riverar commented Jul 5, 2016

NeerajW commented Jul 5, 2016

david-c-kline commented Jul 5, 2016

riverar commented Jul 5, 2016

david-c-kline commented Jul 5, 2016

jevertt commented Jul 5, 2016 •

edited

Loading

riverar commented Jul 5, 2016

jevertt commented Jul 5, 2016

riverar Jul 5, 2016

david-c-kline commented Jul 5, 2016

riverar commented Jul 6, 2016

jwittner Jul 6, 2016

jwittner Jul 6, 2016

jevertt Jul 6, 2016

jwittner commented Jul 6, 2016

jwittner Jul 6, 2016 •

edited

Loading

jevertt Jul 6, 2016

NeerajW commented Jul 6, 2016

riverar commented Jul 6, 2016

david-c-kline commented Jul 6, 2016

Build automation window for faster test iteration with device #87

Build automation window for faster test iteration with device #87

Conversation

jevertt commented Jul 1, 2016

msftclas commented Jul 1, 2016

jevertt commented Jul 1, 2016

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

NeerajW commented Jul 1, 2016

jwittner commented Jul 4, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

riverar commented Jul 5, 2016

Choose a reason for hiding this comment

david-c-kline commented Jul 5, 2016

riverar commented Jul 5, 2016

NeerajW commented Jul 5, 2016

david-c-kline commented Jul 5, 2016

riverar commented Jul 5, 2016

david-c-kline commented Jul 5, 2016

jevertt commented Jul 5, 2016 • edited Loading

riverar commented Jul 5, 2016

jevertt commented Jul 5, 2016

Choose a reason for hiding this comment

david-c-kline commented Jul 5, 2016

riverar commented Jul 6, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jwittner commented Jul 6, 2016

jwittner Jul 6, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NeerajW commented Jul 6, 2016

riverar commented Jul 6, 2016

david-c-kline commented Jul 6, 2016

jwittner commented Jul 4, 2016 •

edited

Loading

jevertt commented Jul 5, 2016 •

edited

Loading

jwittner Jul 6, 2016 •

edited

Loading