Broken "not" and platform file paths

I’m trying to figure out why a bunch of tests are failing on Windows, and the first thing I see is that the “not” program from utils/not reports that the path given to it is not executable, which is because of forward slashes in the Path object passed around, which nearly every Windows call that takes a path will choke on. The input path appears to come from the cmake LLVM_BINARY_DIR variable through the lit.site.config file entry as part of getting the path to the clang executable.

Before I start hacking on it, are you aware of this, and how do you want to handle it, or is someone working on it?

I see that the PathV1 class is being deprecated, but it appears the Program class depends on it, and PathV2 stuff is way different.

If it weren’t deprecated, I would probably put in a function like “PlatformPath”, which assumes the string the Path object stores is kind of a logical file name (as-is with the forward slashes), and the PlatformPath function is called by any platform function to get the platform version of it.

-John

Good morning, John.

Do you mention "XFAIL-marked" tests? If so, it would be interesting for me too.

I know utils/not has issues. it would be same also on mingw.

ps. I can hardly biuld and check msvc stuff due to scheduled power cut
in Japan. :frowning:

...Takumi

Hi John,

Sorry, I should have looked into it further, but I didn’t want to start hacking yet. The real problem is that the path doesn’t have the .exe suffix, which if checked by adding this hack, seems to work:

Index: utils/not/not.cpp

Hi,

I’m not seeing a response to this. May I check in this hack, or get enough info to do a better one?

Without it, I get about 22 test failures on Windows.

Thanks.

-John

John,

Index: utils/not/not.cpp

--- utils/not/not.cpp (revision 127522)
+++ utils/not/not.cpp (working copy)
@@ -15,6 +15,11 @@
int main(int argc, const char **argv) {
sys::Path Program = sys::Program::FindProgramByName(argv[1]);

+#if defined(_MSC_VER)
+ if (strstr(Program.c_str(), ".exe") == NULL)
+ Program.appendSuffix("exe");
+#endif
+
std::string ErrMsg;
int Result = sys::Program::ExecuteAndWait(Program, argv + 1, 0, 0, 0, 0,
&ErrMsg);

I don't think it would be a right fix.

  - sys::Program::FindProgramByName() should canonicalize a pathname
  - sys::Program::ExecuteAndWait() should accept "x:/path/to/foo"
(without .exe).

Even if this patch were the right fix, I don't understand why this is
MSC-specific. It might be _WIN32 not _MSC_VER.

...Takumi

Hi,

How about this::

Index: lib/Support/Windows/Program.inc