CC @stanley-cheung

Local run of DOCKERHUB_ORGANIZATION=grpctesting tools/run_tests/task_runner.py -f php linux artifact has also passed.

CC @markdroth @stanley-cheung

Several observations and concerns:

1. I verified that this PR, as is, works for PHP. I am able to build the PHP extension and pass unit test (so that means all the relevant symbols are properly compiled and accessible at run time. I had a previous trouble with this - at one of my earlier attempts, I am able to compile the extension but some symbols are missing at run time. This PR, as is, does not have that problem).

2. So for this PR, here's the end state:

• the existing upb-generated directory will contain all the .upb.c, .upb.h and .upbdefs.h files.
• a new upbdefs-genearted directory will contain all the .upbdefs.c files.
• That means, there will be no renaming of any files. Just moving the .upbdefs.c files. So there is also no need to change any #include references.

3. I am a little unsure how this all works. Here's my understanding, please correct if I am wrong:

• The #include statements only ever reference .upbdefs.h files. So as long as they stay in the same location (i.e. upb-generated directory), there is no need to change any of these references.
• The .upbdefs.c files provide the implementations of all the symbols required from these .upbdefs.h files, and more importantly, these .upbdefs.c files never reference each other, nor by their corresponding .upbdefs.h file. So as long as each language's "extension"/'gems'/nugets/etc compile these upbdefs.c files, that would be enough to provide the symbols at runtime, and it doesn't matter where they live.

4. I have a slight concern on step 3 in this PR. Just to clarify, this PR has 4 steps / commits, as of now:
   1. add another operation to gen_upb_api.sh, to move the .upbdefs.c files from upb-generated to upbdefs-generated. So this step will be executed automatically every time gen_upb_api.sh is run
   2. this is just checking in the moved files from step 1. (so this step will be combined with step 1 in the future).
   3. there's a manual step to fix the BUILD files because of these moved .upbdefs.c files
   4. run generate_projects.sh

• So for step 3 here, does it mean whenever next time there's a change caused by gen_upb_api.sh, that someone will need to remember to fix the BUILD file manually too? That seems a bit unsafe. Can this step be automated somehow too? Coz looks like there will be plenty of churn on these upb generated files in the near future? Or did I misunderstand that, even these changes will be part of the future step 1 too?

5. Overall I also agree that this is a better solution because there is no need for a specific fix for PHP, and there's no need to rename any files. If we can make this as safe as possible for future churn, I agree this should be the way to go.

• So for step 3 here, does it mean whenever next time there's a change caused by gen_upb_api.sh, that someone will need to remember to fix the BUILD file manually too? That seems a bit unsafe. Can this step be automated somehow too? Coz looks like there will be plenty of churn on these upb generated files in the near future? Or did I misunderstand that, even these changes will be part of the future step 1 too?

Note that it's already the case that when the set of files we're generating changes, we need to make the corresponding changes in the BUILD file and then re-run generate_projects.sh. This PR does not make that any worse, as far as I can tell.

• So for step 3 here, does it mean whenever next time there's a change caused by gen_upb_api.sh, that someone will need to remember to fix the BUILD file manually too? That seems a bit unsafe. Can this step be automated somehow too? Coz looks like there will be plenty of churn on these upb generated files in the near future? Or did I misunderstand that, even these changes will be part of the future step 1 too?

Note that it's already the case that when the set of files we're generating changes, we need to make the corresponding changes in the BUILD file and then re-run generate_projects.sh. This PR does not make that any worse, as far as I can tell.

Ah I see. Thanks for clarifying. I was actually looking more into this and was coming to a similar conclusion - that if someone is adding/changing the list of .protos being generated in gen_upb_api.sh, that same someone would likely be looking into changing the BUILD file anyway.

So the slightly new cognitive load is that .upbdefs.c files will now be generated into a different directory upbdefs-generated.

Several observations and concerns:

1. I verified that this PR, as is, works for PHP. I am able to build the PHP extension and pass unit test (so that means all the relevant symbols are properly compiled and accessible at run time. I had a previous trouble with this - at one of my earlier attempts, I am able to compile the extension but some symbols are missing at run time. This PR, as is, does not have that problem).

Thanks for confirming!

2. So for this PR, here's the end state:

• the existing upb-generated directory will contain all the .upb.c, .upb.h and .upbdefs.h files.
• a new upbdefs-genearted directory will contain all the .upbdefs.c files.
• That means, there will be no renaming of any files. Just moving the .upbdefs.c files. So there is also no need to change any #include references.

That's accurate. If the idea of this PR looks good I can improve the comment to make this clearer.

I'd probably also change it to make ".upbdefs.h" files also land in upbdefs-generated (for consistency, after that you'd have one directory for plain upb messages - .upb.h, upb.c; and one directory for upb reflection upbdefs.h and upbdefs.c which is a layout that looks pretty reasonable to me).
That of course would require a bit of extra work (I don't include that part to avoid obscuring the idea of this PR), but should be pretty straightforward.

1. I am a little unsure how this all works. Here's my understanding, please correct if I am wrong:

• The #include statements only ever reference .upbdefs.h files. So as long as they stay in the same location (i.e. upb-generated directory), there is no need to change any of these references.
• The .upbdefs.c files provide the implementations of all the symbols required from these .upbdefs.h files, and more importantly, these .upbdefs.c files never reference each other, nor by their corresponding .upbdefs.h file. So as long as each language's "extension"/'gems'/nugets/etc compile these upbdefs.c files, that would be enough to provide the symbols at runtime, and it doesn't matter where they live.

IMHO the location of .h files is not important at all (of course as long as all the build use the right include paths). .h files are not compilation units (.c files are, .h are just included in them by a preprocessor so their location is completely irrelevant) and the issue we're facing is purely about the resulting object files colliding with each other.

1. I have a slight concern on step 3 in this PR. Just to clarify, this PR has 4 steps / commits, as of now:

   1. add another operation to gen_upb_api.sh, to move the .upbdefs.c files from upb-generated to upbdefs-generated. So this step will be executed automatically every time gen_upb_api.sh is run
   2. this is just checking in the moved files from step 1. (so this step will be combined with step 1 in the future).
   3. there's a manual step to fix the BUILD files because of these moved .upbdefs.c files
   4. run generate_projects.sh

• So for step 3 here, does it mean whenever next time there's a change caused by gen_upb_api.sh, that someone will need to remember to fix the BUILD file manually too? That seems a bit unsafe. Can this step be automated somehow too? Coz looks like there will be plenty of churn on these upb generated files in the near future? Or did I misunderstand that, even these changes will be part of the future step 1 too?

That was explained by Mark. Currently the BUILD file needs to be updated manually (at least before #23122 is in place) so this is really only about knowing that upbdefs.c files will be generated under upbdefs-generated instead of upb-generated, which seems pretty straightforward.

1. Overall I also agree that this is a better solution because there is no need for a specific fix for PHP, and there's no need to rename any files. If we can make this as safe as possible for future churn, I agree this should be the way to go.

I'll check if this trick seems to cause problems in conjunction with planned PR #23122.
We'd also need to check if this would cause any problems with import (I don't think it should, at least not until #23122 , but we'd need to check first).
Other than that, I am not aware of any additional problems this would cause.

@jtattermusch We probably should move this forward. Anything I can help?

superseded by #23835.