Skip to content

Commit

Permalink
Merge branch 'DOR-401_locales' into 'master'
Browse files Browse the repository at this point in the history
DOR-401 Fix check setting to user locale

Closes DOR-401

See merge request machine-learning/dorado!841
  • Loading branch information
hpendry-ont committed Feb 9, 2024
2 parents 68b6666 + 7ffc860 commit bbe6ad6
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 19 deletions.
11 changes: 2 additions & 9 deletions dorado/main.cpp
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
#include "Version.h"
#include "cli/cli.h"
#include "compat/compat_utils.h"
#include "utils/locale_utils.h"

#include <minimap.h>
#include <spdlog/cfg/env.h>

#include <clocale>
#include <functional>
#include <iostream>
#include <map>
Expand Down Expand Up @@ -55,13 +54,7 @@ int main(int argc, char* argv[]) {
// Load logging settings from environment/command-line.
spdlog::cfg::load_env_levels();

if (auto prev = std::setlocale(LC_ALL, ""); !prev) {
// user has a LANG value set but that locale is not available - override with default C locale
setenv("LANG", "C", true);
} else {
// restore whatever we just changed testing the locale
std::setlocale(LC_ALL, prev);
}
dorado::utils::ensure_user_locale_may_be_set();

const std::map<std::string, entry_ptr> subcommands = {
{"basecaller", &dorado::basecaller},
Expand Down
2 changes: 2 additions & 0 deletions dorado/utils/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ add_library(dorado_utils
fs_utils.h
gpu_monitor.cpp
gpu_monitor.h
locale_utils.cpp
locale_utils.h
log_utils.cpp
log_utils.h
math_utils.h
Expand Down
44 changes: 44 additions & 0 deletions dorado/utils/locale_utils.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
#include "locale_utils.h"

#ifndef _WIN32
#include <stdlib.h>

#include <clocale>
#endif

namespace dorado::utils {

void ensure_user_locale_may_be_set() {
// JIRA issue DOR-234.
// The indicators library tries to set the user preferred locale, so we ensure
// this is possible.
#ifdef _WIN32
// No-op on windows as setlocale with an empty locale will succeed as it
// will set the locale name to the value returned by GetUserDefaultLocaleName
// whose only failure mode is ERROR_INSUFFICIENT_BUFFER, which will not be the case.
#else
// An invalid value for the LANG environment variable (e.g. if it's not present
// on the machine) may cause setting the user preferred locale with
// setlocale(LC_ALL, "") to fail and return null.
// So test for null and provide a valid LANG value if needed
// For fallback behaviour of setlocale(LC_ALL, "") see:
// https://man7.org/linux/man-pages/man3/setlocale.3.html

// Passing nullptr as the locale will cause the current default value to be returned
// for a c++ program this should have been set to "C" during program startup
auto default_locale_to_restore = std::setlocale(LC_NUMERIC, nullptr);

// passing "" as the locale will set and return the user preferred locale (if valid)
auto user_preferred_locale = std::setlocale(LC_ALL, "");
if (!user_preferred_locale) {
// The user preferred locale could not be set.
// Provide a valid value for LANG so that it may be correctly set with ""
setenv("LANG", "C", true);
} else {
// restore the original default locale
std::setlocale(LC_ALL, default_locale_to_restore);
}
#endif
}

} // namespace dorado::utils
7 changes: 7 additions & 0 deletions dorado/utils/locale_utils.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#pragma once

namespace dorado::utils {
// Ensures std::setlocale(LC_ALL, "") will succeed.
// See DOR-234 for details.
void ensure_user_locale_may_be_set();
} // namespace dorado::utils
12 changes: 2 additions & 10 deletions tests/main.cpp
Original file line number Diff line number Diff line change
@@ -1,24 +1,16 @@
#define CATCH_CONFIG_RUNNER

#include "compat/compat_utils.h"
#include "utils/locale_utils.h"
#include "utils/torch_utils.h"

#include <catch2/catch.hpp>
#include <nvtx3/nvtx3.hpp>
#include <torch/utils.h>

#include <clocale>

int main(int argc, char* argv[]) {
// global setup...

if (auto prev = std::setlocale(LC_ALL, ""); !prev) {
// user has a LANG value set but that locale is not available - override with default C locale
setenv("LANG", "C", true);
} else {
// restore whatever we just changed testing the locale
std::setlocale(LC_ALL, prev);
}
dorado::utils::ensure_user_locale_may_be_set();

dorado::utils::make_torch_deterministic();
torch::set_num_threads(1);
Expand Down

0 comments on commit bbe6ad6

Please sign in to comment.