Changeset View
Standalone View
clang-tools-extra/clangd/test/remote-index/pipeline_helper.py
- This file was added.
| #!/usr/bin/env python | |||||
| # | |||||
| #===- pipeline_helper.py - Remote Index pipeline Helper *- python -------*--===# | |||||
| # | |||||
| # Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. | |||||
| # See https://llvm.org/LICENSE.txt for license information. | |||||
| # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | |||||
| # | |||||
| #===------------------------------------------------------------------------===# | |||||
| import argparse | |||||
| import os | |||||
| import subprocess | |||||
| from socket import socket | |||||
| import sys | |||||
| import time | |||||
| import signal | |||||
| def main(): | |||||
kadircet: nit: maybe perform the magic after arg parsing ? | |||||
| parser = argparse.ArgumentParser() | |||||
| parser.add_argument('--input-file-name', required=True) | |||||
| parser.add_argument('--project-root', required=True) | |||||
this still binds the socket to all interfaces, can we specify localhost or 127.0.0.1 explicitly for the interface address. kadircet: this still binds the socket to all interfaces, can we specify `localhost` or `127.0.0.1`… | |||||
| parser.add_argument('--index-file', required=True) | |||||
| args = parser.parse_args() | |||||
can we rename test-directory to project-root and pass it directly from lit ? this will also contain \\ when run on windows. kadircet: can we rename `test-directory` to `project-root` and pass it directly from lit ?
this will… | |||||
| # Grab an available port. | |||||
| with socket() as s: | |||||
| s.bind(('localhost', 0)) | |||||
| server_address = 'localhost:' + str(s.getsockname()[1]) | |||||
| index_server_process = subprocess.Popen([ | |||||
do we really need the abspath conversion here ? is it to get rid of .. ? i am uneasy about this as the call is likely to introduce \\. kadircet: do we really need the abspath conversion here ? is it to get rid of `..` ? i am uneasy about… | |||||
| 'clangd-index-server', '--server-address=' + server_address, | |||||
| args.index_file, args.project_root | |||||
| ], | |||||
| stderr=subprocess.PIPE) | |||||
can we instead wait until server prints Server listening on ... ? kadircet: can we instead wait until server prints `Server listening on ...` ? | |||||
nit: formatting looks off, is this really what yapf offers? kadircet: nit: formatting looks off, is this really what yapf offers? | |||||
Yep, I'm formatting with it :( kbobyrev: Yep, I'm formatting with it :( | |||||
| # Wait for the server to warm-up. | |||||
| time.sleep(4) | |||||
i would rather scan through all lines and look for Server listening on $server_addres. We should be able to either hit the end of stream (i.e. server encounters a failure and shuts down) or find the log message we are looking for. just to make test less reliant on log ordering. kadircet: i would rather scan through all lines and look for `Server listening on $server_addres`. We… | |||||
Unfortunately, it's not as easy :( Then I would have to set a time to wait for the server to warm up and read lines then only. The problem here is that the server runs, doesn't shut down but also doesn't print the message. We'll hang the buildbots then. But I'll go with the timeout option. I don't think it's better than checking the first line as the initialize one but still. kbobyrev: Unfortunately, it's not as easy :( Then I would have to set a time to wait for the server to… | |||||
Not Done ReplyInline Actions
Why? isn't the readline() on stderr blocking ? If not how was this code working without sleep before?
If the argument above is correct, this was the case already, i.e. stderr.readline() would block until a line is available or server had shutdown. To overcome this problem I would suggest having a separate thread that would kill the server if it is still running after N seconds. kadircet: > Unfortunately, it's not as easy :( Then I would have to set a time to wait for the server to… | |||||
| found_init_message = False | |||||
| for line in index_server_process.stderr: | |||||
kadircetUnsubmitted Not Done ReplyInline Actionsso if we think .readline() is blocking but this is not, can't we just keep calling readline instead? kadircet: so if we think `.readline()` is blocking but this is not, can't we just keep calling readline… | |||||
| if b'Server listening' in line: | |||||
| found_init_message = True | |||||
| break | |||||
| if not found_init_message: | |||||
| sys.exit(1) | |||||
can we just wait until clangd process shuts down ? kadircet: can we just wait until `clangd` process shuts down ? | |||||
| in_file = open(args.input_file_name) | |||||
| clangd_process = subprocess.Popen([ | |||||
can we rather use signal.SIGXXX here instead of 9 ? Also rather than kill, SIGINT might be more applicable. https://docs.python.org/3/library/signal.html#signal.SIGKILL claims sigkill is not available on windows. kadircet: can we rather use `signal.SIGXXX` here instead of `9` ?
Also rather than kill, SIGINT might be… | |||||
The problem with SIGINT is that it needs to wait for the hot reload sync to finish :) I'll add a patch to modify the frequency via CLI to avoid long test terminaiton. kbobyrev: The problem with `SIGINT` is that it needs to wait for the hot reload sync to finish :) I'll… | |||||
| 'clangd', '--remote-index-address=' + server_address, | |||||
| '--project-root=' + args.project_root, '--lit-test', '--sync' | |||||
| ], | |||||
| stdin=in_file) | |||||
| clangd_process.wait() | |||||
| os.kill(index_server_process.pid, signal.SIGINT) | |||||
| if __name__ == '__main__': | |||||
| main() | |||||
nit: maybe perform the magic after arg parsing ?