Skip to content

Conversation

@jtdavis777
Copy link
Collaborator

@jtdavis777 jtdavis777 commented Nov 19, 2025

Fixes #8784

Adds (my best attempt at) a minimally invasive implementation of --file-names-only.

Note that this version prints every file the program attempts to write -- during testing I found #8787 where python tries and silently fails to make a file. Other languages may have the same problem.

@jtdavis777
Copy link
Collaborator Author

image

@jtdavis777 jtdavis777 requested a review from aardappel November 25, 2025 12:18
Copy link
Collaborator Author

@jtdavis777 jtdavis777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aardappel thanks for the thorough review! I can go back and not be so heavy handed with my cleanup.

Most of the file manager code that I cleaned up was brought in #7821, which was never used in our repo. I respect the callout that other people ingesting our code base may take advantage of them though.

@aardappel
Copy link
Collaborator

Ok, I'd agree any code that comes from an incomplete/non-functional PR that was merged is probably good to delete if not used in your fix. Still would prefer to minimize renames.

@jtdavis777
Copy link
Collaborator Author

seems like a decent middle ground. I'll compare with the original and roll back to being less aggressive, and will put back the helper functions.

@jtdavis777 jtdavis777 requested a review from aardappel November 27, 2025 01:39
aardappel
aardappel previously approved these changes Dec 3, 2025
@aardappel
Copy link
Collaborator

aardappel commented Dec 3, 2025

looks like the way the fuzzer builds needs updating

CMake Error at CMakeLists.txt:220 (add_executable):
  Cannot find source file:

    /src/flatbuffers/src/file_name_saving_file_manager.cpp```

@aardappel aardappel enabled auto-merge (squash) December 3, 2025 04:33
@aardappel aardappel merged commit a1e125a into google:master Dec 3, 2025
48 checks passed
@fliiiix
Copy link
Contributor

fliiiix commented Dec 3, 2025

@jtdavis777 some docs for this feature would be nice

@jtdavis777
Copy link
Collaborator Author

jtdavis777 commented Dec 3, 2025

@fliiiix totally fair -- see #8821

@jtdavis777 jtdavis777 deleted the feature/file_names_only branch December 3, 2025 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

--file-names-only Option exists but not fully implemented

3 participants