Add llvm-echo command.

[ruiu]

Summary

"echo" command is not very portable. Particularly on Windows, we have
various types of echo command that interpret backslashes, double-quotes
and single-quotes differently. On windows, shell does not tokenize
command line arguments but each command does, so the interpretation
varies depending on your crt.

As a result, we observed hard-to-fix errors that happened only on a
limited set of Windows buildbots.

This patch adds a portable "echo" command which always interprets
arguments in the Unix style even on Windows. llvm-echo returns the
same string for a string no matter what platform it is running.
This command should fix the compatibility issue.

[mgorny]

Could you provide a few examples of things that could be solved with this? I agree that we should not be using 'echo' because the command is pretty much unpredictable and each shell interprets it differently. However, I'd rather not reinvent a wheel when we should have e.g. POSIX printf command that's much more sane.

That said, I don't understand why would you need to tokenize arguments to echo.

[rnk]

In D25929#578441, @mgorny wrote:

That said, I don't understand why would you need to tokenize arguments to echo.

If the kernel gives you argv directly, then you don't need to tokenize. If, as on Windows, the kernel gives userspace a quoted string, then different executables may interpret it differently. We've had problems with shell commands like echo '"' where the way CPython quotes the string isn't compatible with the way MSys echo tokenizes it. See rL284768. We could try to standardize on printf everywhere, but it's hard to enforce that for new tests.

[inglorion]

I'm happy you're working on this, because writing short strings in lit tests is a common case, and having it not work reliably is painful. That said, I feel we need some way for lit to automatically do the right thing. If we don't have that, people are going to continue to add tests that will work fine on some platforms and mysteriously write the wrong string on other platforms. Since I expect that most people will continue to write echo for this, is the plan to follow this up with a change that makes lit use llvm-echo when people write echo in lit run recipe?

[ruiu]

It should be easy to run our own "echo" instead of system's echo because I think it's doable just by renaming this executable "echo" as opposed to "llvm-echo". I think the build subdirectory has higher precedence in $PATH in the lit tests, so any command that has the same name as a system command overwrites the existing one. If everybody's fine, I can name this echo instead of llvm-echo.

But if we do that, I think this echo command needs more features. I believe some tests pass command line options such as -n to echo.

[rnk]

I think we should leave it as llvm-echo and use lit substitutions to prefer our version.

[inglorion]

As for adding features, such as the -n option, I would like to make the case for modding those to using printf. echo is non-portable because different implementations have different, and sometimes conflicting features. printf behaves much more predictably and has more features. For this reason, many people advise to use printf instead of echo. Unfortunately, many people's first instinct is to use echo. I think it's a good idea to make sure this works consistently, regardless of what variant of echo may be installed on someone's system or listed first in their PATH.

For the purpose of consistent behavior, I feel it's good enough to have a very simple implementation of echo that just prints its arguments, as your implementation does. I don't see as strong a case for implementing anything more advanced. It would require more work to implement, and we would have to decide if we're going to support -n, and if we're going to support a way to print a literal -n, and whatever we decide would probably surprise someone somewhere. It seems to me we might as well go with the simple implementation and tell people to use printf if they want something more advanced. We have the benefit that if someone tries to use -n to omit the final newline, the simple implementation will very obviously not do what they want, which will hopefully cause them to look for a solution and use printf instead. That, in turn, would then work if someone were to copy-paste the command line from the lit test into their shell, regardless of what variant of echo is on their system. This is much better than what we currently have, where one can use echo to write something that works on one system and misbehaves on another.

[ruiu]

I'm OK with going with this echo and replace all tests using "echo -n" with printf or something.

But printf is not a solution for the specific problem I'm trying to address. Different printf commands on Windows could tokenize command line arguments differently, so it doesn't matter whether it's printf or echo. The problem is that the Windows command line tokenization rule is weird and inconsistent with command to command.

[inglorion]

Right, that's a good point. Of course, we could also implement an llvm-printf command, but that sounds like more effort than just implementing support for -e in llvm-echo. Given that, I'm fine if you want to go that route.

[ruiu]

• Use llvm-echo instead of echo for all lit tests.

[• rafael]

LGTM

[rnk]

utils/lit/lit/TestRunner.py

662 ↗

(On Diff #75824)

lit is supposed to be independent from LLVM. These substitutions are normally done in each project's lit.cfg. If you search for where we add the FileCheck substitution you should be able to do something similar.

utils/llvm-echo/llvm-echo.cpp

19

Let's use llvm::outs() instead of std::cout just for consistency with the rest of LLVM.

34

Why is this TokenizeGNUCommandLine and not TokenizeWindowsCommandLine? Which one works with CPython?

[ruiu]

• Reverted a change to TestRunner.py.
• Use llvm::outs() instead of std::cout.

[ruiu]

utils/llvm-echo/llvm-echo.cpp

34

Doe it make sense? I'm trying to get the exact same result on all platforms instead of trying to get consistent results only on Windows.

[majnemer]

utils/llvm-echo/llvm-echo.cpp

28–35

This should probably be a call to Process::GetArgumentVector, it abstracts away this difference.

[rnk]

I have a better idea, let's fix this in lit. I'll send a patch soon.

utils/llvm-echo/llvm-echo.cpp

34

I think this will do the wrong thing for a command line like:

echo C:\asdf\t.exe

The gnu version will probably eat the backslashes.

[rnk]

I think https://reviews.llvm.org/D25929 makes this unnecessary.

CMakeLists.txt

 
 add_subdirectory(lib)
 
 if( LLVM_INCLUDE_UTILS )
   add_subdirectory(utils/FileCheck)
   add_subdirectory(utils/PerfectShuffle)
   add_subdirectory(utils/count)
   add_subdirectory(utils/not)
+  add_subdirectory(utils/llvm-echo)
   add_subdirectory(utils/llvm-lit)
   add_subdirectory(utils/yaml-bench)
   add_subdirectory(utils/unittest)
 else()
   if ( LLVM_INCLUDE_TESTS )
     message(FATAL_ERROR "Including tests when not building utils will not work.
     Either set LLVM_INCLUDE_UTILS to On, or set LLVM_INCLDE_TESTS to Off.")
   endif()

utils/llvm-echo/CMakeLists.txt

+add_llvm_utility(llvm-echo llvm-echo.cpp)
+
+target_link_libraries(llvm-echo LLVMSupport)

utils/llvm-echo/llvm-echo.cpp

+//===- llvm-echo.cpp - The 'echo' command --------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This is an implementation of a portable "echo" command.
+// It tokenizes command line arguments in the Unix style even on Windows.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/Support/StringSaver.h"
+#include "llvm/Support/raw_ostream.h"
+
+#if LLVM_ON_WIN32
+#include <windows.h>
+#endif
+
+using namespace llvm;
+
+int main(int Argc, const char **Argv) {
+  SmallVector<const char *, 4> Args;
+
+#if LLVM_ON_WIN32
+  const char *Cmdline = GetCommandLineA();
+  BumpPtrAllocator Alloc;
+  StringSaver Saver(Alloc);
+  llvm::cl::TokenizeGNUCommandLine(Cmdline, Saver, Args);
+#else
+  Args.insert(Args.begin(), Argv, Argv + Argc);
+#endif
+
+  for (int I = 1, E = Args.size(); I < E; ++I) {
+    if (I != 1)
+      llvm::outs() << " ";
+    llvm::outs() << Args[I];
+  }
+  llvm::outs() << "\n";
+  return 0;
+}