]> git.lyx.org Git - features.git/commitdiff
Fix bug #8537: LyX creates the environment variable LC_ALL
authorJean-Marc Lasgouttes <lasgouttes@lyx.org>
Tue, 5 Feb 2013 13:19:33 +0000 (14:19 +0100)
committerJean-Marc Lasgouttes <lasgouttes@lyx.org>
Fri, 15 Feb 2013 17:23:48 +0000 (18:23 +0100)
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.

configure.ac
development/cmake/ConfigureChecks.cmake
src/support/Messages.cpp
src/support/environment.cpp
src/support/environment.h
status.20x

index 1a872476f587e65cce9fe87828de910af882a1e1..f4df86c588373245e645c21708e7a2c2d5f8713e 100644 (file)
@@ -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
index 289bccddfa5ebde9983ad2e4e68f58654261d679..cb6c4d40e75bea5a64d787e792b80cbdc127b150 100644 (file)
@@ -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)
index e8d93d3a8130e5c405f1cf20ca59fc8c4187a3b0..1693b2ed04755f6b95fc8d833a03e24dece91575 100644 (file)
@@ -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<char_type const *>(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<char_type const *>(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<TranslationCache::iterator, bool> result =
                cache_.insert(make_pair(m, trans));
 
index a364725c21173c137c00960c0ba3ea2cc3bb9270..dfdc2d4867c804d63ff1b4fa2dc65b9bc680ac18 100644 (file)
@@ -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
index 296ebc15fbd09d7dfb3416a7755296d4bc9ab51f..78af2e3574dffed7205c63115e88ef5738c77f02 100644 (file)
 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<std::string> 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
 
index eb23083c133333b9c77e12d1979589c18b32be92..6bf65927b298ebff0483f038203a8ed571478158 100644 (file)
@@ -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