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

DYN-7337 Make TuneUp an Built-In package, part II #15536

Merged
merged 19 commits into from
Oct 15, 2024
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,15 @@
###################
*.com
*.class
*.dll

*.exe
*.msi
*.o
*.so
bin/
AnyCPU/
Debug/
Release/
originalBinaries/
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ignore dlls under built directory more precisely

obj/
int/
packages/
Expand Down Expand Up @@ -96,6 +99,7 @@ src/AssemblySharedInfoGenerator/AssemblySharedInfo.cs
/src/Libraries/PythonNodeModelsWpf/*.resources
/src/Libraries/PythonNodeModels/*.resources
/test/core/dsevaluation/DSFiles/test.ds
/test/test_dependencies
GregStandaloneAuth.dll
/src/Libraries/Watch3DNodeModels/*.resources
*.resources
Expand Down
File renamed without changes.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<ViewExtensionDefinition>
<AssemblyPath>..\Built-In Packages\TuneUp.dll</AssemblyPath>
<AssemblyPath>..\bin\TuneUp.dll</AssemblyPath>
<TypeName>TuneUp.TuneUpViewExtension</TypeName>
</ViewExtensionDefinition>
24 changes: 24 additions & 0 deletions extern/TuneUp/pkg.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
{
"license": "MIT",
"file_hash": null,
"name": "TuneUp",
"version": "1.0.12",
reddyashish marked this conversation as resolved.
Show resolved Hide resolved
"description": "*** THE TUNEUP EXTENSION USES NEW API FEATURES THAT ARE ONLY AVAILABLE IN DYNAMO VERSION 2.5 OR ABOVE ***\r\n\r\nTuneUp is in beta and is a view extension for analyzing the performance of Dynamo graphs. TuneUp allows you to see overall graph execution time, per-node execution time, and other helpful information about what\u0027s happening under the hood, e.g. nodes run in the current execution v.s. nodes run in the previous execution (which are skipped this run for optimization/ caching). Read more here: https://dynamobim.org/tuneup-extension-explore-your-node-and-graph-execution-times/ \r\n\r\nKnown issues:\r\n1. TuneUp does not work with .dyfs (custom nodes) yet.\r\n2.TuneUp binaries are not semantically versioned and are not intended to be built on top of as an API. Do not treat these binaries like DynamoCore.\r\n3. TuneUp requires Dynamo 3.0 or higher for access to new extension APIs.\r\n4. When user have TuneUp open, after switching workspace in Dynamo, the first graph run does not give execution time and nodes order.",
"group": "",
"keywords": [
"profiler",
"tuneup"
],
"dependencies": [],
"host_dependencies": [],
"contents": "",
"engine_version": "3.0.0.7186",
"engine": "dynamo",
"engine_metadata": "",
"site_url": "https://dynamobim.org/",
"repository_url": "https://github.com/DynamoDS/TuneUp",
"contains_binaries": true,
"node_libraries": [],
"copyright_holder": "DynamoTeam",
"copyright_year": "2024"
}
1 change: 1 addition & 0 deletions src/DynamoCore/Models/DynamoModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -930,6 +930,7 @@ protected DynamoModel(IStartConfiguration config)

if (extensions.Any())
{
Logger.Log("\nLoading Dynamo extensions:");
var startupParams = new StartupParams(this);

foreach (var ext in extensions)
Expand Down
6 changes: 2 additions & 4 deletions src/DynamoCoreWpf/DynamoCoreWpf.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -1857,11 +1857,9 @@
</ItemGroup>
<Target Name="AfterBuildOps" AfterTargets="Build">
<ItemGroup>
<ExternTuneUpDll Include="$(SolutionDir)..\extern\TuneUp\TuneUp.dll" />
<ExternTuneUpManifest Include="$(SolutionDir)..\extern\TuneUp\TuneUp_ViewExtensionDefinition.xml" />
<ExternTuneUpPkg Include="$(SolutionDir)..\extern\TuneUp\**\*.*" />
</ItemGroup>
<MakeDir Directories="$(OutputPath)\viewExtensions\" />
<Copy SourceFiles="@(ExternTuneUpDll)" DestinationFolder="$(OutputPath)\Built-In Packages" />
<Copy SourceFiles="@(ExternTuneUpManifest)" DestinationFolder="$(OutputPath)\viewExtensions" />
<Copy SourceFiles="@(ExternTuneUpPkg)" DestinationFolder="$(OutputPath)\Built-In Packages\packages\TuneUp\%(RecursiveDir)" />
Copy link
Contributor Author

@QilongTang QilongTang Oct 11, 2024

Choose a reason for hiding this comment

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

Copy files recursively and keeping the folder structure

</Target>
</Project>
14 changes: 4 additions & 10 deletions src/DynamoPackages/PackageLoader.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.IO;
using System.Linq;
using System.Reflection;
Expand Down Expand Up @@ -247,7 +248,8 @@ private void TryLoadPackageIntoLibrary(Package package)

// load custom nodes
var packageInfo = new Graph.Workspaces.PackageInfo(package.Name, new Version(package.VersionName));
var customNodes = OnRequestLoadCustomNodeDirectory(package.CustomNodeDirectory, packageInfo);
// skip loding if the CustomNodeDirectory does not exist
var customNodes = Directory.Exists(package.CustomNodeDirectory)? OnRequestLoadCustomNodeDirectory(package.CustomNodeDirectory, packageInfo) : [];
package.LoadedCustomNodes.AddRange(customNodes);

package.EnumerateAdditionalFiles();
Expand Down Expand Up @@ -643,16 +645,8 @@ private Version CheckAndGetPackageVersion(string version, string name, string di
/// </summary>
/// <param name="packageDirectoryPath">path to package location</param>
/// <param name="discoveredPkg">package object to check</param>
private void CheckPackageNodeLibraryCertificates(string packageDirectoryPath, Package discoveredPkg)
private static void CheckPackageNodeLibraryCertificates(string packageDirectoryPath, Package discoveredPkg)
{
var dllfiles = new System.IO.DirectoryInfo(discoveredPkg.BinaryDirectory).EnumerateFiles("*.dll");
if (discoveredPkg.Header.node_libraries.Count() == 0 && dllfiles.Count() != 0)
{
Log(String.Format(
String.Format(Resources.InvalidPackageNoNodeLibrariesDefinedInPackageJson,
discoveredPkg.Name, discoveredPkg.RootDirectory)));
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remove the node library dll check because Built-In package moving forward may not always include nodes so this is no longer needed, even the logging


foreach (var nodeLibraryAssembly in discoveredPkg.Header.node_libraries)
{

Expand Down
13 changes: 2 additions & 11 deletions src/DynamoPackages/Properties/Resources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 2 additions & 6 deletions src/DynamoPackages/Properties/Resources.en-US.resx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<?xml version="1.0" encoding="utf-8"?>
<root>
<!--
Microsoft ResX Schema
Expand Down Expand Up @@ -136,12 +136,8 @@
<value>"A package called {0} found at {1} could not be verified to have signed dll files. {2} Ignoring it."</value>
<comment>This warning message is shown when the node libraries defined for the package in the pkg.json manifest are invalid or do not have a verified certificate file</comment>
</data>
<data name="InvalidPackageNoNodeLibrariesDefinedInPackageJson" xml:space="preserve">
<value>A package called {0} found at {1} includes dll files but none are defined in node libraries in the package manifest. Ignoring it.</value>
<comment>This warning message is shown when packages are loaded and require verification of their certificate. Having Dll's and no defined node libararies in the pkg json manifest stops the validation process</comment>
</data>
<data name="PackagesDirectorySkipped" xml:space="preserve">
<value>{0} was not scanned for packages because a preference setting disabled loading from that location."</value>
<comment>This warning message is shown when a package directory is skipped due to a preference setting disabling loads from that location type.</comment>
</data>
</root>
</root>
8 changes: 2 additions & 6 deletions src/DynamoPackages/Properties/Resources.resx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<?xml version="1.0" encoding="utf-8"?>
<root>
<!--
Microsoft ResX Schema
Expand Down Expand Up @@ -136,12 +136,8 @@
<value>"A package called {0} found at {1} could not be verified to have signed dll files. {2} Ignoring it."</value>
<comment>This warning message is shown when the node libraries defined for the package in the pkg.json manifest are invalid or do not have a verified certificate file</comment>
</data>
<data name="InvalidPackageNoNodeLibrariesDefinedInPackageJson" xml:space="preserve">
<value>A package called {0} found at {1} includes dll files but none are defined in node libraries in the package manifest. Ignoring it.</value>
<comment>This warning message is shown when packages are loaded and require verification of their certificate. Having Dll's and no defined node libararies in the pkg json manifest stops the validation process</comment>
</data>
<data name="PackagesDirectorySkipped" xml:space="preserve">
<value>{0} was not scanned for packages because a preference setting disabled loading from that location."</value>
<comment>This warning message is shown when a package directory is skipped due to a preference setting disabling loads from that location type.</comment>
</data>
</root>
</root>
10 changes: 4 additions & 6 deletions src/Tools/DynamoFeatureFlags/FeatureFlagsClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,7 @@ internal FeatureFlagsClient(string userkey, string mobileKey = null, bool testMo
if (testMode)
{
MessageLogged?.Invoke($"LD startup: testmode true, no LD connection. ");
MessageLogged?.Invoke($"LD startup time: {sw.ElapsedMilliseconds} ");
MessageLogged?.Invoke("<<<<<InitDone>>>>>");
MessageLogged?.Invoke($"LD startup time: {sw.ElapsedMilliseconds} ms ");
AllFlags = LdValue.ObjectFrom(new Dictionary<string,LdValue> { { "TestFlag1",LdValue.Of(true) },
{ "TestFlag2", LdValue.Of("I am a string") },
//in tests we want instancing on so we can test it.
Expand All @@ -95,8 +94,7 @@ internal FeatureFlagsClient(string userkey, string mobileKey = null, bool testMo

Init(mobileKey);
sw.Stop();
MessageLogged?.Invoke($"LD startup time: {sw.ElapsedMilliseconds} ");
MessageLogged?.Invoke("<<<<<InitDone>>>>>");
MessageLogged?.Invoke($"Launch Darkly startup time: {sw.ElapsedMilliseconds} ms");
//gather all the user's flags and create a top level ldvalue object containing all of them.
if (ldClient.Initialized)
{
Expand All @@ -110,11 +108,11 @@ internal void Init(string mobileKey)
ldClient = LaunchDarkly.Sdk.Client.LdClient.Init(mobileKey, LaunchDarkly.Sdk.Client.ConfigurationBuilder.AutoEnvAttributes.Disabled, user, TimeSpan.FromSeconds(5));
if (ldClient.Initialized)
{
MessageLogged?.Invoke($"launch darkly initalized");
MessageLogged?.Invoke($"Launch Darkly initalized");
}
else
{
MessageLogged?.Invoke($"launch darkly failed to initalize");
MessageLogged?.Invoke($"Launch Darkly failed to initalize");
}
}

Expand Down
2 changes: 1 addition & 1 deletion test/DynamoCoreTests/Logging/FeatureFlagTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public void FeatureFlagsShouldMessageLoggedShouldContainAllLogs()
testflagsManager.MessageLogged -= TestflagsManager_MessageLogged;
//json ordering cannot be controlled between .net target versions.
StringAssert.StartsWith("LD startup: testmode true, no LD connection. LD startup time:" , log);
StringAssert.Contains("<<<<<InitDone>>>>>feature flag exe starting<<<<<Sod>>>>>",log);
StringAssert.Contains("feature flag exe starting<<<<<Sod>>>>>",log);
StringAssert.Contains("\"TestFlag1\":true", log);
StringAssert.Contains("\"TestFlag2\":\"I am a string\"", log);
StringAssert.Contains("\"graphics-primitive-instancing\":true", log);
Expand Down
6 changes: 4 additions & 2 deletions test/DynamoCoreWpfTests/CrashReportingTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,8 @@ public void NoLoadedPackages()
//Gets package loader
var packageLoader = CurrentDynamoModel.GetPackageManagerExtension()?.PackageLoader;
Assert.IsNotNull(packageLoader);
Assert.IsEmpty(packageLoader.LocalPackages, string.Join(", ", packageLoader.LocalPackages.Select(x => x.Name)));
Assert.IsTrue(packageLoader.LocalPackages.Count() == 1);
Assert.IsNotEmpty(packageLoader.LocalPackages, string.Join(", ", packageLoader.LocalPackages.Select(x => x.Name)));

//Get packages data from null package loader
var packagesData = Wpf.Utilities.CrashUtilities.PackagesToMakrdown(packageLoader);
Expand All @@ -170,7 +171,8 @@ public void NoLoadedPackages()
var body = url.Substring(startIndex);
var decoded = Uri.UnescapeDataString(body);

var expectedString = "No loaded packages were found.";
// At least one built-in package is always loaded
var expectedString = "- TuneUp";

// Verify request contains the packages information
Assert.True(decoded.Contains(expectedString));
Expand Down
12 changes: 7 additions & 5 deletions test/DynamoCoreWpfTests/PackageManager/PackageManagerUITests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -950,7 +950,7 @@ public void PackageManagerUninstallCommand()

// This function is called upon addition of new package paths in the UI.
loader.LoadAll(loadPackageParams);
Assert.AreEqual(1, loader.LocalPackages.Count());
Assert.AreEqual(2, loader.LocalPackages.Count());

var dlgMock = new Mock<MessageBoxService.IMessageBox>();
dlgMock.Setup(m => m.Show(It.IsAny<Window>(), It.IsAny<string>(), It.IsAny<string>(), It.Is<MessageBoxButton>(x => x == MessageBoxButton.OKCancel || x == MessageBoxButton.OK), It.IsAny<MessageBoxImage>()))
Expand Down Expand Up @@ -1010,7 +1010,7 @@ public void PackageManagerLoadManuallyUnloadedBuiltIn()
var loader = currentDynamoModel.GetPackageManagerExtension().PackageLoader;

loader.LoadAll(loadPackageParams);
Assert.AreEqual(1, loader.LocalPackages.Count());
Assert.AreEqual(2, loader.LocalPackages.Count());
Assert.IsTrue(loader.LocalPackages.Count(x => x.Name == "SignedPackage") == 1);

var dlgMock = new Mock<MessageBoxService.IMessageBox>();
Expand All @@ -1025,9 +1025,11 @@ public void PackageManagerLoadManuallyUnloadedBuiltIn()

ViewModel.PreferencesViewModel.InitPackageListFilters();
var filters = ViewModel.PreferencesViewModel.Filters;
Assert.AreEqual(2, filters.Count);
Assert.AreEqual(3, filters.Count);
Assert.AreEqual(@"All", filters[0].Name);
Assert.AreEqual(@"Unloaded", filters[1].Name);
Assert.AreEqual(@"Loaded", filters[1].Name);
Assert.AreEqual(@"Unloaded", filters[2].Name);


builtInPkgViewModel.LoadCommand.Execute();

Expand Down Expand Up @@ -1061,7 +1063,7 @@ public void PackageManagerNoDuplicatesOnPathIsRemovedThenAdded()
var loader = currentDynamoModel.GetPackageManagerExtension().PackageLoader;

loader.LoadAll(loadPackageParams);
Assert.AreEqual(0, loader.LocalPackages.Count());
Assert.AreEqual(1, loader.LocalPackages.Count());
var vm = new PackagePathViewModel(loader, loadPackageParams, Model.CustomNodeManager);

vm.RequestShowFileDialog += (sender, args) => { args.Path = BuiltinPackagesTestDir; };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ public void PackageManagerLoadsRuntimeGeneratedExtension()
public void PackageManagerViewExtensionHasCorrectNumberOfRequestedExtensions()
{
var pkgViewExtension = View.viewExtensionManager.ViewExtensions.OfType<PackageManagerViewExtension>().FirstOrDefault();
Assert.AreEqual(pkgViewExtension.RequestedExtensions.Count(), 1);
Assert.AreEqual(pkgViewExtension.RequestedExtensions.Count(), 2);
}

[Test]
Expand Down Expand Up @@ -208,7 +208,7 @@ public void LateLoadedViewExtensionsHaveMethodsCalled()
var pkg = loader.ScanPackageDirectory(pkgDir);
loader.LoadPackages(new List<Package> { pkg });
Assert.AreEqual(0, loader.RequestedExtensions.Count());
Assert.AreEqual(2, View.viewExtensionManager.ViewExtensions.OfType<PackageManagerViewExtension>().FirstOrDefault().RequestedExtensions.Count());
Assert.AreEqual(3, View.viewExtensionManager.ViewExtensions.OfType<PackageManagerViewExtension>().FirstOrDefault().RequestedExtensions.Count());
Assert.IsTrue(viewExtensionLoadStart);
Assert.IsTrue(viewExtensionAdd);
Assert.IsTrue(viewExtensionLoaded);
Expand Down
Copy link
Contributor

Choose a reason for hiding this comment

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

Add this dll to the list of shipped binaries?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a test dll generated runtime, I have removed it and updated gitignore

Binary file not shown.
2 changes: 1 addition & 1 deletion tools/DynamoPackagesAnalyzer/DynamoPackagesAnalyzer.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
<ItemGroup>
<PackageReference Include="CommandLineParser" Version="2.8.0" />
<PackageReference Include="CsvHelper" Version="30.0.1" />
<PackageReference Include="Greg" Version="3.0.1.4707" />
<PackageReference Include="Greg" Version="3.0.2.6284" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we changing the greg version only for this project?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@reddyashish This project seems to be behind (not aligned with all the other project)

<PackageReference Include="Microsoft.Extensions.Configuration.Json" Version="6.0.0" />
<PackageReference Include="RestSharp.Serializers.NewtonsoftJson" Version="106.12.0" />
</ItemGroup>
Expand Down
Loading