Skip to content

Metadata of native assemblies is now generated and exposed in virtual device #3023

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

Merged
merged 27 commits into from
Sep 26, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
50263be
- A file native_assemblies.csv is generated when a firmware package i…
Sep 23, 2024
6fc83d8
Make coderabbitai even more happy
Sep 23, 2024
ec0756e
Code style fixes
nfbot Sep 23, 2024
65eb7b2
Merge pull request #1 from nanoframework/nfbot/clang-format-fix/5ec8a…
frobijn Sep 23, 2024
894c168
Weird error
Sep 23, 2024
69513f9
Merge
Sep 23, 2024
4807232
Make the rabbit even more happy
Sep 23, 2024
f1dde0e
How can a merge lead to an empty code file????
Sep 23, 2024
2849918
Code style issues
Sep 24, 2024
2284649
Changes based on review comments
Sep 24, 2024
4187d18
- A file native_assemblies.csv is generated when a firmware package i…
Sep 23, 2024
7b3ec9b
Make coderabbitai even more happy
Sep 23, 2024
96caad7
Weird error
Sep 23, 2024
998b20e
Make the rabbit even more happy
Sep 23, 2024
52385bd
Code style issues
Sep 24, 2024
f7ea8ce
Changes based on review comments
Sep 24, 2024
ee61992
Refactor options to remove duplicate ones
josesimoes Sep 25, 2024
39db7de
Add new exit code to warn about mutually exclusive options
josesimoes Sep 25, 2024
132e84d
Tidy up output of native assemblies list
josesimoes Sep 25, 2024
0c5844e
Add back LocalInstance option
josesimoes Sep 25, 2024
1607ffb
Changes from CdeRabbitai review
josesimoes Sep 25, 2024
63e9e5b
Replace magic number with constant and add information about the origin
josesimoes Sep 25, 2024
b1415ee
Update targets/netcore/nanoFramework.nanoCLR.Host/NanoClrHostBuilder.cs
frobijn Sep 26, 2024
b88878f
Merge
Sep 26, 2024
467b9c2
Native assemblies sorted by name, LocalInstance => PathToCLRInstance …
Sep 26, 2024
4cdfb12
Code style fixes
nfbot Sep 26, 2024
a09eec1
Merge pull request #2 from nanoframework/nfbot/clang-format-fix/d7be1…
frobijn Sep 26, 2024
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
213 changes: 213 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,213 @@
# EditorConfig for Visual Studio 2022: https://learn.microsoft.com/en-us/visualstudio/ide/create-portable-custom-editor-options?view=vs-2022

# This is a top-most .editorconfig file
root = true

#=====================================================
#
# nanoFramework specific settings
#
#
#=====================================================
[*]
# Generic EditorConfig settings
end_of_line = crlf
charset = utf-8-bom

# Visual Studio spell checker
spelling_languages = en-us
spelling_checkable_types = strings,identifiers,comments
spelling_error_severity = information
spelling_exclusion_path = spelling_exclusion.dic

#=====================================================
#
# Settings copied from the .NET runtime
#
# https://github.com/dotnet/runtime
#
#=====================================================
# Default settings:
# A newline ending every file
# Use 4 spaces as indentation
insert_final_newline = true
indent_style = space
indent_size = 4
trim_trailing_whitespace = true

# Generated code
[*{_AssemblyInfo.cs,.notsupported.cs,AsmOffsets.cs}]
generated_code = true

# C# files
[*.cs]
# New line preferences
csharp_new_line_before_open_brace = all
csharp_new_line_before_else = true
csharp_new_line_before_catch = true
csharp_new_line_before_finally = true
csharp_new_line_before_members_in_object_initializers = true
csharp_new_line_before_members_in_anonymous_types = true
csharp_new_line_between_query_expression_clauses = true

# Indentation preferences
csharp_indent_block_contents = true
csharp_indent_braces = false
csharp_indent_case_contents = true
csharp_indent_case_contents_when_block = false
csharp_indent_switch_labels = true
csharp_indent_labels = one_less_than_current

# Modifier preferences
csharp_preferred_modifier_order = public,private,protected,internal,file,static,extern,new,virtual,abstract,sealed,override,readonly,unsafe,required,volatile,async:suggestion

# avoid this. unless absolutely necessary
dotnet_style_qualification_for_field = false:suggestion
dotnet_style_qualification_for_property = false:suggestion
dotnet_style_qualification_for_method = false:suggestion
dotnet_style_qualification_for_event = false:suggestion

