[WIP] Restructure EXTENSIONS/WINDOWS_EXTENSIONS

[wrowe]

Description:

• The test/ tree was ignoring PPC_SKIP_TARGETS
• The test/ tree did not filter on WINDOWS_EXTENSIONS
• The converse of WINDOWS_EXTENSIONS is a much easier list to maintain.

This select() logic is not working, looking for #bazel help to structure
this correctly.

Risk Level: low
Testing:

Fails on msvc/gcc, trying to work around

ERROR: C:/workspace/envoy/source/extensions/all_extensions.bzl:16:62: Traceback (most recent call last):
        File "C:/workspace/envoy/test/extensions/filters/network/ratelimit/BUILD", line 14
                envoy_extension_cc_test(<4 more arguments>)
        File "C:/workspace/envoy/test/extensions/extensions_build_system.bzl", line 19, in envoy_extension_cc_test
                envoy_all_extensions(<1 more arguments>)
        File "C:/workspace/envoy/source/extensions/all_extensions.bzl", line 16, in envoy_all_extensions
                blacklist.values()
type 'select' has no method values()

Docs Changes: n/a
Release Notes: n/a

[sunjayBhatia]

actually, that error message was from a different attempt, the actual message we want to get past is:

argument of type 'select' is not iterable. in operator only works on lists, tuples, dicts and strings.

it seems we don't properly understand select(), it seems like its return value is not what we expect

[wrowe]

Hello @lizan this corresponds to what we discussed many months ago, refactoring the inclusion/exclusion of tests in a consistent way between Windows, PPC, etc etc. Any guidance you can offer is appreciated.

We should also consider that extension_name itself could be a name list, consisting of the several different extensions each required by a given integration test, but I'm happy to get beyond the current select() confusion first.

[Yannic]

This, unfortunately, isn't compatible with how select() works today.
The call to select() doesn't immediately evaluate to a value, that decision only happens when the "SelectObject" returned by select() is used as a parameter to a rule. bazelbuild/bazel#8419 might provide some more insights to the issue.

[wrowe]

That's our understanding as well. Guidance/workarounds are appreciated, that's why we've shared this non-working PR. It ties into #7903

[stale]

This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

[wrowe]

unstale

[Mythra]

Mentioned in slack, but I think the closest way to restructure this, but still allow it to work is to not inspect the select, but instead structure it so it returns a select. Like so:

def envoy_extension_cc_test(
        name,
        extension_name,
        **kwargs):
    actual_rule = envoy_cc_test(name, **kwargs)
    windows_rule = actual_rule
    ppc_rule = actual_rule
    if extension_name in WINDOWS_SKIP_TARGETS:
        windows_rule = []
    if extension_name in PPC_SKIP_TARGETS:
        ppc_rule = []
​
    return select({
        "//bazel:windows_x86_64": windows_rule,
        "//bazel:linux_ppc": ppc_rule,
        "//conditions:default": actual_rule,
    })

At this point WINDOWS_SKIP_TARGETS/PPC_SKIP_TARGETS could also become arrays instead of dict's. I've tested this compiles fine for me locally, but I haven't confirmed if it creates the proper filter we're looking for on Windows/PPC.

[sunjayBhatia]

just to close the loop on this context, the suggestion above got us close but didn't quite work b/c whenever a macro that generates a rule is invoked, the rule is put into the build graph, rules are side-effect generators

instead, we have simply whittled down the list of extensions that do not compile on Windows (to 4) and are skipping tests that use these extensions on Windows with the tag skip_on_windows and flag to exclude compilation of these tests

[stale]

This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

[wrowe]

Since this went off into the weeds, we'll close this proposal for now, awaiting lizan's vision of how this might be restructured. A workaround for four broken tracer extensions on win32 is sufficient for now.