Skip to content

Bump conan dependencies (which will allow MSVC 2022 and C++20) and clang 13.1+ fixes#4596

Merged
tijcolem merged 51 commits intodevelopfrom
bump_deps
Jun 21, 2022
Merged

Bump conan dependencies (which will allow MSVC 2022 and C++20) and clang 13.1+ fixes#4596
tijcolem merged 51 commits intodevelopfrom
bump_deps

Conversation

@jmarrec
Copy link
Collaborator

@jmarrec jmarrec commented May 24, 2022

Pull request overview

Boost msut be bumped to 1.78+ for C++20

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 and others added 30 commits April 4, 2022 10:56
Similar to the C++ 20 branch commit: 1a6c628
… am applying it to conan-openstudio-ruby at the same time)
… called with a boost::optional

```
 1596 |   BOOST_STATIC_ASSERT_MSG(sizeof(CharType) == 0, "If you want to output boost::optional, include header <boost/optional/optional_io.hpp>");
```
…:filesystem (which is worse), and doesn't treat a directory with a trailing separator the same way

cf https://www.boost.org/doc/libs/1_78_0/libs/filesystem/doc/v4.html
@jmarrec jmarrec requested a review from tijcolem May 24, 2022 00:21
@jmarrec jmarrec self-assigned this May 24, 2022
Comment on lines +1 to +9
name: "Conan Dependabot"
on:
# schedule:
# # Every week on Thursdays
# - cron: '23 10 * * 4'
push:
branches: [ bump_deps, dependabot_conan ]

jobs:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added a dependabot for conan...

Comment on lines 84 to 86
set(CONAN_RUBY
"openstudio_ruby/2.7.2@jmarrec/testing#98444b7bc8d391ea1521d7f79d4d4926"
) # TODO: pending https://github.com/NREL/conan-openstudio-ruby/pull/39
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Comment on lines 99 to 110
"openssl/1.1.1o#213dbdeb846a4b40b4dec36cf2e673d7" # force every package to align on the same as our conan-openstudio-ruby
"boost/1.79.0#f664bfe40e2245fa9baf1c742591d582"
"pugixml/1.12.1#5a39f82651eba3e7d6197903a3202e21"
"jsoncpp/1.9.5#536d080aa154e5853332339bf576747c"
"minizip/1.2.12#0b5296887a2558500d0323c6c94c8d02" # This depends on zlib/1.2.11, and basically patches it
"zlib/1.2.12#3b9e037ae1c615d045a06c67d88491ae" # Also needed, so we can find zlib.h and co (+ pinning exactly is good)
"fmt/8.1.1#b3e969f8561a85087bd0365c09bbf4fb"
"sqlite3/3.38.5#68f3bf289cde01176e39355869c39ac0"
"cpprestsdk/2.10.18#df2f6ac88e47cadd9c9e8e0971e00d89"
"websocketpp/0.8.2#3fd704c4c5388d9c08b11af86f79f616"
"geographiclib/1.52#76536a9315a003ef3511919310b2fe37"
"swig/4.0.2#9fcccb1e39eed9acd53a4363d8129be5"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

bump bump bump

Comment on lines +1 to +4
import re
import shlex
import subprocess
from pathlib import Path
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the dependabot

Comment on lines +187 to +188
ASSERT_EQ(i_central_hp.getString(CentralHeatPumpSystemFields::CoolingLoopInletNodeName).get(),
central_hp.supplyInletModelObject().get().nameString());
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Lots of these, when EXPECT_EQ is used with one of two arguments being a boost::optional and not a T

 1596 |   BOOST_STATIC_ASSERT_MSG(sizeof(CharType) == 0, "If you want to output boost::optional, include header <boost/optional/optional_io.hpp>");

Comment on lines +71 to +100
// Boost 1.73.0 has this bit in has_self_intersections.hpp:
//
// For backward compatibility
// template <typename Geometry>
// inline bool has_self_intersections(Geometry const& geometry, bool throw_on_self_intersection = true) {
// typedef typename geometry::point_type<Geometry>::type point_type;
// typedef typename geometry::rescale_policy_type<point_type>::type rescale_policy_type;

