Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid outputting Python files for already generated types #8500

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

akb825
Copy link
Contributor

@akb825 akb825 commented Jan 22, 2025

Outputting Python files for already generated types may overwrite previously generated code in situations where flabuffer files are included, rendering them unusable. It can also lead to undesired extra files when including shared flatbuffer definitions from other namespaces.

Partially reverts changes from #8292
Fixes #8490

@github-actions github-actions bot added c++ codegen Involving generating code from schema python labels Jan 22, 2025
@akb825
Copy link
Contributor Author

akb825 commented Jan 22, 2025

CC @anton-bobukh, I'm not sure what the intention was for adding the empty Python files for generated types, but it causes a pretty major regression. See #8490 for more info.

@akb825
Copy link
Contributor Author

akb825 commented Jan 28, 2025

@dbaileychess would you mind taking a look at this PR? This Python regression has meant that I have been unable to update to recent versions of flatbuffers, and I'd imagine it is the same for others as well. I am open to a different approach if you have another idea, but looking through the code I can't think of a situation where outputting a file for an already generated type would be desirable.

This may overwrite types that have already been generated and can create
unwanted empty files. Fixes google#8490
@akb825
Copy link
Contributor Author

akb825 commented Feb 12, 2025

@dbaileychess Pinging you again as there has been a new release yesterday. This regression has been present since the v24.12.23 release, and has rendered projects that generate for Python largely unusable when flatbuffers with include statements are used. I'm open to suggestions if you feel that this fix isn't appropriate, but I would like to get the regression fixed so it's possible to use newer releases.

@EmixamPP
Copy link

Is there a reason why you removed many files from the tests folder?

@akb825
Copy link
Contributor Author

akb825 commented Feb 12, 2025

@EmixamPP these were added in #8292 which introduced the problem. The cause of the regression is that commit chose to output empty files (apart from comments) for any "already generated" types, which occur when you include another .fbs file. This can lead to both:

  1. Overwriting previously generated files for those types with empty ones.
  2. Adding extra files on the filesystem you don't want, such as including a .fbs file for another library (in a different namespace) where you will import them from a different location.

This commit reverts to the previous behavior of ignoring "already generated" types, and as a result removes these empty files that did not exist previously.

@EmixamPP
Copy link

Thanks for the clarification!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ codegen Involving generating code from schema python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Python] Regression - Generated code gets overwritten with empty contents when included in another .fbs file
2 participants