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.

Sign up for GitHub

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

Add t8n full version printing #612

Merged
merged 2 commits into from
Apr 20, 2023
Merged

Add t8n full version printing #612

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
67888ae
Fix -Wstrict-prototypes warning in buildinfo source files
rodiazet Apr 20, 2023
990ef97
Add full version printing to evmone-t8n
rodiazet Apr 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions cmake/cable/CableBuildInfo.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
#
# CHANGELOG
#
# 1.0.1 - 2023-04-19
# - Fix -Wstrict-prototypes warning in buildinfo source files.
#
# 1.0.0 - 2022-02-12

include_guard(GLOBAL)
Expand Down
2 changes: 1 addition & 1 deletion cmake/cable/buildinfo/buildinfo.c.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

#include "buildinfo.h"

const struct buildinfo* @FUNCTION_NAME@()
const struct buildinfo* @FUNCTION_NAME@(void)
Copy link
Member

Choose a reason for hiding this comment

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

Why are these needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

/evmone/build/test/t8n/evmone_t8n/buildinfo.h:29:49: error: a function declaration without a prototype is deprecated in all versions of C [-Werror,-Wstrict-prototypes]
const struct buildinfo* evmone_t8n_get_buildinfo();
                                                ^
                                                 void

Copy link
Member

Choose a reason for hiding this comment

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

Yeah but why wasn't this a problem before?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's enabled by default since clang 15.
https://releases.llvm.org/14.0.0/tools/clang/docs/DiagnosticsReference.html#wstrict-prototypes
https://releases.llvm.org/15.0.0/tools/clang/docs/DiagnosticsReference.html#wstrict-prototypes

Copy link
Member

Choose a reason for hiding this comment

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

Yeah but why is master working?

Copy link
Member

Choose a reason for hiding this comment

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

Are you sure you need (void) in the function definition?

FYI this is related to C23 where the meaning of declaration of int f() changes from int f(...) to int f(void). But I think they overdone the warning.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So for clang 16.0.0 I need but for appleclang 14.0.0.14000029I don't need. I can stop using newer compiler and we don't need to change anything.

Copy link
Collaborator Author

@rodiazet rodiazet Apr 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

but it looks that it's not only my problem ;) https://app.circleci.com/pipelines/github/ethereum/evmone/4335/workflows/d5b6d7cf-9826-45b1-8586-a9366b89a908/jobs/47373
Ci uses clang 15 which enables this warning by default.

Copy link
Collaborator Author

@rodiazet rodiazet Apr 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I think i tested definition too, but will check again on CI

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also the same error.
https://app.circleci.com/pipelines/github/ethereum/evmone/4336/workflows/2a430d17-fbc4-428c-ac0e-80aa38298ef5/jobs/47384

{
static const struct buildinfo buildinfo = {
.project_name = "@PROJECT_NAME@",
Expand Down
2 changes: 1 addition & 1 deletion cmake/cable/buildinfo/buildinfo.h.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ struct buildinfo
const char* build_type;
};

const struct buildinfo* @FUNCTION_NAME@();
const struct buildinfo* @FUNCTION_NAME@(void);
Copy link
Member

Choose a reason for hiding this comment

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

Put changes to cable in separate commit and possibly add a CHANGELOG entry in the cmake file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done


#ifdef __cplusplus
}
Expand Down
11 changes: 4 additions & 7 deletions test/t8n/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,13 @@
# Copyright 2022 The evmone Authors.
# SPDX-License-Identifier: Apache-2.0

include(CableBuildInfo)
cable_add_buildinfo_library(PROJECT_NAME evmone)

hunter_add_package(nlohmann_json)
find_package(nlohmann_json CONFIG REQUIRED)

add_executable(evmone-t8n)
target_link_libraries(evmone-t8n PRIVATE evmone::statetestutils nlohmann_json::nlohmann_json)
target_link_libraries(evmone-t8n PRIVATE evmc::evmc evmone)
target_link_libraries(evmone-t8n PRIVATE evmc::evmc evmone evmone-buildinfo)
target_sources(evmone-t8n PRIVATE t8n.cpp)

# Provide the project version to selected source files.
set_source_files_properties(
t8n.cpp
PROPERTIES COMPILE_DEFINITIONS PROJECT_VERSION="${PROJECT_VERSION}"
)
3 changes: 2 additions & 1 deletion test/t8n/t8n.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "../state/rlp.hpp"
#include "../statetest/statetest.hpp"
#include <evmone/evmone.h>
#include <evmone/version.h>
#include <nlohmann/json.hpp>
#include <filesystem>
#include <fstream>
Expand Down Expand Up @@ -39,7 +40,7 @@ int main(int argc, const char* argv[])

if (arg == "-v")
{
std::cout << "evmone-t8n " PROJECT_VERSION "\n";
std::cout << "evmone-t8n " EVMONE_VERSION "\n";
Copy link
Member

Choose a reason for hiding this comment

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

What is the difference in output?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

PROJECT_VERSION is not defined any longer.
No we print use evmone-t8n 0.10.0-dev+commit.f8f94f51 previously it was evmone-t8n 0.10.0-dev

return 0;
}
if (arg == "--state.fork" && ++i < argc)
Expand Down