# Types: use keywords instead of BCL types, and permit var only when the type is clear
csharp_style_var_for_built_in_types = false:suggestion
csharp_style_var_when_type_is_apparent = false:none
csharp_style_var_elsewhere = false:suggestion
dotnet_style_predefined_type_for_locals_parameters_members = true:suggestion
dotnet_style_predefined_type_for_member_access = true:suggestion

# name all constant fields using PascalCase
dotnet_naming_rule.constant_fields_should_be_pascal_case.severity = suggestion
dotnet_naming_rule.constant_fields_should_be_pascal_case.symbols = constant_fields
dotnet_naming_rule.constant_fields_should_be_pascal_case.style = pascal_case_style
dotnet_naming_symbols.constant_fields.applicable_kinds = field
dotnet_naming_symbols.constant_fields.required_modifiers = const
dotnet_naming_style.pascal_case_style.capitalization = pascal_case

# static fields should have s_ prefix
dotnet_naming_rule.static_fields_should_have_prefix.severity = suggestion
dotnet_naming_rule.static_fields_should_have_prefix.symbols = static_fields
dotnet_naming_rule.static_fields_should_have_prefix.style = static_prefix_style
dotnet_naming_symbols.static_fields.applicable_kinds = field
dotnet_naming_symbols.static_fields.required_modifiers = static
dotnet_naming_symbols.static_fields.applicable_accessibilities = private, internal, private_protected
dotnet_naming_style.static_prefix_style.required_prefix = s_
dotnet_naming_style.static_prefix_style.capitalization = camel_case

# internal and private fields should be _camelCase
dotnet_naming_rule.camel_case_for_private_internal_fields.severity = suggestion
dotnet_naming_rule.camel_case_for_private_internal_fields.symbols = private_internal_fields
dotnet_naming_rule.camel_case_for_private_internal_fields.style = camel_case_underscore_style
dotnet_naming_symbols.private_internal_fields.applicable_kinds = field
dotnet_naming_symbols.private_internal_fields.applicable_accessibilities = private, internal
dotnet_naming_style.camel_case_underscore_style.required_prefix = _
dotnet_naming_style.camel_case_underscore_style.capitalization = camel_case

# Code style defaults
csharp_using_directive_placement = outside_namespace:suggestion
dotnet_sort_system_directives_first = true
csharp_prefer_braces = true:silent
csharp_preserve_single_line_blocks = true:none
csharp_preserve_single_line_statements = false:none
csharp_prefer_static_local_function = true:suggestion
csharp_prefer_simple_using_statement = false:none
csharp_style_prefer_switch_expression = true:suggestion
dotnet_style_readonly_field = true:suggestion

# Expression-level preferences
dotnet_style_object_initializer = true:suggestion
dotnet_style_collection_initializer = true:suggestion
dotnet_style_prefer_collection_expression = when_types_exactly_match
dotnet_style_explicit_tuple_names = true:suggestion
dotnet_style_coalesce_expression = true:suggestion
dotnet_style_null_propagation = true:suggestion
dotnet_style_prefer_is_null_check_over_reference_equality_method = true:suggestion
dotnet_style_prefer_inferred_tuple_names = true:suggestion
dotnet_style_prefer_inferred_anonymous_type_member_names = true:suggestion
dotnet_style_prefer_auto_properties = true:suggestion
dotnet_style_prefer_conditional_expression_over_assignment = true:silent
dotnet_style_prefer_conditional_expression_over_return = true:silent
csharp_prefer_simple_default_expression = true:suggestion

# Expression-bodied members
csharp_style_expression_bodied_methods = true:silent
csharp_style_expression_bodied_constructors = true:silent
csharp_style_expression_bodied_operators = true:silent
csharp_style_expression_bodied_properties = true:silent
csharp_style_expression_bodied_indexers = true:silent
csharp_style_expression_bodied_accessors = true:silent
csharp_style_expression_bodied_lambdas = true:silent
csharp_style_expression_bodied_local_functions = true:silent

# Pattern matching
csharp_style_pattern_matching_over_is_with_cast_check = true:suggestion
csharp_style_pattern_matching_over_as_with_null_check = true:suggestion
csharp_style_inlined_variable_declaration = true:suggestion

# Null checking preferences
csharp_style_throw_expression = true:suggestion
csharp_style_conditional_delegate_call = true:suggestion

# Other features
csharp_style_prefer_index_operator = false:none
csharp_style_prefer_range_operator = false:none
csharp_style_pattern_local_over_anonymous_function = false:none

# Space preferences
csharp_space_after_cast = false
csharp_space_after_colon_in_inheritance_clause = true
csharp_space_after_comma = true
csharp_space_after_dot = false
csharp_space_after_keywords_in_control_flow_statements = true
csharp_space_after_semicolon_in_for_statement = true
csharp_space_around_binary_operators = before_and_after
csharp_space_around_declaration_statements = do_not_ignore
csharp_space_before_colon_in_inheritance_clause = true
csharp_space_before_comma = false
csharp_space_before_dot = false
csharp_space_before_open_square_brackets = false
csharp_space_before_semicolon_in_for_statement = false
csharp_space_between_empty_square_brackets = false
csharp_space_between_method_call_empty_parameter_list_parentheses = false
csharp_space_between_method_call_name_and_opening_parenthesis = false
csharp_space_between_method_call_parameter_list_parentheses = false
csharp_space_between_method_declaration_empty_parameter_list_parentheses = false
csharp_space_between_method_declaration_name_and_open_parenthesis = false
csharp_space_between_method_declaration_parameter_list_parentheses = false
csharp_space_between_parentheses = false
csharp_space_between_square_brackets = false

# License header
file_header_template = Licensed to the .NET Foundation under one or more agreements.\nThe .NET Foundation licenses this file to you under the MIT license.

# C++ Files
[*.{cpp,h,in}]
curly_bracket_next_line = true
indent_brace_style = Allman

# Xml project files
[*.{csproj,vbproj,vcxproj,vcxproj.filters,proj,nativeproj,locproj}]
indent_size = 2

[*.{csproj,vbproj,proj,nativeproj,locproj}]
charset = utf-8

# Xml build files
[*.builds]
indent_size = 2

# Xml files
[*.{xml,stylecop,resx,ruleset}]
indent_size = 2

# Xml config files
[*.{props,targets,config,nuspec}]
indent_size = 2

# YAML config files
[*.{yml,yaml}]
indent_size = 2

