Update run_clang_tidy.sh to support bazel targets with '@repository' prefixes

[murray-stripe]

Commit Message:

Support running run_clang_tidy.sh when Envoy is a sub-workspace within a
project (such as used when developing custom filters and nesting Envoy as a
git-submodule). This can be done by setting the compilation DB targets as an
environment variable:

COMP_DB_TARGETS="@envoy//source/... @envoy//test/... @envoy//tools/..."

Additional Description: In order to build from the SRCDIR (not ENVOY_SRCDIR), such as with the envoy-filter-example, need to be able to specify a repository on the targets built by gen_compilation_database.py. Simplest way to enable this is to allow for specifying the comp-db targets.

Risk Level: Low. Default maintains current behavior
Testing: Tested manually in docker container with setup similar to envoy-filter-example repo via:

SRCDIR='/src' COMP_DB_TARGETS="@envoy//source/... @envoy//test/... @envoy//tools/... //filters/..." bash -ex ./envoy/ci/do_ci.sh 'bazel.clang_tidy' '//filters/...'

Docs Changes: n/a
Release Notes: n/a
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Deprecated:]
[Optional API Considerations:]

[phlax]

im not sure but i think the array expansion wont work here - ill check further

[phlax]

arrays arent carried in the env vars so there isnt a way that "${COMP_DB_TARGETS[@]}" would ever be evaluated as one (unless either this script was sourced, or sourced another script that set this)

[murray-stripe]

I set this in an env-var with 4 targets and tested it (as above in PR description) and it worked. Are you saying that if I set and exported the value, you would not expect it to work?

[phlax]

i created a file with the following:

#!/bin/bash

FOO=${COMP_DB_TARGETS:+${COMP_DB_TARGETS[@]}}

echo "$FOO"

and then did the following:

$ export COMP_DB_TARGETS="xyz"
$ ./runfoo
xyz
$ export COMP_DB_TARGETS=("abc" "xyz")
$ ./runfoo

[phlax]

if COMP_DB_TARGETS is an array it wont work

i think you need to do something like

read -ra COMP_DB_TARGETS <<< "$COMP_DB_TARGETS"

if you want to use an array

[phlax]

and then you would also need to set it to a string

[murray-stripe]

I have this currently working in our internal CI system with the following:

COMP_DB_TARGETS="@envoy//source/... @envoy//test/... @envoy//tools/... //filters/..."
export COMP_DB_TARGETS

bash -ex ./envoy/ci/do_ci.sh 'bazel.clang_tidy' '//filters/...'

And then with -x on run_clang_tidy.sh I see:

+ /src/envoy/tools/gen_compilation_database.py --include_headers @envoy//source/... @envoy//test/... @envoy//tools/... //filters/...

[phlax]

its set to a string - my point is $COMP_DB_TARGETS[@] is wrong

[murray-stripe]

ah, gotcha. I'll update to not do any array expansion.

[phlax]

tbh i think you do want array expansion
i added a py script (similar to what is happening in this file)

export COMP_DB_TARGETS="abc xyz"
$ ./runfoo
abc xyz
['./runfoo.py', 'abc xyz']

if you want the args to be separate at least

[phlax]

could you clarify that this works - in my (reduced) testing this would always evaluate to ""

[murray-stripe]

@phlax confirmed that this works in our internal CI system. Currently defining the targets as a string, so probably don't need to use array expansion syntax.

[phlax]

why not just ${COMP_DB_TARGETS} ?

[phlax]

see above comment about separate args

[murray-stripe]

shellcheck complains

but then quoting it will register as an empty-string target in the Python script.

[murray-stripe]

So I suppose the "expand only if it is defined" is the most appropriate thing to do in this case. I think? :-)

[phlax]

i think you need to what i suggested above with read -ra

[murray-stripe]

Legitimate question, what is the difference between that vs what I have currently?

[phlax]

a) works b) shellcheck is happy

[murray-stripe]

Sure, but that's also the case with what I currently have (using :+ syntax). Happy to make the update if it's preferred for stylistic reasons, just trying to understand the rationale for future changes.

[phlax]

its the right way to do it

stepping through

• in shell you set a string
• in bash script split string into array
• in python receive args as separate args

[phlax]

thanks @murray-stripe lgtm

[phlax]

(oops, meant to approve)