From: Jean-Marc Lasgouttes Date: Tue, 5 Feb 2013 13:19:33 +0000 (+0100) Subject: Fix bug #8537: LyX creates the environment variable LC_ALL X-Git-Tag: 2.0.6~102 X-Git-Url: https://git.lyx.org/gitweb/?a=commitdiff_plain;h=d2ea4aaebb0ec44c1c5b7914110c157077aff63f;p=features.git Fix bug #8537: LyX creates the environment variable LC_ALL The current code is not able to unset an environment variable, only to set it to an empty value. This patch refactors a bit the Message class and uses a new EnvChanger helper class that allows to change temporarily an environment variable and that is able to unset variables if needed. The patch also adds new functions hasEnv and unsetEnv in environment.cpp. Open issues: * there may be systems where unsetenv is not available and putenv("name=") does not do the right thing; * unsetenv may lead to leaks on some platforms. * when using unsetenv, we may need to remove strings from the internal map that setEnv uses. --- diff --git a/configure.ac b/configure.ac index 1a872476f5..f4df86c588 100644 --- a/configure.ac +++ b/configure.ac @@ -186,7 +186,7 @@ AC_TYPE_UID_T LYX_CHECK_DEF(PATH_MAX, limits.h, [int n = PATH_MAX;]) -AC_CHECK_FUNCS(chmod close _close fork getpid _getpid lstat mkfifo open _open pclose _pclose popen _popen readlink strerror) +AC_CHECK_FUNCS(chmod close _close fork getpid _getpid lstat mkfifo open _open pclose _pclose popen _popen readlink strerror unsetenv) # Check the form of mkdir() AC_FUNC_MKDIR AC_FUNC_SELECT_ARGTYPES diff --git a/development/cmake/ConfigureChecks.cmake b/development/cmake/ConfigureChecks.cmake index 289bccddfa..cb6c4d40e7 100644 --- a/development/cmake/ConfigureChecks.cmake +++ b/development/cmake/ConfigureChecks.cmake @@ -56,6 +56,7 @@ check_function_exists(_getpid HAVE__GETPID) check_function_exists(mkdir HAVE_MKDIR) check_function_exists(_mkdir HAVE__MKDIR) check_function_exists(setenv HAVE_SETENV) +check_function_exists(unsetenv HAVE_UNSETENV) check_function_exists(putenv HAVE_PUTENV) check_function_exists(fcntl HAVE_FCNTL) check_function_exists(strerror HAVE_STRERROR) diff --git a/src/support/Messages.cpp b/src/support/Messages.cpp index e8d93d3a81..1693b2ed04 100644 --- a/src/support/Messages.cpp +++ b/src/support/Messages.cpp @@ -133,6 +133,38 @@ bool Messages::available(string const & c) } +namespace { + +// Trivial wrapper around gettext() +docstring const getText(string const & m) +{ + // FIXME: gettext sometimes "forgets" the ucs4_codeset we set + // in init(), which leads to severe message corruption (#7371) + // We set it again here unconditionally. A real fix must be found! + LASSERT(bind_textdomain_codeset(PACKAGE, ucs4_codeset), /**/); + + char const * m_c = m.c_str(); + char const * trans_c = gettext(m_c); + docstring trans; + if (!trans_c) { + LYXERR(Debug::LOCALE, "Undefined result from gettext for `" << m << "'."); + trans = from_ascii(m); + } else if (trans_c == m_c) { + //LYXERR(Debug::LOCALE, "Same as entered returned"); + trans = from_ascii(m); + } else { + //LYXERR(Debug::LOCALE, "We got a translation"); + // m is actually not a char const * but ucs4 data + trans = reinterpret_cast(trans_c); + } + + cleanTranslation(trans); + + return trans; +} + +} + docstring const Messages::get(string const & m) const { @@ -145,16 +177,12 @@ docstring const Messages::get(string const & m) const return it->second; // The string was not found, use gettext to generate it - static string oldLC_ALL; - static string oldLANGUAGE; + docstring trans; if (!lang_.empty()) { - oldLC_ALL = getEnv("LC_ALL"); // This GNU extension overrides any language locale // wrt gettext. LYXERR(Debug::LOCALE, "Setting LANGUAGE to " << lang_); - oldLANGUAGE = getEnv("LANGUAGE"); - if (!setEnv("LANGUAGE", lang_)) - LYXERR(Debug::LOCALE, "\t... failed!"); + EnvChanger language_chg("LANGUAGE", lang_); // However, setting LANGUAGE does nothing when the // locale is "C". Therefore we set the locale to // something that is believed to exist on most @@ -162,52 +190,20 @@ docstring const Messages::get(string const & m) const // load German documents even without having de_DE // installed. LYXERR(Debug::LOCALE, "Setting LC_ALL to en_US"); - if (!setEnv("LC_ALL", "en_US")) - LYXERR(Debug::LOCALE, "\t... failed!"); + EnvChanger lc_all_chg("LC_ALL", "en_US"); #ifdef HAVE_LC_MESSAGES setlocale(LC_MESSAGES, ""); #endif - } - - // FIXME: gettext sometimes "forgets" the ucs4_codeset we set - // in init(), which leads to severe message corruption (#7371) - // We set it again here unconditionally. A real fix must be found! - LASSERT(bind_textdomain_codeset(PACKAGE, ucs4_codeset), /**/); - - char const * m_c = m.c_str(); - char const * trans_c = gettext(m_c); - docstring trans; - if (!trans_c) { - LYXERR(Debug::LOCALE, "Undefined result from gettext for `" << m << "'."); - trans = from_ascii(m); - } else if (trans_c == m_c) { - //LYXERR(Debug::LOCALE, "Same as entered returned"); - trans = from_ascii(m); - } else { - //LYXERR(Debug::LOCALE, "We got a translation"); - // m is actually not a char const * but ucs4 data - trans = reinterpret_cast(trans_c); - } - - cleanTranslation(trans); + trans = getText(m); + } else + trans = getText(m); + - // Reset environment variables as they were. - if (!lang_.empty()) { - // Reset everything as it was. - LYXERR(Debug::LOCALE, "restoring LANGUAGE from " - << getEnv("LANGUAGE") - << " to " << oldLANGUAGE); - if (!setEnv("LANGUAGE", oldLANGUAGE)) - LYXERR(Debug::LOCALE, "\t... failed!"); - LYXERR(Debug::LOCALE, "restoring LC_ALL from " << getEnv("LC_ALL") - << " to " << oldLC_ALL); - if (!setEnv("LC_ALL", oldLC_ALL)) - LYXERR(Debug::LOCALE, "\t... failed!"); #ifdef HAVE_LC_MESSAGES - setlocale(LC_MESSAGES, ""); + setlocale(LC_MESSAGES, ""); #endif - } + // store translation in cache pair result = cache_.insert(make_pair(m, trans)); diff --git a/src/support/environment.cpp b/src/support/environment.cpp index a364725c21..dfdc2d4867 100644 --- a/src/support/environment.cpp +++ b/src/support/environment.cpp @@ -28,10 +28,17 @@ using namespace std; namespace lyx { namespace support { -string const getEnv(string const & envname) + +bool hasEnv(string const & name) +{ + return getenv(name.c_str()); +} + + +string const getEnv(string const & name) { // f.ex. what about error checking? - char const * const ch = getenv(envname.c_str()); + char const * const ch = getenv(name.c_str()); return ch ? to_utf8(from_local8bit(ch)) : string(); } @@ -117,5 +124,20 @@ void prependEnvPath(string const & name, string const & prefix) setEnvPath(name, env_var); } + +bool unsetEnv(string const & name) +{ +#if defined(HAVE_UNSETENV) + // FIXME: does it leak? + return unsetenv(name.c_str()) == 0; +#elif defined(HAVE_PUTENV) + // This is OK with MSVC and MinGW at least. + return putenv((name + "=").c_str()) == 0; +#else +#error No environment-unsetting function has been defined. +#endif +} + + } // namespace support } // namespace lyx diff --git a/src/support/environment.h b/src/support/environment.h index 296ebc15fb..78af2e3574 100644 --- a/src/support/environment.h +++ b/src/support/environment.h @@ -18,8 +18,11 @@ namespace lyx { namespace support { +/// @returns true if the environment variable @c name exists. +bool hasEnv(std::string const & name); + /// @returns the contents of the environment variable @c name encoded in utf8. -std::string const getEnv(std::string const & envname); +std::string const getEnv(std::string const & name); /** @returns the contents of the environment variable @c name, * split into path elements using the OS-dependent separator token @@ -54,6 +57,42 @@ void setEnvPath(std::string const & name, std::vector const & env); */ void prependEnvPath(std::string const & name, std::string const & prefix); +/** Remove the variable @c name from the environment. + * @returns true if the variable was unset successfully. + */ +bool unsetEnv(std::string const & name); + + +/** Utility class to change temporarily an environment variable. The + * variable is reset to its original state when the dummy EnvChanger + * variable is deleted. + */ +class EnvChanger { +public: + /// + EnvChanger(std::string const & name, std::string const & value) + : name_(name), set_(hasEnv(name)), value_(getEnv(name)) + { + setEnv(name, value); + } + /// + ~EnvChanger() + { + if (set_) + setEnv(name_, value_); + else + unsetEnv(name_); + } + +private: + /// the name of the variable + std::string name_; + /// was the variable set? + bool set_; + /// + std::string value_; +}; + } // namespace support } // namespace lyx diff --git a/status.20x b/status.20x index eb23083c13..6bf65927b2 100644 --- a/status.20x +++ b/status.20x @@ -35,6 +35,7 @@ What's new * TEX2LYX IMPROVEMENTS - support for listings with options (bug #8066). + - add new option -m to select needed modules (bug #8393). @@ -131,6 +132,8 @@ What's new - File format viewer & editor combo boxes are correctly initialized (bug 8237). +- Do not create an empty environment variable LC_ALL when launching + external processes (bug 8537). * DOCUMENTATION AND LOCALIZATION