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

Develop to Master #3081

Merged
merged 27 commits into from May 12, 2023
Merged

Develop to Master #3081

merged 27 commits into from May 12, 2023

Conversation

safinsaf
Copy link
Contributor

@safinsaf safinsaf commented Jan 25, 2023

Description of the Change

Routine merging of changes from Develop to Main.

Issue

N/A

Benefits

Release 1.7.0

Possible Drawbacks

None

iceseer and others added 14 commits October 12, 2022 14:00
Signed-off-by: iceseer <iceseer@gmail.com>
Signed-off-by: G.Bazior <bazior@agh.edu.pl>
…:chrono::duration

Signed-off-by: Grzegorz Bazior <bazior@agh.edu.pl>
… repair building of abseil library: std::max ambiguous call (#2902)

Signed-off-by: Grzegorz Bazior <bazior@agh.edu.pl>

Signed-off-by: Grzegorz Bazior <bazior@agh.edu.pl>
* Add Iroha select label

Signed-off-by: safinsaf <safinsaft@gmail.com>

* Fix stray space comments from review

Signed-off-by: safinsaf <safinsaft@gmail.com>

* Fix called jobs for git merge-commit in workflow

Signed-off-by: safinsaf <safinsaft@gmail.com>

Signed-off-by: safinsaf <safinsaft@gmail.com>
… repair building of benchmark library: std::numeric_limits requires <limits> header

Signed-off-by: Grzegorz Bazior <bazior@agh.edu.pl>
Signed-off-by: Grzegorz Bazior <bazior@agh.edu.pl>
Signed-off-by: safinsaf <safinsaft@gmail.com>
Signed-off-by: Andrzej <dekoderer@gmail.com>
Signed-off-by: Andrzej <dekoderer@gmail.com>
Added description for rpi

Signed-off-by: Grzegorz Bazior <bazior@agh.edu.pl>
Apply suggestion

Co-authored-by: Aleksandr Petrosyan <a-p-petrosyan@yandex.ru>
Signed-off-by: G. Bazior <gbaz@o2.pl>
Corrected typo

Signed-off-by: G. Bazior <bazior@agh.edu.pl>
Copy link
Member

@baziorek baziorek left a comment

Choose a reason for hiding this comment

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

I was able to review 40/56 files connected with features/documentation/internship project in which I was participating. Here is file list of what was reviewed by me and it is OK (files are marked as "Viewed"):
image

About rest of files I don't have knowledge. But maybe if You provide more information e.g.:
how the MergeRequest was tested (which commands) and what was results I will be able to review also rest of files.

@baziorek
Copy link
Member

baziorek commented Feb 13, 2023

I was able to review 40/56 files connected with features/documentation/internship project in which I was participating. Here is file list of what was reviewed by me and it is OK (files are marked as "Viewed"):

About rest of files I don't have knowledge. But maybe if You provide more information e.g.: how the MergeRequest was tested (which commands) and what was results I will be able to review also rest of files.

Here is list of files which I was unable to review:

  1. .github/build-iroha1-fork.src.yml
  2. .github/build-iroha1.src.yml
  3. .github/workflows/build-iroha1-fork.yml
  4. .github/workflows/build-iroha1.yml - I'm not expert about .github CI
  5. docs/source/requirements.txt - this file is for Sphinx documentation
  6. irohad/consensus/yac/transport/impl/consensus_service_impl.cpp
  7. irohad/consensus/yac/transport/impl/consensus_service_impl.hpp
  8. irohad/consensus/yac/transport/impl/network_impl.cpp
  9. irohad/consensus/yac/transport/impl/network_impl.hpp
  10. irohad/ordering/impl/on_demand_ordering_service_impl.cpp
  11. schema/yac.proto
  12. shared_model/validators/validators_common.cpp
  13. test/module/irohad/consensus/yac/network_test.cpp
  14. test/module/irohad/ordering/on_demand_os_test.cpp

It looks that all C++ files not reviewed by me are from this commit: #2749 (made by Alexander). Changes to yml files are made here: #2903 (made by @safinsaf) and cef5e4d (also made by @safinsaf )

.github/build-iroha1.src.yml Outdated Show resolved Hide resolved
.github/workflows/build-iroha1.yml Show resolved Hide resolved
# ${{env.HOME}}/.cache/vcpkg/archives
# key: ${{runner.os}}-vcpkg-${{env.CC_NAME}}.${{ hashFiles('vcpkg/**') }}
# restore-keys: ${{runner.os}}-vcpkg-${{env.CC_NAME}}.
name: Build iroha vcpkg dependancies
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
name: Build iroha vcpkg dependancies
name: Build iroha vcpkg dependencies

.github/workflows/build-iroha1.yml Show resolved Hide resolved
.github/workflows/build-iroha1.yml Show resolved Hide resolved
iroha-lib/model/LibsAndClassDeclarations.h Outdated Show resolved Hide resolved
iroha-lib/model/ClassDeclarations.h Outdated Show resolved Hide resolved
iroha-lib/model/ClassDeclarations.h Outdated Show resolved Hide resolved
iroha-lib/model/Query.cpp Outdated Show resolved Hide resolved
iroha-lib/model/Query.hpp Outdated Show resolved Hide resolved
@appetrosyan
Copy link
Contributor

I reviewed the code, while there are a few things which I would have done differently, there's no problem with that in principle.

appetrosyan
appetrosyan previously approved these changes Mar 14, 2023
Grzegorz Bazior and others added 3 commits March 19, 2023 19:13
Signed-off-by: Grzegorz Bazior <g.bazior@yodiss.pl>
Co-authored-by: Aleksandr Petrosyan <a-p-petrosyan@yandex.ru>
Signed-off-by: G. Bazior <gbaz@o2.pl>
Signed-off-by: Grzegorz Bazior <g.bazior@yodiss.pl>
Signed-off-by: Grzegorz Bazior <g.bazior@yodiss.pl>
Grzegorz Bazior added 2 commits March 19, 2023 19:13
Signed-off-by: Grzegorz Bazior <g.bazior@yodiss.pl>
Signed-off-by: Grzegorz Bazior <g.bazior@yodiss.pl>
Signed-off-by: Aleksandr Petrosyan <a-p-petrosyan@yandex.ru>
baziorek pushed a commit that referenced this pull request Mar 24, 2023
Signed-off-by: Grzegorz Bazior <bazior@agh.edu.pl>
@baziorek baziorek mentioned this pull request Mar 24, 2023
5 tasks
baziorek pushed a commit that referenced this pull request Mar 24, 2023
Signed-off-by: Grzegorz Bazior <bazior@agh.edu.pl>
Signed-off-by: Grzegorz Bazior <g.bazior@yodiss.pl>
@baziorek
Copy link
Member

baziorek commented Mar 24, 2023

@appetrosyan I've corrected changes applied to me in another PR: #3328

About failing build:
image
I don't know what it is. For sure not c++:D.

About MacOs:
image
I don't have computer with MacOs. If You send me file:
/Users/runner/work/iroha/iroha/vcpkg-build/buildtrees/benchmark/install-x64-osx-rel-out.log maybe I would be able to fix

EDIT: It is similar thing to what I fixed (I hope it is similar):
#2906

@baziorek
Copy link
Member

EDIT: It is similar thing to what I fixed (I hope it is similar): #2906
I have the log file for MacOS. This is the reason:

[5/22] /Library/Developer/CommandLineTools/usr/bin/clang++ -DHAVE_POSIX_REGEX -DHAVE_STD_REGEX -DHAVE_STEADY_CLOCK -I/Users/qwerty/workdir/iroha/vcpkg-build/buildtrees/benchmark/src/eeee844489-b89864c19e.clean/include -I/Users/qwerty/workdir/iroha/vcpkg-build/buildtrees/benchmark/src/eeee844489-b89864c19e.clean/src -I/Users/qwerty/workdir/iroha/vcpkg-build/buildtrees/benchmark/src/eeee844489-b89864c19e.clean/src/../include -fPIC  -std=c++11  -Wall  -Wextra  -Wshadow  -Wshorten-64-to-32  -fstrict-aliasing  -Wno-deprecated-declarations  -Wstrict-aliasing  -Wthread-safety -O3 -DNDEBUG  -Werror  -Wno-deprecated -arch x86_64 -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX13.1.sdk -mmacosx-version-min=12.6 -MD -MT src/CMakeFiles/benchmark.dir/complexity.cc.o -MF src/CMakeFiles/benchmark.dir/complexity.cc.o.d -o src/CMakeFiles/benchmark.dir/complexity.cc.o -c /Users/qwerty/workdir/iroha/vcpkg-build/buildtrees/benchmark/src/eeee844489-b89864c19e.clean/src/complexity.cc
FAILED: src/CMakeFiles/benchmark.dir/complexity.cc.o 
/Library/Developer/CommandLineTools/usr/bin/clang++ -DHAVE_POSIX_REGEX -DHAVE_STD_REGEX -DHAVE_STEADY_CLOCK -I/Users/qwerty/workdir/iroha/vcpkg-build/buildtrees/benchmark/src/eeee844489-b89864c19e.clean/include -I/Users/qwerty/workdir/iroha/vcpkg-build/buildtrees/benchmark/src/eeee844489-b89864c19e.clean/src -I/Users/qwerty/workdir/iroha/vcpkg-build/buildtrees/benchmark/src/eeee844489-b89864c19e.clean/src/../include -fPIC  -std=c++11  -Wall  -Wextra  -Wshadow  -Wshorten-64-to-32  -fstrict-aliasing  -Wno-deprecated-declarations  -Wstrict-aliasing  -Wthread-safety -O3 -DNDEBUG  -Werror  -Wno-deprecated -arch x86_64 -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX13.1.sdk -mmacosx-version-min=12.6 -MD -MT src/CMakeFiles/benchmark.dir/complexity.cc.o -MF src/CMakeFiles/benchmark.dir/complexity.cc.o.d -o src/CMakeFiles/benchmark.dir/complexity.cc.o -c /Users/qwerty/workdir/iroha/vcpkg-build/buildtrees/benchmark/src/eeee844489-b89864c19e.clean/src/complexity.cc
/Users/qwerty/workdir/iroha/vcpkg-build/buildtrees/benchmark/src/eeee844489-b89864c19e.clean/src/complexity.cc:85:10: error: variable 'sigma_gn' set but not used [-Werror,-Wunused-but-set-variable]
  double sigma_gn = 0.0;
         ^
1 error generated.
ninja: build stopped: subcommand failed

So now I should make path to vcpkg packages which would remove -Werror from benchmark library

Signed-off-by: safinsaf <safinsaft@gmail.com>
Signed-off-by: safinsaf <safinsaft@gmail.com>
Signed-off-by: safinsaf <safinsaft@gmail.com>
Grzegorz Bazior and others added 3 commits April 13, 2023 12:48
2. Removed incorrect patches to vcpkg libraries after VCPKG version bump
3. Fixed building for irohad and iroha-cli after after VCPKG version bump (and 3rd part libraries version bump)

Signed-off-by: Grzegorz Bazior <g.bazior@yodiss.pl>
Signed-off-by: safinsaf <safinsaft@gmail.com>
Signed-off-by: Grzegorz Bazior <bazior@agh.edu.pl>
Signed-off-by: Grzegorz Bazior <g.bazior@yodiss.pl>
@baziorek
Copy link
Member

@appetrosyan Your suggestions was corrected in #3328, which was merged.
There are few comments to correct connected with *.yml files, but I don't know those files, so somebody else should correct those suggestions.


Building tests is failing, but if somebody tries to build Iroha according to documentation: https://iroha.readthedocs.io/en/main/build/index.html#id9
without test with command:

cmake --build ./build --target irohad

everything should work. I think nobody (who is beginner with Iroha) would like to build tests:

cmake --build ./build --target all

@appetrosyan appetrosyan merged commit f3e4226 into main May 12, 2023
28 of 48 checks passed
6r1d pushed a commit to 6r1d/iroha that referenced this pull request Aug 28, 2023
Signed-off-by: Grzegorz Bazior <bazior@agh.edu.pl>
Signed-off-by: Grzegorz Bazior <g.bazior@yodiss.pl>
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

5 participants