Fix #4931 - Install C# OpenStudio.dll properly. #4933

jmarrec · 2023-07-19T05:58:23Z

Pull request overview

Fixes Why is OpenStudio.dll no longer included in the installers? #4931

Let's see if CI makes an installer with the OpenStudio.dll

Pull Request Author

Model API Changes / Additions
Any new or modified fields have been implemented in the EnergyPlus ForwardTranslator (and ReverseTranslator as appropriate)
Model API methods are tested (in src/model/test)
EnergyPlus ForwardTranslator Tests (in src/energyplus/Test)
If a new object or method, added a test in NREL/OpenStudio-resources: Add Link
If needed, added VersionTranslation rules for the objects (src/osversion/VersionTranslator.cpp)
Verified that C# bindings built fine on Windows, partial classes used as needed, etc.
All new and existing tests passes
If methods have been deprecated, update rest of code to use the new methods

Labels:

If change to an IDD file, add the label IDDChange
If breaking existing API, add the label APIChange
If deemed ready, add label Pull Request - Ready for CI so that CI builds your PR

Review Checklist

This will not be exhaustively relevant to every PR.

Perform a Code Review on GitHub
Code Style, strip trailing whitespace, etc.
All related changes have been implemented: model changes, model tests, FT changes, FT tests, VersionTranslation, OS App
Labeling is ok
If defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
If feature, test running new feature, try creative ways to break it
CI status: all green or justified

jmarrec · 2023-07-19T05:58:43Z

csharp/CMakeLists.txt

@@ -60,14 +61,11 @@ target_compile_options(csharp_warnings INTERFACE "$<${is_gnu_or_clang_genex}:-Wn
 # custom command to make OPENSTUDIO_CSHARP_DLL
 add_custom_command(
  OUTPUT ${OPENSTUDIO_CSHARP_DLL}
-  COMMAND "${DOTNET}" "build" "-c" "$<CONFIGURATION>" "/p:Platform=${CSHARP_PLATFORM}" "${PROJECT_BINARY_DIR}/csharp_wrapper/OpenStudio.csproj"
+  COMMAND "${DOTNET}" "build" "-c" "$<CONFIG>" "/p:Platform=${CSHARP_PLATFORM}" "${PROJECT_BINARY_DIR}/csharp_wrapper/OpenStudio.csproj"


CONFIGURATION genex is deprecated since cmake 3.0 and replaced with CONFIG

jmarrec · 2023-07-19T05:59:08Z

csharp/CMakeLists.txt

-install(FILES "${OPENSTUDIO_CSHARP_DLL}" DESTINATION CSharp/openstudio/ CONFIGURATIONS DEBUG COMPONENT "CSharpAPI")
-
-
+install(FILES "${OPENSTUDIO_CSHARP_DLL}" DESTINATION CSharp/openstudio/ COMPONENT "CSharpAPI")


Install it regardless of the configuration. It was done only for DEBUG prior

jmarrec · 2023-07-19T05:59:18Z

csharp/CMakeLists.txt

+# Note: can't use CMAKE_BUILD_TYPE as it's empty for multi generators (MSVC for eg). So we use a genex
+set(OPENSTUDIO_CSHARP_DLL "${CSHARP_LIBRARY_OUTPUT_DIRECTORY}/$<CONFIG>/netstandard2.0/OpenStudio.dll")


See the note.

I replaced the CMAKE_BUILD_TYPE with a genex too. This was working on CI because we do pass explicitly cmake -DCMAKE_BUILD_TYPE:STRING=Release when building on MSVC. But MSVC by default is Multi-configuration and CMAKE_BUILD_TYPE is empty if used like this.

CONFIGURATION genex is deprecated since cmake 3.0 and replaced with CONFIG

jmarrec · 2023-08-24T08:26:33Z

CMakeLists.txt

+    if (BUILD_CSHARP_BINDINGS)
+      install(FILES ${CMAKE_INSTALL_SYSTEM_RUNTIME_LIBS} DESTINATION "CSharp/openstudio/" COMPONENT "CSharpAPI")
+    endif()


Don't install the runtimes libs there if C# isn't selected to begin with.

This threw me off when I was looking at the CI-built packages, because actually the incremental runner does not build C#!

jmarrec · 2023-08-24T08:27:00Z

ProjectMacros.cmake

+    # The -py3 flag is now deprecated as Python 3 is the default the version used is Python 3
+    set(SWIG_PYTHON_3_FLAGS "-relativeimport")


Unrelated cleanup: Avoid having multiple warnings from swig saying "-py3 flag is deprecated"

jmarrec · 2023-08-24T08:27:45Z

src/utilities/core/CommonInclude.i

@@ -66,7 +66,7 @@
 // and nested classes and "nothing known about <type>"
 // also ignoring #509, shadowed method. Its a red herring for all of our cases
 //#pragma SWIG nowarn=401,362,365,366,368,378,503,801,312
-#pragma SWIG nowarn=362,365,366,368,378,503,801,312,509,401
+#pragma SWIG nowarn=362,365,366,368,378,503,801,312,509,401,516


Avoid thousands of "overloaded method ignore, using XXX instead". In particular for every modelObject, the Ctor is overloaded (rule of 5/6) so it was getting out of control.

jmarrec · 2023-08-24T08:28:21Z

python/SetupPython.cmake

@@ -9,7 +9,7 @@ set(Python_USE_STATIC_LIBS OFF)
 if (PYTHON_VERSION)
  find_package(Python ${PYTHON_VERSION} EXACT REQUIRED COMPONENTS Interpreter Development OPTIONAL_COMPONENTS NumPy)
 else()
-  find_package(Python REQUIRED COMPONENTS Interpreter Development OPTIONAL_COMPONENTS NumPy)
+  find_package(Python 3 EXACT REQUIRED COMPONENTS Interpreter Development OPTIONAL_COMPONENTS NumPy)


Force finding python3, not python2, related to SWIG using py3 now. Python 2 is EOL since 2020 anyways, we do not want to support it.

jmarrec · 2023-08-24T08:28:33Z

ProjectMacros.cmake

@@ -511,7 +499,7 @@ macro(MAKE_SWIG_TARGET NAME SIMPLENAME KEY_I_FILE I_FILES PARENT_TARGET PARENT_S
      # This is not working: "cannot open file 'python37.lib'"
      target_link_libraries(${swig_target} PUBLIC  ${${PARENT_TARGET}_depends})
      if (MSVC)
-        message("Python_LIBRARIES=${Python_LIBRARIES}")
+        message(DEBUG "Python_LIBRARIES=${Python_LIBRARIES}")


Shush this message too

jmarrec · 2023-08-24T08:29:00Z

src/utilities/core/jsoncpp.i

+  // ignores toJSON/fromJSON globally
+  %rename("$ignore", regextarget=1, fullname=1) "openstudio::.*::toJSON$";
+  %rename("$ignore", regextarget=1, fullname=1) "openstudio::.*::fromJSON$";


If SWIGCSHARP, ignore toJSON and fromJSON globally.

jmarrec · 2023-08-24T08:29:22Z

src/utilities/filetypes/Filetypes.i

+  // Ignore Json::Value return type (toJSON / fromJSON are globally ignored already)
  %ignore openstudio::StandardsJSON::getPrimaryKey;

+  %ignore openstudio::CustomOutputAdapter::CustomOutputAdapter;
+  %ignore openstudio::CustomOutputAdapter::optionsJSON;


Ignore other Json::Value related things in Filetypes.i when C#

This will unbrick the develop build (@wenyikuang FYI), which fails due to this problem currently, after the merge of #4917 (which we didn't realize it was going to break the build because the CI incremental windows runner does not build c# bindings...), cf https://ci.openstudio.net/blue/organizations/jenkins/openstudio-develop-full/detail/openstudio-develop-full/360/pipeline/98#step-101-log-921

D:\OSN\build\csharp_wrapper\generated_sources\OpenStudioUtilitiesData\SWIGTYPE_p_Json__Value.cs(13,14): error CS0101: The namespace 'OpenStudio' already contains a definition for 'SWIGTYPE_p_Json__Value' [D:\OSN\build\csharp_wrapper\OpenStudio.csproj] D:\OSN\build\csharp_wrapper\generated_sources\OpenStudioUtilitiesFileTypes\SWIGTYPE_p_Json__Value.cs(13,14): error CS0101: The namespace 'OpenStudio' already contains a definition for 'SWIGTYPE_p_Json__Value' [D:\OSN\build\csharp_wrapper\OpenStudio.csproj] D:\OSN\build\csharp_wrapper\generated_sources\OpenStudioUtilitiesData\SWIGTYPE_p_Json__Value.cs(16,12): error CS0111: Type 'SWIGTYPE_p_Json__Value' already defines a member called 'SWIGTYPE_p_Json__Value' with the same parameter types [D:\OSN\build\csharp_wrapper\OpenStudio.csproj] D:\OSN\build\csharp_wrapper\generated_sources\OpenStudioUtilitiesData\SWIGTYPE_p_Json__Value.cs(20,13): error CS0111: Type 'SWIGTYPE_p_Json__Value' already defines a member called 'SWIGTYPE_p_Json__Value' with the same parameter types [D:\OSN\build\csharp_wrapper\OpenStudio.csproj] D:\OSN\build\csharp_wrapper\generated_sources\OpenStudioUtilitiesFileTypes\SWIGTYPE_p_Json__Value.cs(16,12): error CS0111: Type 'SWIGTYPE_p_Json__Value' already defines a member called 'SWIGTYPE_p_Json__Value' with the same parameter types [D:\OSN\build\csharp_wrapper\OpenStudio.csproj]

jmarrec · 2023-08-24T08:55:24Z

Running a full build on this branch at https://ci.openstudio.net/blue/organizations/jenkins/openstudio-develop-full/detail/openstudio-develop-full/363/pipeline/11

Meh that failed due to a mt python error (concurrency?), replaying on the windows build at 364.
mt.exe : general error c101008d: Failed to write the updated manifest to the resource of file "Products\python\_openstudioutilitiesgeometry.pyd". Access is denied.

I Tested the changes locally on Windows 10 + macOS m1 too. C# build now works, and I do get the installed OpenStudio.dll

ci-commercialbuildings · 2023-08-24T09:31:24Z

CI Results for 709fec9:

Ubuntu 20.04:
Ubuntu 22.04:
- OpenStudio-3.7.0-alpha+709fec93d1-Ubuntu-22.04-x86_64.deb
- OpenStudio-3.7.0-alpha+709fec93d1-Ubuntu-22.04-x86_64.tar.gz
Darwin:
- OpenStudio-3.7.0-alpha+709fec93d1-Darwin-x86_64.dmg
- OpenStudio-3.7.0-alpha+709fec93d1-Darwin-x86_64.tar.gz
Windows:
- OpenStudio-3.7.0-alpha+709fec93d1-Windows.exe
- OpenStudio-3.7.0-alpha+709fec93d1-Windows.tar.gz
Code Coverage:

jmarrec · 2023-08-24T12:32:22Z

Ok 364 replay worked on windows as well: https://ci.openstudio.net/blue/organizations/jenkins/openstudio-develop-full/detail/openstudio-develop-full/364/pipeline/

Testing the windows installer to see if the OpenStudio.dll is in there

OpenStudio-3.7.0-alpha+709fec93d1-Windows$ find . -name "*.dll"
./CSharp/openstudio/openstudio_model_csharp.dll
./CSharp/openstudio/vcruntime140_1.dll
./CSharp/openstudio/msvcp140_1.dll
./CSharp/openstudio/vcruntime140.dll
./CSharp/openstudio/concrt140.dll
./CSharp/openstudio/openstudio_csharp.dll
./CSharp/openstudio/msvcp140_2.dll
./CSharp/openstudio/openstudio_translators_csharp.dll
./CSharp/openstudio/msvcp140.dll
./CSharp/openstudio/msvcp140_atomic_wait.dll
+./CSharp/openstudio/OpenStudio.dll
./CSharp/openstudio/msvcp140_codecvt_ids.dll

jmarrec · 2023-08-24T12:38:09Z

Merging right away, tested it works, and this is going to fix develop nightly builds which is important!

jmarrec added component - C# Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge. labels Jul 19, 2023

jmarrec self-assigned this Jul 19, 2023

jmarrec commented Jul 19, 2023

View reviewed changes

jmarrec mentioned this pull request Jul 19, 2023

Why is OpenStudio.dll no longer included in the installers? #4931

Closed

jmarrec added 3 commits August 16, 2023 17:15

Fix #4931 - Install C# OpenStudio.dll properly.

3214cb2

CONFIGURATION genex is deprecated since cmake 3.0 and replaced with CONFIG

Shush SWIG warnings: 516 (overloaded) + -py3 flag deprecated

b0f08cb

Ignore JSON stuff for C#

5e33131

jmarrec force-pushed the 4931_Csharp_OpenStudio_dll branch from 2ba6376 to 5e33131 Compare August 24, 2023 08:04

jmarrec added 2 commits August 24, 2023 10:11

Make a cmake message a DEBUG one

d0dbd46

don't try to do a C# install if it's not selected

5bc0985

jmarrec commented Aug 24, 2023

View reviewed changes

Final tweaks: fix install rule on windows

709fec9

jmarrec requested a review from wenyikuang August 24, 2023 10:26

jmarrec merged commit a04ae99 into develop Aug 24, 2023
2 checks passed

jmarrec deleted the 4931_Csharp_OpenStudio_dll branch August 24, 2023 12:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #4931 - Install C# OpenStudio.dll properly. #4933

Fix #4931 - Install C# OpenStudio.dll properly. #4933

jmarrec commented Jul 19, 2023 •

edited

Loading

jmarrec Jul 19, 2023

jmarrec Jul 19, 2023

jmarrec Jul 19, 2023

jmarrec Jul 19, 2023

jmarrec Aug 24, 2023

jmarrec Aug 24, 2023

jmarrec Aug 24, 2023

jmarrec Aug 24, 2023

jmarrec Aug 24, 2023

jmarrec Aug 24, 2023

jmarrec Aug 24, 2023

jmarrec Aug 24, 2023 •

edited

Loading

jmarrec commented Aug 24, 2023 •

edited

Loading

ci-commercialbuildings commented Aug 24, 2023 •

edited

Loading

jmarrec commented Aug 24, 2023 •

edited

Loading

jmarrec commented Aug 24, 2023

		install(FILES "${OPENSTUDIO_CSHARP_DLL}" DESTINATION CSharp/openstudio/ CONFIGURATIONS DEBUG COMPONENT "CSharpAPI")


		install(FILES "${OPENSTUDIO_CSHARP_DLL}" DESTINATION CSharp/openstudio/ COMPONENT "CSharpAPI")

		# Note: can't use CMAKE_BUILD_TYPE as it's empty for multi generators (MSVC for eg). So we use a genex
		set(OPENSTUDIO_CSHARP_DLL "${CSHARP_LIBRARY_OUTPUT_DIRECTORY}/$<CONFIG>/netstandard2.0/OpenStudio.dll")

		# The -py3 flag is now deprecated as Python 3 is the default the version used is Python 3
		set(SWIG_PYTHON_3_FLAGS "-relativeimport")

Fix #4931 - Install C# OpenStudio.dll properly. #4933

Fix #4931 - Install C# OpenStudio.dll properly. #4933

Conversation

jmarrec commented Jul 19, 2023 • edited Loading

Pull request overview

Pull Request Author

Review Checklist

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

jmarrec Aug 24, 2023 • edited Loading

Choose a reason for hiding this comment

jmarrec commented Aug 24, 2023 • edited Loading

ci-commercialbuildings commented Aug 24, 2023 • edited Loading

jmarrec commented Aug 24, 2023 • edited Loading

jmarrec commented Aug 24, 2023

jmarrec commented Jul 19, 2023 •

edited

Loading

jmarrec Aug 24, 2023 •

edited

Loading

jmarrec commented Aug 24, 2023 •

edited

Loading

ci-commercialbuildings commented Aug 24, 2023 •

edited

Loading

jmarrec commented Aug 24, 2023 •

edited

Loading