Skip to content
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.

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

build: Add show-% target for multi-line variables and debug info #24283

Closed
wants to merge 2 commits into from

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented Feb 7, 2022

This PR is a continuation of #17087.

Our depends builds system uses a bunch of variables which values consist of some lines, i.e., multi-line variables.

During debugging it is annoying that print-% is unable to handle such variables:

$ cd depends
$ make print-sqlite_set_vars
/bin/sh: 1: Syntax error: Unterminated quoted string
make: *** [Makefile:5: print-sqlite_set_vars] Error 2

This PR adds show-% target which prints multi-line variable value and its origin and flavor as well:

$ cd depends
$ make show-sqlite_set_vars
- name:   sqlite_set_vars
- origin: file
- flavor: simple
- value, printed from the new line:
sqlite_config_opts=--disable-shared --disable-readline --disable-dynamic-extensions --enable-option-checking
sqlite_config_opts_linux=--with-pic
sqlite_config_opts_freebsd=--with-pic
sqlite_config_opts_netbsd=--with-pic
sqlite_config_opts_openbsd=--with-pic

For connoisseurs suggesting make show-qt_set_vars :)


Other useful examples of usage:

$ make show-CC
- name:   CC
- origin: default
- flavor: recursive
- value, printed from the new line:
cc
$ make show-CC CC=clang
- name:   CC
- origin: command line
- flavor: recursive
- value, printed from the new line:
clang
$ CC=clang make show-CC
- name:   CC
- origin: environment
- flavor: recursive
- value, printed from the new line:
clang
$ make show-host
- name:   host
- origin: file
- flavor: simple
- value, printed from the new line:
x86_64-pc-linux-gnu

Please note that it is unwanted to just modify the print-% rule as it is used in scripts.

@laanwj
Copy link
Member

laanwj commented Feb 9, 2022

Concept ACK. I think this kind of debugging functionality is very useful, especially for complex makefiles such as those used in the depends system.

@DrahtBot
Copy link
Contributor

Guix builds

File commit eca694a
(master)
commit 37930cc
(master and this pull)
SHA256SUMS.part 52bb278f472c04ff... ecf6af9fdb2c36c6...
*-aarch64-linux-gnu-debug.tar.gz 1ea67072871fb3e9... b94b8141e2f6b497...
*-aarch64-linux-gnu.tar.gz ab7737ea08cce568... e677b2e06b9f3eba...
*-arm-linux-gnueabihf-debug.tar.gz ae6c042bb3938d92... 720cc6307f3e4a3a...
*-arm-linux-gnueabihf.tar.gz 13c2fd99fa3ff645... ef2dcf56d8ef0f63...
*-arm64-apple-darwin.tar.gz f2af6d80043feda6... df1cd52f10dde100...
*-osx-unsigned.dmg 77200c90800d39ed... 8a58f09f6560a72b...
*-osx-unsigned.tar.gz 9f07da42c43106fb... 985359524a727ad0...
*-osx64.tar.gz 43d3aee5c66221ba... ff727a0f931924a1...
*-powerpc64-linux-gnu-debug.tar.gz 4ba7a36e05279e3b... 3cc3cfa73956fd68...
*-powerpc64-linux-gnu.tar.gz 882fe0c68aad339a... 8c8d14aedfab8046...
*-powerpc64le-linux-gnu-debug.tar.gz d35588fadf892dea... 50a7b91e7881b3e3...
*-powerpc64le-linux-gnu.tar.gz 19808899c534b9b0... aadb8d614eff760f...
*-riscv64-linux-gnu-debug.tar.gz 1693a2d394dccb60... ac140f2b9cd70160...
*-riscv64-linux-gnu.tar.gz 27cdc3197b74b915... 677c26a94aa694ef...
*-x86_64-linux-gnu-debug.tar.gz 0ea261146b90e7dc... 5ce6c765b0f9c23a...
*-x86_64-linux-gnu.tar.gz bca2953d8b7fe08a... 58a391cb8e9c3204...
*.tar.gz 527dddd72b8fb635... b0484d3379dd40b8...
guix_build.log 066fdaf5e045c1dc... 54a5b2cbea1be9d4...
guix_build.log.diff 7fcffb58297acb76...