# Shell scripts
[*.sh]
end_of_line = lf
[*.{cmd,bat}]
end_of_line = crlf
70 changes: 68 additions & 2 deletions CMake/Modules/FindNF_NativeAssemblies.cmake
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#
#
# Copyright (c) .NET Foundation and Contributors
# See LICENSE file in the project root for full license information.
#
Expand Down Expand Up @@ -62,6 +62,53 @@ option(API_Hardware.GiantGecko "option for Hardware.Gia

###################################

#################################################################
# macro to find the version of an API or Interop assembly
macro(AddNativeAssemblyVersion apiNamespace apiNamespaceWithoutDots nativeAssemblySources)

# find the source file that contains the value of CLR_RT_NativeAssemblyData g_CLR_AssemblyNative_${apiNamespaceWithoutDots}
foreach(apiSourceFile ${nativeAssemblySources})

file(READ "${apiSourceFile}" sourceCode)

string(REGEX MATCH "[ \t\r\n]+CLR_RT_NativeAssemblyData[ \t\r\n]+g_CLR_AssemblyNative_${apiNamespaceWithoutDots}[ \t\r\n]*=[^{]+{[^0]+0x([0-9A-Za-z]+)[^{]+{[ \t\r\n]*([0-9]+)[ \t\r\n]*,[ \t\r\n]*([0-9]+)[ \t\r\n]*,[ \t\r\n]*([0-9]+)[ \t\r\n]*,[ \t\r\n]*([0-9]+)" _ "${sourceCode}")

if(NOT "${CMAKE_MATCH_1}" STREQUAL "")
list(APPEND NF_NativeAssemblies_VERSIONS "${apiNamespace},${CMAKE_MATCH_2}.${CMAKE_MATCH_3}.${CMAKE_MATCH_4}.${CMAKE_MATCH_5},${CMAKE_MATCH_1}")
break()
endif()

endforeach()

endmacro()
#################################################################

#################################################################
# macro to find the version of the CorLib assembly
macro(AddCorLibAssemblyVersion apiNamespace apiNamespaceWithoutDots nativeAssemblySources)

# find the source file that contains the value of CLR_RT_NativeAssemblyData g_CLR_AssemblyNative_${apiNamespaceWithoutDots}
foreach(apiSourceFile ${nativeAssemblySources})

file(READ "${apiSourceFile}" sourceCode)

string(REGEX MATCH "[ \t\r\n]+CLR_RT_NativeAssemblyData[ \t\r\n]+g_CLR_AssemblyNative_${apiNamespaceWithoutDots}[ \t\r\n]*=[^{]+{[^0]+0x([0-9A-Za-z]+)[^0]+0x([0-9A-Za-z]+)[^{]+{[ \t\r\n]*([0-9]+)[ \t\r\n]*,[ \t\r\n]*([0-9]+)[ \t\r\n]*,[ \t\r\n]*([0-9]+)[ \t\r\n]*,[ \t\r\n]*([0-9]+)" _ "${sourceCode}")

if(NOT "${CMAKE_MATCH_1}" STREQUAL "")
if (NF_FEATURE_SUPPORT_REFLECTION)
list(APPEND NF_NativeAssemblies_VERSIONS "${apiNamespace},${CMAKE_MATCH_3}.${CMAKE_MATCH_4}.${CMAKE_MATCH_5}.${CMAKE_MATCH_6},${CMAKE_MATCH_1}")
else()
list(APPEND NF_NativeAssemblies_VERSIONS "${apiNamespace},${CMAKE_MATCH_3}.${CMAKE_MATCH_4}.${CMAKE_MATCH_5}.${CMAKE_MATCH_6},${CMAKE_MATCH_2}")
endif()

break()
endif()

endforeach()

endmacro()
#################################################################

#################################################################
# macro to perform individual settings to add an API to the build
macro(PerformSettingsForApiEntry apiNamespace)
Expand All @@ -85,10 +132,12 @@ macro(PerformSettingsForApiEntry apiNamespace)
list(APPEND NF_NativeAssemblies_INCLUDE_DIRS "${${apiNamespace}_INCLUDE_DIRS}")
list(REMOVE_DUPLICATES NF_NativeAssemblies_INCLUDE_DIRS)

# append source files to list wiht source files for all the APIs
# append source files to list with source files for all the APIs
list(APPEND NF_NativeAssemblies_SOURCES "${${apiNamespace}_SOURCES}")
list(REMOVE_DUPLICATES NF_NativeAssemblies_SOURCES)

# add the assembly version to the list
AddNativeAssemblyVersion("${apiNamespace}" "${apiNamespaceWithoutDots}" "${${apiNamespace}_SOURCES}")
endmacro()
#################################################################

Expand Down Expand Up @@ -123,6 +172,8 @@ macro(PerformSettingsForInteropEntry interopAssemblyName)
list(APPEND NF_NativeAssemblies_SOURCES "${${interopAssemblyName}_SOURCES}")
list(REMOVE_DUPLICATES NF_NativeAssemblies_SOURCES)

# add the assembly version to the list
AddNativeAssemblyVersion("${interopAssemblyName}" "${interopAssemblyNameWithoutDots}" "${${interopAssemblyName}_SOURCES}")
endmacro()

#################################################################
Expand Down Expand Up @@ -172,6 +223,14 @@ macro(ParseInteropAssemblies)

endmacro()

############################################################################################
# Add versions of native assemblies that are always included
############################################################################################

AddCorLibAssemblyVersion("mscorlib" "mscorlib" "${CMAKE_SOURCE_DIR}/src/CLR/CorLib/corlib_native.cpp")

AddNativeAssemblyVersion("nanoFramework.Runtime.Native" "nanoFramework_Runtime_Native" "${CMAKE_SOURCE_DIR}/src/nanoFramework.Runtime.Native/nf_rt_native.cpp")

############################################################################################
# WHEN ADDING A NEW API add the corresponding block below
# required changes:
Expand Down Expand Up @@ -424,6 +483,13 @@ configure_file("${CMAKE_SOURCE_DIR}/InteropAssemblies/CLR_RT_InteropAssembliesTa
# ... now add Interop Assemblies table to ChibiOS nanoCLR sources list
list(APPEND NF_NativeAssemblies_SOURCES "${CMAKE_CURRENT_BINARY_DIR}/CLR_RT_InteropAssembliesTable.cpp")

# create a .csv file with native assembly versions in the output directory
string(REPLACE ";" "\r\n" NF_NativeAssemblies_CSV "${NF_NativeAssemblies_VERSIONS}")
file(WRITE "${CMAKE_BINARY_DIR}/native_assemblies.csv" "${NF_NativeAssemblies_CSV}" RESULT_VARIABLE WRITE_RESULT)
if(NOT WRITE_RESULT EQUAL 0)
message(WARNING "Failed to write native_assemblies.csv file")
endif()

# output the list of APIs included
list(LENGTH apiListing apiListingLenght)

Expand Down
1 change: 1 addition & 0 deletions spelling_exclusion.dic
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
nano
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
//
// Copyright (c) .NET Foundation and Contributors
// See LICENSE file in the project root for full license information.
//
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using CommandLine;

Expand All @@ -18,6 +16,12 @@ public class ClrInstanceOperationsOptions
HelpText = "Gets the version of the current nanoCLR instance.")]
public bool GetCLRVersion { get; set; }

