ArcsinX added inline comments.
================ Comment at: clang-tools-extra/clangd/test/remote-index/pipeline.test:3 +# RUN: clangd-indexer %S/Inputs/Source.cpp > %t.idx +# RUN: %python %S/pipeline_helper.py --input-file-name=%s --project-root=%/S --index-file=%t.idx | FileCheck %s +# REQUIRES: clangd-remote-index ---------------- Is there any reason to use `%/S` in `--project-root`? We support `--project-root` with native slashes (at least `--project-root=%S` works for me on Windows + MS compiler) It looks more reasonable to use `%/S` in paths like this `%/S/Inputs/Source` to avoid mixing slashes on Windows (but it works correctly with mixing slashes anyway) ================ Comment at: clang-tools-extra/clangd/test/remote-index/pipeline.test:3 +# RUN: clangd-indexer %/S/../Inputs/remote-index/Source.cpp > %t.in.dex +# RUN: python %/S/pipeline_helper.py --input-file-name=%s --server-address=0.0.0.0:50051 --test-directory=%/S --index-file=%t.in.dex | FileCheck %s +# REQUIRES: clangd-remote-index ---------------- kbobyrev wrote: > ArcsinX wrote: > > kadircet wrote: > > > use `%python` instead to enable lit substitution, just in case there are > > > weird build bots without python on the PATH or the default one pointing > > > at a wrong version. > > > > > > can we use 127.0.0.1:50051 instead? as starting a server on 0.0.0.0 might > > > be blocked on some bots again. And also I would rather use a random port > > > number (greater than 1024, possibly by letting the python script choose > > > the number, and getting rid of the argument completely), to lower the > > > chances of failing accidentally when 50051 is in use. (it might be > > > because bot is running multiple tests in parallel, or bot itself has a > > > grpc server running on default port for whatever reason). > > > can we use 127.0.0.1:50051 instead? > > I think we should do this, because we also pass this address into > > `--remote-index-address=` clangd parameter, which is illegal. Seems Linux > > and Mac allow to call connect with 0.0.0.0 as a destination address, but > > Windows does not and this test fails. > Thanks for checking! Can you try run this test now and see if it works? It works ok now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90291/new/ https://reviews.llvm.org/D90291 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