@laanwj
Copy link
Member

laanwj commented Apr 5, 2022

Any reason for keeping this in draft status?

@DrahtBot
Copy link
Contributor

DrahtBot commented Apr 12, 2022

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #25964 (build: fix mingw miniupnpc cflags by fanquake)
  • #21778 (build: LLVM 14 & LLD based macOS toolchain by fanquake)

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.

@jonatack
Copy link
Member

Concept ACK

@hebasto hebasto changed the title build: Add print-all-% target for multi-line variables and debug info build: Add show-% target for multi-line variables and debug info Apr 16, 2022
@hebasto
Copy link
Member Author

hebasto commented Apr 16, 2022

@laanwj

Any reason for keeping this in draft status?

The wrong output here:

$ make show-sqlite_set_vars
- name:   sqlite_set_vars
- origin: file
- flavor: recursive
- value, printed from the new line:
zeromq_config_opts=--disable-shared --disable-readline --disable-dynamic-extensions --enable-option-checking
zeromq_config_opts_linux=--with-pic
zeromq_config_opts_freebsd=--with-pic
zeromq_config_opts_netbsd=--with-pic
zeromq_config_opts_openbsd=--with-pic

@hebasto hebasto force-pushed the 220207-print branch 3 times, most recently from 584b2bc to f323829 Compare April 16, 2022 18:59
@hebasto hebasto marked this pull request as ready for review April 16, 2022 19:04
@hebasto
Copy link
Member Author

hebasto commented Apr 16, 2022

Any reason for keeping this in draft status?

Ready for reviewing now.

@laanwj

Thank you for your reminding.

@hebasto
Copy link
Member Author

hebasto commented Apr 16, 2022

The PR description has been updated.

@jarolrod
Copy link
Member

concept ack

@@ -6,7 +6,7 @@ $(package)_sha256_hash=12edc0df75bf9abd7f82f821795bcee50f42cb2e5f76a6a281b857327
$(package)_build_subdir=build_unix
$(package)_patches=clang_cxx_11.patch

define $(package)_set_vars
define $(package)_set_vars :=
Copy link
Member

Choose a reason for hiding this comment

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

Maybe a dumb question, but what does the added := do?

Copy link
Member Author

Choose a reason for hiding this comment

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

It makes $(package)_set_vars a simply expanded variable which value is defined just after parsing.

@hebasto
Copy link
Member Author

hebasto commented Jul 20, 2022

Rebased f323829 -> 2d290d8 (pr24283.03 -> pr24283.04).

@hebasto
Copy link
Member Author

hebasto commented Jul 27, 2022

Rebased 2d290d8 -> 6c18d85 (pr24283.04 -> pr24283.05) due to the conflict with #25697.

hebasto added 2 commits August 4, 2022 10:13
This change guarantees that variable values are correct at any time.

-BEGIN VERIFY SCRIPT-
s() { sed --in-place --expression="s/$1$/$2/g" $(git grep --files-with-matches "$1" "$3"); }
s 'define $(package)_set_vars' 'define $(package)_set_vars :=' './depends'
-END VERIFY SCRIPT-
@hebasto
Copy link
Member Author

hebasto commented Aug 4, 2022

Rebased 6c18d85 -> 9d568cc (pr24283.05 -> pr24283.06) due to the conflict with #25633.

@achow101
Copy link
Member

Are you still working on this?

@hebasto
Copy link
Member Author

hebasto commented Oct 12, 2022

Are you still working on this?

I am. However, closing due to the lack of interest from other developers.

@hebasto hebasto closed this Oct 12, 2022
@bitcoin bitcoin locked and limited conversation to collaborators Oct 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants