fuzzers: print verbose maketodo output to stderr, not stdout - #2571
Open
ylemiesa57 wants to merge 1 commit into
Open
fuzzers: print verbose maketodo output to stderr, not stdout#2571ylemiesa57 wants to merge 1 commit into
ylemiesa57 wants to merge 1 commit into
Conversation
int_maketodo.py writes its actual output (the list of todo PIPs) to stdout, and pip_loop.mk redirects that stdout straight into todo_all.txt. When --verbose is passed, the diagnostic messages were also going to stdout via 'verbose and print(...)', so they ended up mixed into the same stream and corrupted the todo file. This changes all the verbose diagnostic print() calls to write to stderr instead, leaving stdout with only the actual todo PIP list that the Makefile expects. Ran yapf -i on the file to match the existing formatting. Fixes f4pga#521 Signed-off-by: Yaphet Lemiesa <yaphkl75@mit.edu>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I was looking for a small, well scoped issue to get familiar with the prjxray codebase and #521 seemed like a good one, it's a clear bug with an obvious fix.
What I changed
fuzzers/int_maketodo.pyprints its real output (the list of todo PIPs) to stdout, andpip_loop.mkpipes that stdout straight intotodo_all.txt. The problem is that when you pass--verbose, all the diagnostic messages (verbose and print(...)) were also going to stdout, so they'd get mixed into the same stream and end up in the todo file, corrupting it. That's exactly what the issue describes.The fix just sends all the verbose diagnostic prints to stderr instead, so stdout only ever has the actual todo entries the Makefile expects. I left the final
print(todo)loop (the real output) on stdout since that's supposed to be there. I also ranyapf -ion the file afterward since a few of the print calls needed re-wrapping to match the repo's.style.yapfconfig.Testing
There isn't a dedicated test file for
int_maketodo.pyand the one test file intests/(test_util.py) covers a different module (prjxray.util) and needs the full toolchain deps installed, so I wrote a quick manual check instead: I calledmaketodo()directly with a small fake pipfile/dbfile, once withverbose=Trueand once withverbose=False, and captured stdout/stderr separately.With
verbose=True, stdout only contained the 3 todo entries and all the diagnostic lines ("entries", "Loading ...", "Post db ...", etc) showed up on stderr instead, so redirecting stdout to a file would no longer corrupt it. Withverbose=Falsethe output was unchanged from before my edit, same 3 entries on stdout, nothing on stderr. Also ranpython3 -m py_compileandyapf -don the file to make sure it compiles and matches the repo's formatting style.Let me know if I should tweak anything, still getting familiar with how the fuzzer scripts fit together here.