Skip to content

Commit

Permalink
Fix #10322 - Bump CLI11 from 2.3.2 to 2.4.2
Browse files Browse the repository at this point in the history
Turns out the issue is that I rely on CLI11's (circa 2.3.2) interpretation of argc/argv, which for linux reads `/proc/self/cmdline` separated by null terminators.

https://github.com/CLIUtils/CLI11/blob/985a19f3860be0cba87354336f6588494b20111c/include/CLI/impl/Argv_inl.hpp#L141

When you run under rosetta, you end up with a duplicated program path, looks like rosetta is trying to remove itself from the command line but duplicating it. So wheen you call `energyplus --help` what cmdline has is `energyplus energyplus --help`.

This bumps to CLI11 to 2.4.2, and moves back to using the actual main argc/argv instead of relying on the `static_args` that reads `/proc/self/cmdline`

Note: I suppose I could also have achieved the same thing without bumping to 2.4.2, but might as well do it since there were numeros unicode related fixes as well
  • Loading branch information
jmarrec committed Jul 8, 2024
1 parent 1033bdc commit fd3337f
Show file tree
Hide file tree
Showing 4 changed files with 1,126 additions and 405 deletions.
5 changes: 3 additions & 2 deletions src/ConvertInputFormat/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -436,10 +436,11 @@ std::vector<fs::path> parse_input_paths(fs::path const &inputFilePath)
return input_paths;
}

int main(/** [[maybe_unused]] int argc, [[maybe_unused]] const char *argv[] */)
int main(int argc, char **argv)
{

CLI::App app{"ConvertInputFormat"};
argv = app.ensure_utf8(argv);
app.description("Run input file conversion tool");
app.set_version_flag("-v,--version", EnergyPlus::DataStringGlobals::VerString);

Expand Down Expand Up @@ -502,7 +503,7 @@ Select one (case insensitive):

// We are not modifying argc/argv, so we defer to CLI11 to find the argc/argv instead. It'll use GetCommandLineW & CommandLineToArgvW on windows
try {
app.parse();
app.parse(argc, argv);
} catch (const CLI::ParseError &e) {
return app.exit(e);
}
Expand Down
9 changes: 5 additions & 4 deletions src/EnergyPlus/main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,16 @@
#include <EnergyPlus/fenv_missing.h>
#endif

int main()
int main(int argc, char **argv)
{
#ifdef DEBUG_ARITHM_GCC_OR_CLANG
feenableexcept(FE_DIVBYZERO | FE_INVALID | FE_OVERFLOW);
#endif

int const argc = CLI::argc();
const char *const *argv = CLI::argv(); // This is going to use CommandLineToArgvW on Windows and **narrow** from wchar_t to char

#ifdef _WIN32
const std::vector<std::string> args = CLI::detail::compute_win32_argv();
#else
const std::vector<std::string> args(argv, std::next(argv, static_cast<std::ptrdiff_t>(argc)));
#endif
return EnergyPlusPgm(args);
}
Loading

5 comments on commit fd3337f

@nrel-bot
Copy link

Choose a reason for hiding this comment

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

10322_docker_cmdline (jmarrec) - Win64-Windows-10-VisualStudio-16: OK (2844 of 2844 tests passed, 0 test warnings)

Build Badge Test Badge

@nrel-bot-3
Copy link

Choose a reason for hiding this comment

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

10322_docker_cmdline (jmarrec) - x86_64-MacOS-10.18-clang-15.0.0: OK (3634 of 3636 tests passed, 0 test warnings)

Messages:\n

  • 2 tests had: Table big diffs.
  • 2 tests had: Table string diffs.

Failures:\n

regression Test Summary

  • Passed: 789
  • Failed: 2

Build Badge Test Badge

@nrel-bot-2c
Copy link

Choose a reason for hiding this comment

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

10322_docker_cmdline (jmarrec) - x86_64-Linux-Ubuntu-22.04-gcc-11.4: OK (3675 of 3677 tests passed, 0 test warnings)

Messages:\n

  • 2 tests had: Table big diffs.
  • 2 tests had: Table string diffs.

Failures:\n

regression Test Summary

  • Passed: 809
  • Failed: 2

Build Badge Test Badge

@nrel-bot-2
Copy link

Choose a reason for hiding this comment

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

10322_docker_cmdline (jmarrec) - x86_64-Linux-Ubuntu-22.04-gcc-11.4-IntegrationCoverage-Debug: OK (795 of 795 tests passed, 0 test warnings)

Build Badge Test Badge Coverage Badge

@nrel-bot-2b
Copy link

Choose a reason for hiding this comment

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

10322_docker_cmdline (jmarrec) - x86_64-Linux-Ubuntu-22.04-gcc-11.4-UnitTestsCoverage-Debug: OK (2052 of 2052 tests passed, 0 test warnings)

Build Badge Test Badge Coverage Badge

Please sign in to comment.