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

Trying to fix sfinae problems by reverting vcpkg upgrade (Iroha 1) #3852

Merged
merged 3 commits into from Sep 1, 2023

Conversation

baziorek
Copy link
Member

@baziorek baziorek commented Aug 30, 2023

Description

Some time ago vcpkg package manager was bumped to newest version (commit: fdc2c1e). It caused dependencies to upgrade a lot so project stopped to compile. I was able to fix compilation of binaries (commit: 3b40f22) but I was unable to fix compilation of tests (SFINAE problem) after some time of trying.

So I decided that we need to make project compiling with not the most professional way, but it should work (and be first step for more professional way). The way is to revert those commits:

  1. fdc2c1e
  2. 3b40f22

After those reverts it compiled locally (I used docker: docker run -it --name='Ubuntu_under_test_22.04' -v ${PWD}/./:/home/ --workdir=/home/ ubuntu:22.04 with g++ (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0) - both binaries and tests compiled with commands:

cmake -B build -DCMAKE_TOOLCHAIN_FILE=$PWD/vcpkg-build/scripts/buildsystems/vcpkg.cmake . -DCMAKE_BUILD_TYPE=RELEASE   -GNinja -DUSE_BURROW=OFF -DUSE_URSA=OFF -DTESTING=ON -DPACKAGE_DEB=OFF  # testing ON
cmake --build ./build --target all  # target = all

For now I want to check if CI is compiling, that is why the PR is not ready for review yet.

Difference between versions

  1. Old version of vcpkg:
$ ./vcpkg-build/vcpkg version
Vcpkg package management program version 2021-01-13-unknownhash
$ git log -1
commit e8dbfcf6797a270ed5be8550248f7fe4fe5dec79 (HEAD)
Author: Matthias C. M. Troffaes <matthias.troffaes@gmail.com>
Date:   Tue May 4 07:14:55 2021 +0100

    [ffmpeg] soxr dependency fix (#17299)
    
    * [ffmpeg] soxr only makes sense with swresample, so add as dependency
    
    * [ffmpeg] bump port version
    
    * [ffmpeg] x-add-version
  1. New version of vcpkg:
$ ./vcpkg-build_newest/vcpkg version
vcpkg package management program version 2023-02-16-12e657924d99511514c0287ca5ce46882d3657c7
$ git log -1
commit a7b6122f6b6504d16d96117336a0562693579933 (HEAD, tag: 2023.02.24)
Author: Frank <65999885+FrankXie05@users.noreply.github.com>
Date:   Sat Feb 25 06:08:21 2023 +0800

    [fmilib] Change to the github and update to fix bug of libexpat (#29805)

Version compare in table (generated by ChatGpt, so there can be mistakes):

Library Before Upgrade After Upgrade
abseil 2021-03-24 20230125.0
benchmark 1.5.2 1.7.1
boost-accumulators 1.75.0 1.81.0#2
boost-algorithm 1.75.0 1.81.0#2
boost-align 1.75.0 1.81.0#2
boost-any 1.75.0 1.81.0#2
boost-array 1.75.0 1.81.0#2
boost-asio 1.75.0#1 1.81.0#2
boost-assert 1.75.0 1.81.0#2
iroha-ed25519 2.0.1 2.0.1
liblzma 5.2.5#2 5.4.1#1
libpq 12.2#16 14.4#3
libpq[openssl] ... ...
libpq[zlib] ... ...
lz4 ... 1.9.4#1
nlohmann-json 3.9.1 3.11.2
openssl 1.1.1k 3.0.8
prometheus-cpp 0.12.2 1.1.0
prometheus-cpp[compression] ... ...
prometheus-cpp[pull] ... ...
protobuf 3.15.8 3.21.12
rapidjson 2020-09-14 2022-06-28#3
re2 2020-10-01 2023-02-01
rocksdb 6.14.6 7.9.2
rocksdb[zlib] ... ...
rxcpp 4.1.0-1 4.1.1#1
soci 4.0.1#3 4.0.3
soci[boost] ... ...
soci[postgresql] ... ...
spdlog 1.8.5#2 1.11.0
upb 2020-12-19#1 2022-06-21
upb[codegen] ... ...
vcpkg-cmake-config 2021-02-26#1 2022-02-06#1
vcpkg-cmake-get-vars ... 2022-12-16
vcpkg-cmake 2021-02-28#1 2022-12-22
zlib 1.2.11#10 1.2.13
zstd 1.4.9 1.5.4

vcpkg_libraries_new.txt
vcpkg_libraries_old.txt

Future plans:

If my fix works packages can be upgraded one by one until we face the SFINAE problem again - that package/packages would stay in older version. Of course this can be done in another PR.

Linked issue

Closes #{issue_number}

Benefits

Iroha 1 starts compilling again and another release 1.6 would be possible.

Checklist

  • I've read CONTRIBUTING.md
  • I've used the standard signed-off commit format (or will squash just before merging)
  • All applicable CI checks pass (or I promised to make them pass later)
  • (optional) I've written unit tests for the code changes
  • I replied to all comments after code review, marking all implemented changes with thumbs up

@baziorek baziorek added version bump 1.x build config-changes Changes in configuration and start up of the Iroha CI 1.5 Dev defect The defect was found at the development stage. Did not have an impact on users. labels Aug 30, 2023
@baziorek baziorek self-assigned this Aug 30, 2023
@baziorek baziorek force-pushed the trying2fixSfinaeProblemsByRevertingVcpkgUpgrade branch 2 times, most recently from 5d5330a to 29bd61d Compare August 31, 2023 06:28
@baziorek
Copy link
Member Author

baziorek commented Aug 31, 2023

I checked the CTest problem:
image
I run the test locally and it passes:

./build$ ../test/tool/test_tool_iroha_wsv_diff.sh -block_store_path /home/iroha/build/test_data/block_store/ -bin_dir /home/iroha/build/bin/

Then I downloaded && checked logs from CTest and it looks that the test is passing:

2023-08-31T00:13:44.7806214Z [  PASSED  ] 3. test_odd_domain_account_role
2023-08-31T00:13:44.7806359Z ----------------------------------------------------
2023-08-31T00:13:44.7806456Z TOTAL 3 tests were executed.
2023-08-31T00:13:44.7806552Z PASSED ALL 3 of 3 tests: 
2023-08-31T00:13:44.7806650Z    [  PASSED  ] 1. test_equal_wsv
2023-08-31T00:13:44.7806785Z    [  PASSED  ] 2. test_wrong_permissions_and_asset_amount
2023-08-31T00:13:44.7806906Z    [  PASSED  ] 3. test_odd_domain_account_role
2023-08-31T00:13:44.7807001Z HOORAY. all tests PASSED.
2023-08-31T00:13:44.7807010Z 
2023-08-31T00:13:44.7807016Z 
2023-08-31T00:13:44.7807136Z 75% tests passed, 1 tests failed out of 4

So all 3 tests are passing, but somewhere test executor expects 4 tests.

EDIT: Interesting thing is that there are both tests:

  1. "build-UD (ubuntu gcc-10 Debug normal)"
  2. "build-UR (ubuntu gcc-10 Release normal)"
    Debug is passing, because it is not executing the test (log file is much shorter).
    So to fix the issue somebody should have devops knowledge. If nobody has time we can add tool_iroha_wsv_diff to iroha/.github/TESTS_ALLOWED_TO_FAIL?

@baziorek baziorek marked this pull request as ready for review August 31, 2023 07:34
Grzegorz Bazior (Yodiss PSA) added 3 commits September 1, 2023 15:35
This reverts commit fdc2c1e.

Signed-off-by: Grzegorz Bazior (Yodiss PSA) <g.bazior@yodiss.pl>
This reverts commit 3b40f22.

Signed-off-by: Grzegorz Bazior (Yodiss PSA) <g.bazior@yodiss.pl>
Signed-off-by: Grzegorz Bazior (Yodiss PSA) <g.bazior@yodiss.pl>
@baziorek baziorek force-pushed the trying2fixSfinaeProblemsByRevertingVcpkgUpgrade branch from 267ef5e to 38d725b Compare September 1, 2023 13:36
@baziorek
Copy link
Member Author

baziorek commented Sep 1, 2023

In the last commit (38d725b) I was trying to fix the test tool_iroha_wsv_diff. To fix this it was necessarily to change iroha_wsv_diff tool. The tool is comparing WSV between Postgres and RocksDB. The test failure was because it contained check:

assert(schema_version == "1#4#0" &&
                   "This version of iroha_wsv_diff can check WSV in RocksDB of version 1.4.0 only");

As I checked now: Iroha 1.5 (https://github.com/hyperledger/iroha/releases/tag/1.5.0) does not change anything in schema after version 1.4. So I've added extra parameter which disables checking of schema version. This argument is used in tests. It is not perfect solution. In future probably the tool needs to be updated to support scheme after changes.

@baziorek baziorek merged commit f2d96e3 into main Sep 1, 2023
25 checks passed
@baziorek baziorek deleted the trying2fixSfinaeProblemsByRevertingVcpkgUpgrade branch September 2, 2023 05:57
@baziorek baziorek mentioned this pull request Sep 2, 2023
21 tasks
baziorek added a commit to dominious1/iroha that referenced this pull request Jan 26, 2024
…yperledger#3852)

* Revert "Update vcpkg from last release"

This reverts commit fdc2c1e.

* Revert "1. Added path to fix building benchmark library on MacOS"

This reverts commit 3b40f22.

* Trying to fix iroha_wsv_diff tool

Signed-off-by: Grzegorz Bazior (Yodiss PSA) <g.bazior@yodiss.pl>

---------

Signed-off-by: Grzegorz Bazior (Yodiss PSA) <g.bazior@yodiss.pl>
Co-authored-by: Grzegorz Bazior (Yodiss PSA) <g.bazior@yodiss.pl>
@nxsaken nxsaken added the iroha1 The legacy version of Iroha. label Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.x 1.5 build CI config-changes Changes in configuration and start up of the Iroha Dev defect The defect was found at the development stage. Did not have an impact on users. iroha1 The legacy version of Iroha. version bump
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants