Skip to content

Cleaning build log #17529

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 7 commits into
base: compatible
Choose a base branch
from
Open

Cleaning build log #17529

wants to merge 7 commits into from

Conversation

dannywillems
Copy link
Member

When building Mina, the log is full of unecessary logs. We could ignore most of it by redirecting to /dev/null.
The reviewer can check commit by commit.

@dannywillems dannywillems requested review from a team as code owners July 14, 2025 14:30
@dannywillems dannywillems force-pushed the dw/removing-more-logs-on-stdout branch from 7010e29 to b3d21b1 Compare July 14, 2025 14:35
@dannywillems dannywillems changed the title Clearing build log Cleaning build log Jul 14, 2025
@dannywillems dannywillems force-pushed the dw/removing-more-logs-on-stdout branch from aa1220d to 1c15a4e Compare July 14, 2025 14:59
@dannywillems
Copy link
Member Author

!ci-build-me

@dannywillems
Copy link
Member Author

!ci-bypass-changelog

@dannywillems dannywillems mentioned this pull request Jul 14, 2025
Open
querolita
querolita approved these changes Jul 14, 2025
View reviewed changes
Copy link
Member

@querolita querolita left a comment

Choose a reason for hiding this comment

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

Thanks!

@dannywillems dannywillems force-pushed the dw/removing-more-logs-on-stdout branch from 1c15a4e to 1297bea Compare July 14, 2025 16:34
@dannywillems
Copy link
Member Author

!ci-build-me

@dannywillems dannywillems requested a review from Copilot July 14, 2025 17:20
Copilot
Copilot AI reviewed Jul 14, 2025
View reviewed changes
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR suppresses extraneous build and tool output by redirecting stdout and using ignore-stdout in Dune actions and Makefiles.

  • Added ignore-stdout wrappers in Dune files to silence routine build steps.
  • Redirected Makefile commands to /dev/null to hide Go and Cap’n Proto output.
  • Adjusted test and JS build flags, and silenced Cargo builds in OCaml stubs.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/libp2p_ipc/dune Wrapped make action in ignore-stdout
src/libp2p_ipc/Makefile Redirected Go and Cap’n Proto commands to null
src/lib/mina_block/tests/dune Silenced curl and gzip in test download step
src/lib/integers_stubs_js/test/dune Removed +nat.js and --source-map flags
src/lib/crypto/kimchi_bindings/stubs/dune Wrapped Cargo builds/runs in ignore-stdout
src/app/cli/src/init/dune Ignored stdout for ocaml-crunch action
Comments suppressed due to low confidence (2)

src/lib/mina_block/tests/dune:18

  • There appears to be one extra closing parenthesis here, which could lead to a syntax error in the Dune file. Please verify the parentheses are balanced.
    (run gzip -d %{target}.gz)))))

src/lib/integers_stubs_js/test/dune:5

  • [nitpick] The removal of +nat.js and --source-map flags changes the JS test build behavior. Please confirm this is intentional and update any related test expectations or documentation.
  (flags --pretty))

glyh
glyh requested changes Jul 15, 2025
View reviewed changes
Copy link
Member

@glyh glyh left a comment

Choose a reason for hiding this comment

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

Not sure on build logs for rust & go, rather see more output logs.

Or is there a way to make it hidden by default, but show all logs when requested?

@dannywillems
Copy link
Member Author

dannywillems commented Jul 15, 2025 •
edited
Loading

Not sure on build logs for rust & go, rather see more output logs.

Or is there a way to make it hidden by default, but show all logs when requested?

See commit message fdc1fa7. It will show the list of commands being run, and in case of an error, the user can see which command fails, and run it.

@glyh
Copy link
Member

glyh commented Jul 16, 2025

See commit message fdc1fa7. It will show the list of commands being run, and in case of an error, the user can see which command fails, and run it.

What about message redirected to /dev/null? I'd rather we throw them in a log directory under git root.

@glyh glyh added cleanup enhancement Not big enough to be a feature, but is a smaller improvement labels Jul 16, 2025
@dannywillems
Copy link
Member Author

See commit message fdc1fa7. It will show the list of commands being run, and in case of an error, the user can see which command fails, and run it.

What about message redirected to /dev/null? I'd rather we throw them in a log directory under git root.

It is uncommon to redirect to a log file in Makefile. When building, you are only interested in errors.

@dannywillems dannywillems force-pushed the dw/removing-more-logs-on-stdout branch from 1297bea to 64427ac Compare July 16, 2025 14:28
@dannywillems
Copy link
Member Author

!ci-build-me

@glyh
Copy link
Member

glyh commented Jul 17, 2025

It is uncommon to redirect to a log file in Makefile. When building, you are only interested in errors.

So are you implying errors will still show up?

@dannywillems
Copy link
Member Author

It is uncommon to redirect to a log file in Makefile. When building, you are only interested in errors.

So are you implying errors will still show up?

Yes, we are not redirecting the stderr (no 2> used).

@dannywillems
Build: clean stdout when running dune build
be348fb 
This ignores the stdout of ocaml-crunch. This removes the following logs given
on the stdout while building:
```
Generating assets.ml
Skipping generation of .mli
```
@dannywillems
Build: removing Rust build info on stdout
2fd7ecd 
This drastically reduces the logging information we get when building Mina.
The user can always use `dune build --verbose` for a verbose build.
@dannywillems
libp2p_ipc: ignore stdout
eaf004e
@dannywillems
libp2p_ipc: redirect to dev/null outputs of Go commands
a8bf82c
@dannywillems
Mina_blocks/test: ignore stdout of curl command
65c1050
@dannywillems
mina_blocks/test: silent curl output
3711f84
@dannywillems
integers_stubs_js: removing +nat.js and --source-map
55c1321 
+nat.js is only useful when using the Num library.
It is useless (for this test), and source-map parameter is also useless for
running the test.

This removes some warning like:
```
warning: overriding primitive "set_to_zero_nat"
  old: +nat.js:50
  new: +nat.js:50
```

Source: https://ocsigen.org/js_of_ocaml/4.0.0/manual/overview
@dannywillems dannywillems force-pushed the dw/removing-more-logs-on-stdout branch from 64427ac to 55c1321 Compare July 20, 2025 14:53
@dannywillems
Copy link
Member Author

!ci-build-me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

Copilot code review Copilot Copilot left review comments

@glyh glyh glyh approved these changes

@querolita querolita querolita approved these changes

Assignees
No one assigned
Labels
cleanup enhancement Not big enough to be a feature, but is a smaller improvement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dannywillems @glyh @querolita