[Option(
"getnativeassemblies",
Required = false,
HelpText = "Gets the names and versions of the native assemblies.")]
public bool GetNativeAssemblies { get; set; }

[Option(
"update",
Required = false,
Expand All @@ -38,6 +42,14 @@ public class ClrInstanceOperationsOptions
HelpText = "Specify a version of nanoCRL to install.")]
public string TargetVersion { get; set; }

[Option(
Copy link
Member

Choose a reason for hiding this comment

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

This is a duplication of ExecuteCommandLineOptions.LocalInstance`, isn't it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes and no. It serves the same purpose. But the way the options are organised is per "verb", so per "instance" and "run", so a new option has to be introduced.

My choice was to name it differently than the LocalInstance option for "run", as the LocalInstance is quite tricky. The LocalInstance option is described as the specification of a file that should be used as runtime. But if you try that as a simple user and pass "\nanoclr-1.2.3.dll", that won't work. Nanoclr uses the directory of the path and always uses the file name "nanoFramework.nanoCLR.dll". There are good reasons for that if you look at the code, but as a user (at least for me) it was a surprise. Because of backward compatibility the existing LocalInstance should not be changed, but I decided to give the new option a better name, let the user pass the directory and mention in the description that the file name is fixed.

Copy link
Member

Choose a reason for hiding this comment

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

Understood on the potential confusion about the path + name. That can be easily fixed, of course.
Now about the verbs, those are organized in that manner on purpose.

The CLI at the end of the day acts as a loader for a CLR instance. That's why you find the localinstance option under the run verb. Because one is requesting the tool to RUN that CLR instance (in lieu of the default one).

You're proposing to move this to the INSTANCE verb. That is not logical because, at that point of the execution its already running THAT instance that was previously requested. All this works perfectly garbled and mixed anyway considering that the CLI options are parsed and only after that the thing will actually do something.
Still, I would prefer to keep the consistency and have the options where they belong.

All for improving the option help text, documentation and code that processes it to simplify.

No strong opinion on allowing a DLL name different than nanoFramework.nanoCLR.dll... if that seems important... this was more a like an extra validation and using the default name seemed like a good approach to make the user life easier.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You're proposing to move this to the INSTANCE verb

No, not to move. Both verbs need this option:

  • If you have a runtime (dll) and want to know the native assemblies, there is no other way than to use "nanoclr.exe instance" that already provides information about the runtime and has the option to update the runtime. Either that or you create a separate tool for that.

  • If you want to use the runtime to run assemblies, you'd use the "nanoclr.exe run". This is also used by the test framework.

The way the CLI program is organized, there is no way to have common options. So I think the choices are: use the same name for both verbs (to stress that it is both concern the runtime DLL) or allow different names and use that to make one option a bit more clear.

No strong opinion on allowing a DLL name different than nanoFramework.nanoCLR.dll..

Maybe I don't understand how the various components work together, but I thought that the runtime DLL is the same one that is referred to by the various DLLImport declarations in the managed nanoFramework.nanoCLR.Host assembly. AFAIK if the DLL is a native DLL, the name used in the DLLImport must match the name of the DLL.

BTW I don't want to use a different file name, but the option suggests that it is possible. Put me on the wrong track.

Copy link
Member

Choose a reason for hiding this comment

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

Absolutely correct on that last part (I overlooked that) it must have that name because of the DLL import declarations as you've pointed out.

"clrinstancepath",
Required = false,
Default = null,
Hidden = true,
HelpText = "Path to the directory where a local version of nanoFramework.nanoCLR.dll is located.")]
public string LocalInstance { get; set; }

/// <summary>
/// Allowed values:
/// q[uiet]
Expand Down
Loading