-
Notifications
You must be signed in to change notification settings - Fork 296
Conversation
Signed-off-by: Nikita Alekseev <[email protected]> # Conflicts: # irohad/ordering/impl/ordering_gate_transport_grpc.hpp # irohad/ordering/impl/ordering_service_impl.cpp # test/module/irohad/validation/CMakeLists.txt Merge pull request #3 from laSinteZ/patch-1 fix: const "size" not matching size of message Signed-off-by: Nikita Alekseev <[email protected]>
Signed-off-by: Nikita Alekseev <[email protected]> # Conflicts: # irohad/ordering/impl/ordering_gate_transport_grpc.hpp # irohad/ordering/impl/ordering_service_impl.cpp # test/module/irohad/validation/CMakeLists.txt Merge pull request #3 from laSinteZ/patch-1 fix: const "size" not matching size of message Signed-off-by: Nikita Alekseev <[email protected]>
This is brew distributed for mac os Signed-off-by: Nikita Alekseev <[email protected]> # Conflicts: # irohad/ordering/impl/ordering_gate_transport_grpc.hpp # irohad/ordering/impl/ordering_service_impl.cpp # test/module/irohad/validation/CMakeLists.txt Merge pull request #3 from laSinteZ/patch-1 fix: const "size" not matching size of message Signed-off-by: Nikita Alekseev <[email protected]>
Just FYI, cmake version in Ubuntu 18.04 LTS is 3.10.2. |
Yep, but we will need the functionality introduced in 3.11 in the future. Downloading and installing CMake is very straightforward, and all our development containers do that. |
Ok, np. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is android dockerfile also supported at the moment? https://github.com/hyperledger/iroha/blob/47304cc034986f1b578e3c1eece210b76b09c037/docker/android/Dockerfile#L1 @bakhtin
The latest cmake version in homebrew is 3.12.1 https://formulae.brew.sh/formula/cmake , so I do not think there is any benefit from using 3.11.2 instead of 3.11.4
This file https://github.com/hyperledger/iroha/blob/47304cc034986f1b578e3c1eece210b76b09c037/docs/source/guides/libraries/nodejs.rst#prerequisites-1 should be either removed, or updated.
Signed-off-by: Nikita Alekseev <[email protected]> # Conflicts: # irohad/ordering/impl/ordering_gate_transport_grpc.hpp # irohad/ordering/impl/ordering_service_impl.cpp # test/module/irohad/validation/CMakeLists.txt Merge pull request #3 from laSinteZ/patch-1 fix: const "size" not matching size of message Signed-off-by: Nikita Alekseev <[email protected]>
RUN set -e; \ | ||
git clone https://gitlab.kitware.com/cmake/cmake.git /tmp/cmake; \ | ||
(cd /tmp/cmake ; git checkout 316bd45439ad8ced6b31bcb10303a788038387ef); \ | ||
(cd /tmp/cmake ; /tmp/cmake/bootstrap --system-curl --parallel=${PARALLELISM} --enable-ccache); \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no PARALLELISM
parameter defined. See other Docker files for how to setup it properly.
Android Docker image will soon be removed as we're migrating to a separate libiroha repo. Yet do we need to bump CMake version in that repo? Will we be using any new CMake functionality there? |
Since it is not removed yet, we need to make sure the project can be built there. |
This allows the usage of FetchContent which is useful for CMake based dependencies Signed-off-by: Nikita Alekseev <[email protected]>
Description of the Change
This PR updates minimum CMake version requirements to 3.11.2. This allows using new features such as FetchContent.
Benefits
More features, fewer bugs in CMake.
Possible Drawbacks
Some users will have to manually install CMake when trying to build iroha.