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.

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

Windows: LocateExecFile() fails to find python3 #661

Open
skyler-rs opened this issue Sep 19, 2022 · 2 comments
Open

Windows: LocateExecFile() fails to find python3 #661

skyler-rs opened this issue Sep 19, 2022 · 2 comments
Assignees

Comments

@skyler-rs
Copy link
Contributor

Describe the bug
When adding a new test, it was discovered that system calls to python in the IP Catalog were failing. After some debug, it was observed that

std::filesystem::path python3Path = FileUtils::LocateExecFile("python3");
is always empty in CI tests run on windows.
On first glance the 2 possible causes are:

  1. Python might not be properly installed on windows in our CI
  2. LocateExecFile might not be working on windows

To Reproduce
Due to windows build not building locally for some right now, this is a bit hard to test, I'm unable to build windows locally so I can only see this error in github CI's when the tests are run on windows (note the test in question is currently disabled on windows).
Steps to reproduce the behavior:

  1. Add the following to the bottom of registerBasicGuiCommands()
  auto locate_exec_file = [](void* clientData, Tcl_Interp* interp, int argc,
                             const char* argv[]) -> int {
    if (argc < 1) {
      Tcl_AppendResult(
          interp, qPrintable("Expected Syntax: locate_exec_file <binary_name>"),
          nullptr);
      return TCL_ERROR;
    }

    std::filesystem::path foundPath =
        FOEDAG::FileUtils::LocateExecFile(argv[1]);

    Tcl_AppendResult(interp, foundPath.c_str(), nullptr);
    return TCL_OK;
  };

  session->TclInterp()->registerCmd("locate_exec_file", locate_exec_file,
                                    GlobalSession->MainWindow(), nullptr);
  1. Add #include "Utils/FileUtils.h" to the same file
  2. Build changes
  3. On windows, open foedag and try locate_exec_file python3 in the console

Expected behavior

  • FileUtils::LocateExecFile("python3") or test tcl command locate_exec_file python3 should return a valid path on windows runs
  • tests/TestBatch/test_ip_configure_load.tcl should successfully run in windows CI (note the test currently disables itself on windows run, so remove the windows check inside the test first before verifying)

Enviornment (please complete the following information):

  • OS: Other: Windows GitHub CI and possibly local windows
  • Version: Current main

Additional context
It's currently not known if this is a python install issue or a util function not working in windows issue so both areas need to be investigated
It's possible that python3 isn't the proper name on windows, might test with python and python2 to see if our target binary names are correct

@alaindargelas
Copy link
Contributor

LocateExecFile needs to be augmented for Windows support, it is only working for Linux.
It needs to be augmented to search the Windows PATH and system install directories similarly to Linux.

std::filesystem::path FileUtils::LocateExecFile(
    const std::filesystem::path& path) {
  std::filesystem::path result;
  char* envpath = getenv("PATH");
  char* dir = nullptr;

  for (dir = strtok(envpath, ":"); dir; dir = strtok(NULL, ":")) {
    std::filesystem::path a_path = std::string(dir) / path;
    if (FileUtils::FileExists(a_path)) {
      return a_path;
    }
  }

  for (std::filesystem::path dir :
       {"/usr/bin", "/usr/local/bin", "~/.local/bin", "./"}) {
    std::filesystem::path a_path = dir / path;
    if (FileUtils::FileExists(a_path)) {
      return a_path;
    }
  }

  return result;
}

@ravic-rs
Copy link
Contributor

LocateExecFile needs to be augmented for Windows support, it is only working for Linux. It needs to be augmented to search the Windows PATH and system install directories similarly to Linux.

std::filesystem::path FileUtils::LocateExecFile(
    const std::filesystem::path& path) {
  std::filesystem::path result;
  char* envpath = getenv("PATH");
  char* dir = nullptr;

  for (dir = strtok(envpath, ":"); dir; dir = strtok(NULL, ":")) {
    std::filesystem::path a_path = std::string(dir) / path;
    if (FileUtils::FileExists(a_path)) {
      return a_path;
    }
  }

  for (std::filesystem::path dir :
       {"/usr/bin", "/usr/local/bin", "~/.local/bin", "./"}) {
    std::filesystem::path a_path = dir / path;
    if (FileUtils::FileExists(a_path)) {
      return a_path;
    }
  }

  return result;
}

This makes sense now, I see that we will have to handle python path look up in mac/centos too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants