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

CMake-based v3.2.1 #25

Merged
merged 6 commits into from
Feb 26, 2021

Conversation

jowr
Copy link
Collaborator

@jowr jowr commented Jan 6, 2021

Changes in this PR:

  • Fixed the compilation issues with the ExternalMedia library
  • Added a download link for CoolProp v5.1.1
  • Requires CMake for compilation, but has been tested with the old CMake from the OMDev environment
  • Some of the examples work with OpenModelica, but the topics discussed in v3.2.3 with CoolProp #24 also appear here.

General comments:

  • There is a new PR coming up that fixes the documentation, please only review the CompilationHowTo help file.
  • Maybe we can merge this to the master branch since it is only a fix for version v3.2.1. It is not related to v3.3.0

@jowr
Copy link
Collaborator Author

jowr commented Jan 6, 2021

@jowr
Copy link
Collaborator Author

jowr commented Jan 6, 2021

Just for the records, this PR potentially fixes #5, #7, #8, #14, #15, #21, #22.

However, the problems with OpenModelica (#9, #10 and #24) seem to be caused by the translation step and are not addressed here.

@jowr
Copy link
Collaborator Author

jowr commented Jan 11, 2021

Dear @casella - could you please share your thoughts on this contribution? It closes a lot of issues and I have the next PR ready to fix the documentation, but this one should be merged first... Let me know if you want anything modified.

@beutlich beutlich requested a review from casella January 16, 2021 13:20
Copy link

@beutlich beutlich left a comment

Choose a reason for hiding this comment

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

Successfully tested that build works with VS2010.

list (APPEND INCLUDE_DIRS "${CMAKE_CURRENT_SOURCE_DIR}/../externals/CoolProp.git/externals/cppformat")
else()
list(REMOVE_ITEM LIB_SOURCES "${CMAKE_CURRENT_SOURCE_DIR}/Sources/coolpropsolver.cpp")
endif()

## We use CMake to handle the dependency since the primary VCS for
## ExternalMedia still is SVN.

Choose a reason for hiding this comment

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

I guess, this comment is outdated.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep, I am going to remove it.

@@ -4,68 +4,36 @@ This directory contains the C/C++ source files in the Sources directory
and script files to build the library using different Modelica tools,
operating systems, and C/C++ compilers.

Instructions below build the static gcc library and copy it and the
externalmedia.h header files in the Resource directories of the

Choose a reason for hiding this comment

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

Suggested change
externalmedia.h header files in the Resource directories of the
externalmedia.h header files in the Resources directories of the

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Please do not waste time on the docs. I have a new PR lined up that fixes the docs, but this one has to be merged first.

@@ -4,68 +4,36 @@ This directory contains the C/C++ source files in the Sources directory
and script files to build the library using different Modelica tools,
operating systems, and C/C++ compilers.

Instructions below build the static gcc library and copy it and the

Choose a reason for hiding this comment

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

Suggested change
Instructions below build the static gcc library and copy it and the
Instructions below build the static library and copy it and the

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Please do not waste time on the docs. I have a new PR lined up that fixes the docs, but this one has to be merged first.

Instructions below build the static gcc library and copy it and the
externalmedia.h header files in the Resource directories of the
Modelica packages, so it can be used right away by just loading the
package in you Modelica software.

Choose a reason for hiding this comment

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

Suggested change
package in you Modelica software.
package in your Modelica software.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Please do not waste time on the docs. I have a new PR lined up that fixes the docs, but this one has to be merged first.

pushd "%EXTERNALS%"
set CP_SRC=!CD!\CoolProp.git
popd

set BUILD_DIR="build"
if not exist "%BUILD_DIR%" (mkdir "%BUILD_DIR%")
Copy link

@beutlich beutlich Jan 20, 2021

Choose a reason for hiding this comment

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

If build dir exists and I previously built MSVC for "x64" a subesequent build for "Win32" will use the same build dir and fails.

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, we may want to remove the script since the whole installation process now only needs 2 CMake commands.

Choose a reason for hiding this comment

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

Indeed, I noticed when checking the CI part which is pretty impressive,

Copy link
Collaborator

@casella casella left a comment

Choose a reason for hiding this comment

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

@jowr, sorry for the delay and thank you for your patience. Now I have this think on the table.

I'll merge this and test it, and then report.

Thank you for the hard work!

@casella casella merged commit e81bb92 into modelica-3rdparty:v.3.3.0-dev Feb 26, 2021
@casella
Copy link
Collaborator

casella commented Feb 26, 2021

@jowr, now that I look closer, I guess this PR was meant to be merged on master, not on v.3.3.0-dev, right? After merging it, v.3.3.0-dev is mostly broken. We probably need to revert it.

Can we have a short webmeeting next week to coordinate? Now I finally have time to devote to ExternalMedia, and I can also get help from @fedetft, my local linux guru. I hope you also have some time now, so we can get this through

Thanks!

@jowr jowr deleted the v3.2.1-compilation_fix branch March 9, 2021 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants