opened
Flamefire
username_0
I noticed that in addition to com_github_googleapis_googleapis being missing as a system lib (see #42361) the new typing_extensions_archive wasn't added to the list of valid options.

This simply removes the com_github_googleapis_googleapis, the alternative is to readd it as the new name com_google_googleapis and readd (and potentially update) the system build file but I can't verify this. Maybe  @username_2 may?

It also adds typing_extensions_archive as a valid option.

I also made a script which points out those issues. I'd like to have that added to CI as this acts like a basic sanity check. Could anyone from the TF team guide me on how to do that?
Update list of systemlibs and add script to verify
issue
created
mihaimaruseac
username_1
I like this, thank you.
comment
created
Flamefire
username_0
@username_1 Could you tell me where I could add this so it is run by the CI scripts for PRs?
comment
created
perfinion
username_2
I talked with mihai at yesterday's SIG-Build meeting, we're not really sure about where yet. Lets get this merged first then maybe we can do a github action, the complicated part is most of their kokoro alerting is internal to google so there isnt a way to notify you and I. initially i'll try running a cronjob on my build machine until we figure out alerting
comment
created
mihaimaruseac
username_1
Sorry, came to review GitHub stuff only now. We could try Github Actions at one point in the future, besides what username_2@ said.
comment
created
Flamefire
username_0
I used GHA in the past and it is generally very easy to use. I could add one to run this to this PR or a follow up. It should be pretty trivial (checkout, setup python, run script) but needs maybe some bikeshedding about names and probably someone need to enable GHA for this repo. Also it will only run AFTER it is merged, so testing is a bit nasty. I could test it in my fork and link that in the PR though
IMO running linters and such things like this are perfect for GHA as they are fast and easy to set up. Also that takes off pressure from the other tests
comment
created
mihaimaruseac
username_1
Let's rebase this on master again, that should fix sanity build
comment
created
Flamefire
username_0
Rebased
comment
created
perfinion
username_2
I'm not sure if im mis-reading the patches after the rebase or not?
it looks like "typing_extensions_archive" is removed from syslibs_configure.bzl?
That got added in commit b7871f8a232536fcb6e388bb05a9fd7171ff3a96 so should stay in the valid list
comment
created
Flamefire
username_0
Good catch, thanks! Corrected and rebased to current master
comment