stdout content is printed to console again if capture is not enabled

[brainelectronics]

stdout content is printed to the console again if capturing to a file is not enabled

[eli-schwartz]

useful reading: https://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html

Currently, it's difficult to tell from git log what you're actually fixing -- it doesn't mention script output at all. And since "Fix #" is in the summary, not the extended description, github itself is completely unable to detect that it's trying to close or even link another issue at all

[eli-schwartz]

CI failure is relevant:

[0/1] Running external command check_exists (wrapped by meson to set env)
Traceback (most recent call last):
  File "/__w/meson/meson/meson.py", line 29, in <module>
    sys.exit(mesonmain.main())
  File "/__w/meson/meson/mesonbuild/mesonmain.py", line 229, in main
    return run(sys.argv[1:], launcher)
  File "/__w/meson/meson/mesonbuild/mesonmain.py", line 218, in run
    return run_script_command(args[1], args[2:])
  File "/__w/meson/meson/mesonbuild/mesonmain.py", line 166, in run_script_command
    return module.run(script_args)
  File "/__w/meson/meson/mesonbuild/scripts/meson_exe.py", line 114, in run
    return run_exe(exe)
  File "/__w/meson/meson/mesonbuild/scripts/meson_exe.py", line 91, in run_exe
    print(stdout.decode())
AttributeError: 'NoneType' object has no attribute 'decode'
FAILED: meson-check_exists 

[bonzini]

This change was done on purpose, since generate_custom_target uses as_meson_exe_cmdline with verbose set to False. @xclaesse, what do you think?

[brainelectronics]

@xclaesse it's fixed now with our pipeline. I'm not sure about the two failing jobs here.

We use python logger and output the logged content to sys.stdout on the console. After your mentioned change all output got piped into the pickle file. We need that content in our console. Enabling capture solved the issue, but showed another one. We (intentionally) set the output to a never existing file (with capture:false) to force a re-run of that custom target. This target is doing some pre- another one post processing of the binary output, which is always required. That tasks/runs will be skipped if capture is set to true, as there is no change detected after the first run. So second and any further run pass simply with "nothing to do" on ninja.

[TheChangeloggers]

@xclaesse any updates or feedback from your side, would like to see this change merged as well. We also lack the stdout content in our build

[eli-schwartz]

Can you fix the commit message as I mentioned above?

Also, this should not be two commits. Please next time fix the first commit using git commit --amend && git push -f.

Since both commits now exist, you can combine them together by using git rebase -i origin/master and changing the word "pick" to "fixup" in the line for the second commit.

[eli-schwartz]

If if p.returncode != 0 then I think this will now print twice.

[brainelectronics]

@eli-schwartz you assumption is wrong I guess. As return p.returncode is leaving the function right after printing the error the new added steps are never reached

[codecov[bot]]

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@504ae2d). Click here to learn what that means. The diff coverage is n/a.

:exclamation: Current head 64201e7 differs from pull request most recent head b1fcdc2. Consider uploading reports for the commit b1fcdc2 to get more accurate results

@@            Coverage Diff            @@
##             master    #8499   +/-   ##
=========================================
  Coverage          ?   67.03%           
=========================================
  Files             ?      394           
  Lines             ?    85286           
  Branches          ?    17429           
=========================================
  Hits              ?    57172           
  Misses            ?    23428           
  Partials          ?     4686           

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 504ae2d...b1fcdc2. Read the comment docs.

[TheChangeloggers]

@eli-schwartz is your commit requirement now satisfied? I don't get it ...

[bonzini]

is your commit requirement now satisfied

You can remove the merge commit. It will be handled when the pull request is merged.

[xclaesse]

I think it would be better to set exe.verbose to True in the cases we want, it has the advantage of not capturing potentially long output into memory before dumping it all.

[xclaesse]

I think you can add verbose=True here: https://github.com/mesonbuild/meson/blob/master/mesonbuild/interpreter/mesonmain.py#L103. That will make postconf/dist/install scripts output to stdout directly without buffering.

[kevr]

What's the blocker on this one; did code coverage fall short? The codecov report doesn't show a diff at the current time; no base commit is associated. I'd love this; I'm willing to fix-up whatever is necessary to get this in.

[bruchar1]

Can be closed. Fixed by #11368.