// typename strategy::intersection::services::default_strategy<typename cs_tag<Geometry>::type>::type strategy;

// rescale_policy_type robust_policy = geometry::get_rescale_policy<rescale_policy_type>(geometry, strategy);

// return has_self_intersections(geometry, strategy, robust_policy, throw_on_self_intersection);
// }

// For backward compatibility
template <typename Geometry>
inline bool has_self_intersections(Geometry const& geometry, bool throw_on_self_intersection = true) {
using point_type = typename boost::geometry::point_type<Geometry>::type;
using rescale_policy_type = typename boost::geometry::rescale_policy_type<point_type>::type;

typename boost::geometry::strategies::relate::services::default_strategy<Geometry, Geometry>::type strategy;

// typename boost::geometry::strategy::intersection::services::default_strategy<typename boost::geometry::cs_tag<Geometry>::type>::type strategy;

auto robust_policy = boost::geometry::get_rescale_policy<rescale_policy_type>(geometry, strategy);

return boost::geometry::detail::overlay::has_self_intersections(geometry, strategy, robust_policy, throw_on_self_intersection);
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

An actual change. has_self_intersections overload we're using was finally removed in 1.76. implement workaround


openstudio::UnzipFile uf(p);
uf.setChunksize(state.range(0));
uf.setChunksize(static_cast<unsigned long>(state.range(0)));
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

MSVC 2022 was complaining here

#include "../core/Logger.hpp"
#include "../core/Optional.hpp"

#include <optional>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

some includes here and there

Json::Value defaultValue(".");
Json::Value root = m_value.get("root", defaultValue);
return toPath(root.asString());
return toPath(root.asString()).remove_trailing_separator();
Copy link
Collaborator Author

@jmarrec jmarrec May 24, 2022

Choose a reason for hiding this comment

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

Boost filesystem in 1.78 is now V4, and it follows the C++17 STL std::filesystem (which is worse), and doesn't treat a directory with a trailing separator the same way

cf https://www.boost.org/doc/libs/1_78_0/libs/filesystem/doc/v4.html

ref c0248cf

@tijcolem
Copy link
Collaborator

tijcolem commented May 24, 2022

okay, NatLabRockies/conan-openstudio-ruby#39 is now merged. Do we need to vendor any of deps up to NREL remote before merging this in? Since I don't have an m1 yet I can't build and push conan packages for m1.

@jmarrec
Copy link
Collaborator Author

jmarrec commented Jun 14, 2022

Full build running at https://ci.openstudio.net/blue/organizations/jenkins/openstudio-develop-nightly/detail/openstudio-develop-nightly/1534/pipeline/

Platform Build Tests
macOS OK OK
Ubuntu 18.04 OK OK
Ubuntu 20.04 OK OK
Windows x86_64 OK TDB
Windows x86 (nuget) FAIL TDB

@jmarrec jmarrec mentioned this pull request Jun 14, 2022
19 tasks
jmarrec added 3 commits June 14, 2022 15:03
…l be removed) and force a x86_64 arch **BUILD** profile even when targeting x86 (for the win32 nuget...)
@jmarrec
Copy link
Collaborator Author

jmarrec commented Jun 14, 2022

Reruning at https://ci.openstudio.net/blue/organizations/jenkins/openstudio-develop-nightly/detail/openstudio-develop-nightly/1537/pipeline after making significant changes to how we use cmake-conan so we can force a build profile with a settings.arch=x86_64 while the host profile is at settings.arch=x86 so we can build the csharp nuget in win 32bit mode. msys2 is particular (which is a build_requires of a LOT of recipes like m4, bison, etc) completely dropped support for 32bit due to the cygwin upstream dropping it in the first place

cf https://www.msys2.org/news/#2020-05-17-32-bit-msys2-no-longer-actively-supported

