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

remove gettext/libintl dependency #95

Merged
merged 3 commits into from
Apr 14, 2022
Merged

remove gettext/libintl dependency #95

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ebc6521
remove gettext/libintl dependency
spoonincode Oct 22, 2019
e71eeda
remove some left over "Intl" cmake variables
spoonincode Apr 30, 2021
b48749a
Merge remote-tracking branch 'origin/main' into remove_gettext
spoonincode Apr 14, 2022
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
4 changes: 0 additions & 4 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,6 @@ endif()

list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/CMakeModules")

if (UNIX AND APPLE)
list(APPEND CMAKE_PREFIX_PATH "/usr/local/opt/gettext")
endif()

include( GNUInstallDirs )
include( InstallDirectoryPermissions )
include( MASSigning )
Expand Down
4 changes: 2 additions & 2 deletions programs/cleos/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ endif()
set(LOCALEDIR ${CMAKE_INSTALL_PREFIX}/share/locale)
set(LOCALEDOMAIN ${CLI_CLIENT_EXECUTABLE_NAME})

target_include_directories(${CLI_CLIENT_EXECUTABLE_NAME} PUBLIC ${Intl_INCLUDE_DIRS} ${CMAKE_CURRENT_BINARY_DIR} ${CMAKE_CURRENT_SOURCE_DIR})
target_include_directories(${CLI_CLIENT_EXECUTABLE_NAME} PUBLIC ${CMAKE_CURRENT_BINARY_DIR} ${CMAKE_CURRENT_SOURCE_DIR})

target_link_libraries( ${CLI_CLIENT_EXECUTABLE_NAME}
PRIVATE appbase version chain_api_plugin producer_plugin chain_plugin http_plugin eosio_chain fc ${CMAKE_DL_LIBS} ${PLATFORM_SPECIFIC_LIBS} ${Intl_LIBRARIES} )
PRIVATE appbase version chain_api_plugin producer_plugin chain_plugin http_plugin eosio_chain fc ${CMAKE_DL_LIBS} ${PLATFORM_SPECIFIC_LIBS} )


copy_bin( ${CLI_CLIENT_EXECUTABLE_NAME} )
Expand Down
3 changes: 1 addition & 2 deletions programs/cleos/localize.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
#pragma once

#include <libintl.h>
#include <fc/variant.hpp>

namespace eosio { namespace client { namespace localize {
Expand All @@ -13,7 +12,7 @@ namespace eosio { namespace client { namespace localize {
inline auto localized_with_variant( const char* raw_fmt, const fc::variant_object& args) {
if (raw_fmt != nullptr) {
try {
return fc::format_string(::gettext(raw_fmt), args);
return fc::format_string(raw_fmt, args);
} catch (...) {
}
return std::string(raw_fmt);
Expand Down
3 changes: 0 additions & 3 deletions programs/cleos/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2423,9 +2423,6 @@ CLI::callback_t header_opt_callback = [](CLI::results_t res) {
};

int main( int argc, char** argv ) {
setlocale(LC_ALL, "");
bindtextdomain(locale_domain, locale_path);
textdomain(locale_domain);
fc::logger::get(DEFAULT_LOGGER).set_log_level(fc::log_level::debug);
context = eosio::client::http::create_http_context();
wallet_url = default_wallet_url;
Expand Down
2 changes: 0 additions & 2 deletions scripts/eosio_build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,9 +180,7 @@ if [[ $ARCH == "Linux" ]]; then
fi

if [ "$ARCH" == "Darwin" ]; then
# opt/gettext: cleos requires Intl, which requires gettext; it's keg only though and we don't want to force linking: https://github.com/EOSIO/eos/issues/2240#issuecomment-396309884
# EOSIO_INSTALL_DIR/lib/cmake: mongo_db_plugin.cpp:25:10: fatal error: 'bsoncxx/builder/basic/kvp.hpp' file not found
CMAKE_PREFIX_PATHS="/usr/local/opt/gettext;${EOSIO_INSTALL_DIR}"
FILE="${SCRIPT_DIR}/eosio_build_darwin.sh"
export CMAKE=${CMAKE}
fi
Expand Down
1 change: 0 additions & 1 deletion scripts/generate_bottle.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ echo "class Eosio < Formula
option :universal

depends_on \"gmp\"
depends_on \"gettext\"
Copy link
Member

Choose a reason for hiding this comment

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

What about gettext-devel in.cicd/platforms/centos-7.6-unpinned.dockerfile and .cicd/platforms/centos-7.6.dockerfile.

If .cicd is not going to be maintained it should be removed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes absolutely -- anything not used or not tested ought to be removed. .cicd & build scripts fall in that bucket now.

I've been reluctant to PR a change to zap all that since there really isn't a replacement for pinned builds outside the build script. But.. the build scripts are no longer tested or getting attention soooo 🤷 😕

Copy link
Member

Choose a reason for hiding this comment

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

There is some thought that the .cicd will be used by a new cicd system, which is what Matthew Darwin is talking about below. Maybe go ahead and make the those two changes so we have them just in case.

Copy link
Member Author

Choose a reason for hiding this comment

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

imo, there is no viable path for .cicd as is: it's outdated, neglected, overcomplicated, interwoven with b1 infra, and of course currently completely untested. The best course of action appears to be to eliminate it and start anew with something else; using the old files as reference if need be.

Copy link
Member

Choose a reason for hiding this comment

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

I'll buy that.

depends_on \"openssl@1.1\"
depends_on \"libusb\"
depends_on macos: :mojave
Expand Down