build: Replace $(AT) with .SILENT #22283

pull dgoncharov wants to merge 1 commits into bitcoin:master from dgoncharov:replace_AT_with_dotsilence changing 2 files +49 −52
  1. dgoncharov commented at 4:37 pm on June 19, 2021: none

    This reduces the amount of syntax noise in the makefiles. Setting V=1 still enables verbose logging.

    The only noticeable difference in behavior is that, unless V=1 is specified, make won’t print its own messages like make: Nothing to be done for ‘all’, make: ‘all’ is up to date, or touch , if -t is specified.

  2. DrahtBot added the label Build system on Jun 19, 2021
  3. hebasto commented at 11:12 pm on June 19, 2021: member
    s/.SILENCE/.SILENT/ in the PR name?
  4. hebasto commented at 11:20 pm on June 19, 2021: member

    On master, for debugging purpose one could easy enable echoing for particular lines in a recipe by removing $(AT).

    How to achieve the same goal with this PR?

  5. DrahtBot commented at 0:23 am on June 20, 2021: member

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #22811 (build: Fix depends build system when working with subtargets by hebasto)
    • #22126 (build: Disable make builtin rules. by dgoncharov)
    • #21995 (build: Make built dependency packages reproducible by hebasto)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  6. dgoncharov renamed this:
    build: Replace $(AT) with .SILENCE.
    build: Replace $(AT) with .SILEN
    on Jun 20, 2021
  7. dgoncharov renamed this:
    build: Replace $(AT) with .SILEN
    build: Replace $(AT) with .SILENT
    on Jun 20, 2021
  8. dgoncharov commented at 1:41 am on June 20, 2021: none

    s/.SILENCE/.SILENT/ in the PR name?

    Indeed.

  9. dgoncharov commented at 2:03 am on June 20, 2021: none

    On master, for debugging purpose one could easy enable echoing for particular lines in a recipe by removing $(AT).

    How to achieve the same goal with this PR?

    You’d need to do the opposite. Which is to add @ in front of those lines which you want to silence. That’d be inconvenient.

    You can do something similar though. If you are willing to modify the makefile, then you can add .SILENT: and make it depend on targets which you want to silence.

    Some of these recipes run a sequence of shell commands bound together through && and ||. You won’t be able to selectively enable or disable echoing parts of these either on master or on this branch. E.g. fetch_file_inner. These recipes would need to be written in a more conventional make style. Without && and ||, but rather on different recipe lines.

  10. dgoncharov commented at 2:27 am on June 20, 2021: none

    On master all the recipe lines have $(AT), on the branch none of them has. This allows to come up with the opposite use case.

    On the branch, for debugging purpose one could easy disable echoing for particular lines in a recipe by adding @. You see, depending on how many lines you need to modify, could be easier on master or this branch.

  11. dongcarl commented at 6:08 pm on June 22, 2021: member
    Can verify that .SILENT exists for both GNU Make and BSD Make (the only two UNIX implementations)
  12. DrahtBot added the label Needs rebase on Jul 18, 2021
  13. dgoncharov commented at 8:52 pm on July 18, 2021: none

    Can verify that .SILENT exists for both GNU Make and BSD Make (the only two UNIX implementations)

    .SILENT exists in other make implementations as well as in posix. .SILENT was even supported by the very first make of Stuart Feldman.

  14. dgoncharov commented at 8:54 pm on July 18, 2021: none
    i merged master onto this branch, resolved the coflict and pushed to my fork. Is anything else needed in regards to the “needs rebase” label?
  15. DrahtBot removed the label Needs rebase on Jul 18, 2021
  16. fanquake commented at 5:19 am on July 19, 2021: member

    Is anything else needed in regards to the “needs rebase” label?

    No, the label is removed by the bot, however you need to get rid of the merge commit here.

  17. MarcoFalke commented at 7:16 am on July 19, 2021: member

    The following should do a proper rebase:

    0git reset --hard 04f41bbca0e462b70b9e5aa4115adf37db43735a
    1git rebase bitcoin-core/master
    
  18. Replace $(AT) with .SILENCE.
    This reduces the amount of syntax noise in the makefiles.
    8494dcae0e
  19. dgoncharov force-pushed on Jul 19, 2021
  20. dgoncharov commented at 12:18 pm on July 19, 2021: none
    Thanks, rebased.
  21. laanwj commented at 8:33 pm on December 17, 2021: member
    Tested ACK 8494dcae0e32716fd7cc7abeacf0a795a1303e6a Depends builds without and with V=1 work after this.
  22. laanwj merged this on Dec 17, 2021
  23. laanwj closed this on Dec 17, 2021

  24. sidhujag referenced this in commit eeacbf6349 on Dec 18, 2021
  25. DrahtBot locked this on Dec 17, 2022

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-12-19 03:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me