2020-05-17 - 32-bit MSYS2 no longer actively supported
32-bit mingw-w64 packages are still supported, this is about the POSIX emulation layer, i.e. the runtime, Bash, MinTTY...
After this date, we don't plan on building updated msys-i686 packages nor releasing i686 installers anymore. This is due to increasingly frustrating difficulties with limited 32-bit address space, high penetration of 64-bit systems and Cygwin (our upstream) starting their way to drop 32-bit support as well.

Platform Build Tests
macOS OK OK
Ubuntu 18.04 OK OK
Ubuntu 20.04 OK OK
Windows x86_64 OK TDB
Windows x86 (nuget) FAIL TDB

Before it was doing `ASSERT_EQ(cooledBeam.supplyAirVolumetricFlowRate(), cloneBeam.supplyAirVolumetricFlowRate());` when really it didn't have a value. I changed that to ASSERT_EQ(lhs.get(), rhs.get()) and logically it started throwing because it's not initialized
@jmarrec
Copy link
Collaborator Author

jmarrec commented Jun 16, 2022

Rerunning again at https://ci.openstudio.net/blue/organizations/jenkins/openstudio-develop-nightly/detail/openstudio-develop-nightly/1540/pipeline after I manually built all win32 (x86) deps and uploaded them to NREL's remote. I sincerely hope it works this time...

I'm going to be building binary packages with MSVC 2022 and upload them as well in the meantime.

Why was openssl not building?

It seems it's due to the fact the DevTools were already initialized to x64 in the command prompt. OpenSSL starts by calling vcvarsall.bat

openssl/1.1.1o: Calling build()
**********************************************************************
** Visual Studio 2022 Developer Command Prompt v17.1.6
** Copyright (c) 2022 Microsoft Corporation
**********************************************************************
[vcvarsall.bat] Environment initialized for: 'x86'

As a result later you end up with:

        cl  /Zi /Fdossl_static.pdb /MD /Zl /Gs0 /GF /Gy -O2 -Ob2 -MD /W3 /wd4090 /nologo /O2 /I "." /I "include" -D"L_ENDIAN" -D"OPENSSL_PIC" -D"OPENSSL_CPUID_OBJ" -D"OPENSSL_BN_ASM_PART_WORDS" -D"OPENSSL_IA32_SSE2" -D"OPENSSL_BN_ASM_MONT" -D"OPENSSL_BN_ASM_GF2m" -D"SHA1_ASM" -D"SHA256_ASM" -D"SHA512_ASM" -D"RC4_ASM" -D"MD5_ASM" -D"RMD160_ASM" -D"AESNI_ASM" -D"VPAES_ASM" -D"WHIRLPOOL_ASM" -D"GHASH_ASM" -D"ECP_NISTZ256_ASM" -D"POLY1305_ASM" -D"OPENSSLDIR=\"C:\\Users\\julien\\.conan\\bump_deps\\openssl\\1.1.1o\\_\\_\\package\\70711aff723b218b02483e600038031bf91007bc\\res\"" -D"ENGINESDIR=\"C:\\Users\\julien\\.conan\\bump_deps\\openssl\\1.1.1o\\_\\_\\package\\70711aff723b218b02483e600038031bf91007bc\\lib\\engines-1_1\"" -D"OPENSSL_SYS_WIN32" -D"WIN32_LEAN_AND_MEAN" -D"UNICODE" -D"_UNICODE" -D"_CRT_SECURE_NO_DEPRECATE" -D"_WINSOCK_DEPRECATED_NO_WARNINGS" -D"NDEBUG" -D"NDEBUG"  /Zs /showIncludes "engines\e_padlock.c" 2>&1 > engines\e_padlock.d
        lib /nologo /out:libcrypto.lib @C:\Users\julien\AppData\Local\Temp\nm3CBB.tmp
crypto\aes\aesni-x86.obj : fatal error LNK1112: module machine type 'x86' conflicts with target machine type 'x64'
NMAKE : fatal error U1077: '"C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.31.31103\bin\HostX64\x64\lib.EXE"' : return code '0x458'
Stop.
NMAKE : fatal error U1077: '"C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.31.31103\bin\HostX64\x64\nmake.exe"' : return code '0x2'
Stop.
openssl/1.1.1o:
openssl/1.1.1o: ERROR: Package '70711aff723b218b02483e600038031bf91007bc' build failed
openssl/1.1.1o: WARN: Build folder C:\Users\julien\.conan\bump_deps\openssl\1.1.1o\_\_\build\70711aff723b218b02483e600038031bf91007bc
ERROR: openssl/1.1.1o: Error in build() method, line 758
        self._make()
while calling '_make', line 698
        self._run_make()
while calling '_run_make', line 653
        self.run(" ".join(command), win_bash=self._win_bash)
        ConanException: Error 2 while executing nmake

I would get that error by doing:

call vcvarsall.bat x64
cmake -G "Visual Studio 16 2019" -A x64  -DCMAKE_BUILD_TYPE=Release `
  -DBUILD_CSHARP_BINDINGS:BOOL=ON  -DBUILD_NUGET_PACKAGE:BOOL=ON -DBUILD_RUBY_BINDINGS:BOOL=OFF -DBUILD_CLI:BOOL=OFF `
  -DBUILD_RADIANCE:BOOL=OFF -DBUILD_DOCUMENTATION:BOOL=OFF -DBUILD_TESTING:BOOL=OFF `
  -DCPACK_BINARY_DEB:BOOL=OFF -DCPACK_BINARY_IFW:BOOL=ON -DCPACK_BINARY_NSIS:BOOL=OFF -DCPACK_BINARY_RPM:BOOL=OFF -DCPACK_BINARY_STGZ:BOOL=OFF `
  -DCPACK_BINARY_TBZ2:BOOL=OFF -DCPACK_BINARY_TGZ:BOOL=ON -DCPACK_BINARY_TXZ:BOOL=OFF -DCPACK_BINARY_TZ:BOOL=OFF `
  ../

# HERE!!!

cmake --build . --config Release --target OS-32

If between both cmake command I exit the prompt, start a new one without calling vcvarsall.bat, openssl builds.

This problem should not arise when the dependencies are downloaded

@jmarrec
Copy link
Collaborator Author

jmarrec commented Jun 16, 2022

I uploaded all x86 (Release) and MSVC 17 (Release and Debug) packages I built on my machine to the NREL remote, so we should be in business!

jmarrec added 3 commits June 17, 2022 14:51
BCL.obj : error LNK2038: mismatch detected for 'boost_log_abi': value 'v2s_mt_nt6' doesn't match value 'v2s_mt_nt62' in ruby_OpenStudioUtilitiesCore_wrap.obj [D:\OSN\build\src\cli\openstudio.vcxproj]

cf boost: include/boost/winapi/config.hpp
https://ci.openstudio.net/blue/organizations/jenkins/openstudio-develop-nightly/detail/openstudio-develop-nightly/1544/pipeline/51

Logger.obj : error LNK2019: unresolved external symbol "private: static unsigned int __cdecl boost::log::v2s_mt_nt62::attribute_name::get_id_from_string(char const *)" (?get_id_from_string@attribute_name@v2s_mt_nt62@log@boost@@CAIPBD@Z) referenced in function "private: __thiscall openstudio::LoggerSingleton::LoggerSingleton(void)" (??0LoggerSingleton@openstudio@@aae@XZ) [D:\OSN\build\OS-32\src\lib\openstudiolib.vcxproj] [D:\OSN\build\csharp\OS-32.vcxproj]
@jmarrec
Copy link
Collaborator Author

jmarrec commented Jun 20, 2022

@tijcolem It finally builds on https://ci.openstudio.net/blue/organizations/jenkins/openstudio-develop-nightly/detail/openstudio-develop-nightly/1550/pipeline/284

Copy link
Collaborator

@tijcolem tijcolem left a comment

Choose a reason for hiding this comment

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

Great that things are building!

@tijcolem tijcolem merged commit 37364f9 into develop Jun 21, 2022
@tijcolem tijcolem deleted the bump_deps branch June 21, 2022 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component - Conan Dependency / Package manager problems Developer Issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update conan packages on our NREL conan Artifactory remote (for MSVC 2022 and mac M1 in particular)

2 participants