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: shorten configuration print on stdout #483

Closed
wants to merge 1 commit into from

Conversation

silverwind
Copy link
Contributor

The configure script prints out a prettyprinted dictionary of mostly v8 options, which right now results in a total of 34 lines being output to stdout on a regular build. Warnings like this or this are printed before the dictionary, and are likely being scrolled out of sight almost instantly on terminals with < 30 lines height.

This commit removes the pretty print resulting in single-line output, making warnings more visible, while on the other hand making the options harder to read by a human, so I'm open to suggestions!

Before:

ctrpp not found in WinSDK path--using pre-gen files from tools/msvs/genfiles.
creating  ./icu_config.gypi
{'target_defaults': {'cflags': [],
                     'default_configuration': 'Release',
                     'defines': [],
                     'include_dirs': [],
                     'libraries': []},
 'variables': {'host_arch': 'ia32',
               'icu_small': 'false',
               'node_install_npm': 'true',
               'node_prefix': '',
               'node_shared_http_parser': 'false',
               'node_shared_libuv': 'false',
               'node_shared_openssl': 'false',
               'node_shared_v8': 'false',
               'node_shared_zlib': 'false',
               'node_tag': '',
               'node_use_dtrace': 'false',
               'node_use_etw': 'true',
               'node_use_mdb': 'false',
               'node_use_openssl': 'true',
               'node_use_perfctr': 'true',
               'openssl_no_asm': 0,
               'python': '/bin/python',
               'target_arch': 'ia32',
               'uv_library': 'static_library',
               'v8_enable_gdbjit': 0,
               'v8_enable_i18n_support': 0,
               'v8_no_strict_aliasing': 1,
               'v8_optimized_debug': 0,
               'v8_random_seed': 0,
               'v8_use_snapshot': 'true',
               'want_separate_host_toolset': 0}}
creating  ./config.gypi
creating  ./config.mk

After:

ctrpp not found in WinSDK path--using pre-gen files from tools/msvs/genfiles.
creating  ./icu_config.gypi
{'variables': {'node_shared_http_parser': 'false', 'want_separate_host_toolset': 0, 'icu_small': 'false', 'node_use_mdb': 'false', 'target_arch': 'ia32', 'host_arch': 'ia32', 'v8_optimized_debug': 0, 'node_install_npm': 'true', 'openssl_no_asm': 0, 'v8_no_strict_aliasing': 1, 'node_shared_zlib': 'false', 'node_use_dtrace': 'false', 'python': '/bin/python', 'v8_enable_gdbjit': 0, 'node_use_etw': 'true', 'v8_enable_i18n_support': 0, 'node_use_perfctr': 'true', 'v8_random_seed': 0, 'node_prefix': '', 'node_shared_openssl': 'false', 'node_shared_v8': 'false', 'node_use_openssl': 'true', 'uv_library': 'static_library', 'v8_use_snapshot': 'true', 'node_shared_libuv': 'false', 'node_tag': ''}, 'target_defaults': {'libraries': [], 'default_configuration': 'Release', 'cflags': [], 'include_dirs': [], 'defines': []}}
creating  ./config.gypi
creating  ./config.mk

@bnoordhuis
Copy link
Member

Looks reasonable, although I suppose some terminals might cut off long lines. Maybe you can refine it to something like this?

print('\n'.join(re.findall('.{78}|.+', str(output)))

(I have no idea if that's an idiomatic way of partitioning a string, perhaps @chrisdickinson or @saghul can chime in.)

@silverwind
Copy link
Contributor Author

Personally, I haven't seen a terminal yet that doesn't wrap lines. An alternative would be to print that dictionary before anything else, if that's possible.

@bnoordhuis
Copy link
Member

Personally, I haven't seen a terminal yet that doesn't wrap lines.

It sometimes happens to me when I ssh into old centos boxes, not sure why.

An alternative would be to print that dictionary before anything else, if that's possible.

Not easily, I think. A lot of code would have to be moved around to make that work.

@silverwind
Copy link
Contributor Author

@bnoordhuis updated PR to use textwrap to wrap at 78. Output look like this now:

ctrpp not found in WinSDK path--using pre-gen files from tools/msvs/genfiles.
creating  ./icu_config.gypi
{'variables': {'node_shared_http_parser': 'false',
'want_separate_host_toolset': 0, 'icu_small': 'false', 'node_use_mdb':
'false', 'target_arch': 'ia32', 'host_arch': 'ia32', 'v8_optimized_debug': 0,
'node_install_npm': 'true', 'openssl_no_asm': 0, 'v8_no_strict_aliasing': 1,
'node_shared_zlib': 'false', 'node_use_dtrace': 'false', 'python':
'/bin/python', 'v8_enable_gdbjit': 0, 'node_use_etw': 'true',
'v8_enable_i18n_support': 0, 'node_use_perfctr': 'true', 'v8_random_seed': 0,
'node_prefix': '', 'node_shared_openssl': 'false', 'node_shared_v8': 'false',
'node_use_openssl': 'true', 'uv_library': 'static_library', 'v8_use_snapshot':
'true', 'node_shared_libuv': 'false', 'node_tag': ''}, 'target_defaults':
{'libraries': [], 'default_configuration': 'Release', 'cflags': [],
'include_dirs': [], 'defines': []}}
creating  ./config.gypi
creating  ./config.mk

@bnoordhuis
Copy link
Member

@silverwind LGTM. Can you sign the commit with your real name and maybe add one or two lines to the commit log explaining the motivation for the change? Thanks!

Reduces the previously pretty-printed configuration dictionary to a
single line wrapped at 78 characters. This makes warning messages which
appear before this print more visible to the user.
@silverwind
Copy link
Contributor Author

Updated. Is this alright? I changed user.name before commiting and force-pushing again.

@saghul
Copy link
Member

saghul commented Jan 18, 2015

Since @bnoordhuis red's me, another LGTM. Using textwrap is the idiomatic way IMHO. 👍

bnoordhuis pushed a commit that referenced this pull request Jan 18, 2015
Reduces the previously pretty-printed configuration dictionary to a
single line wrapped at 78 characters. This makes warning messages which
appear before this print more visible to the user.

PR-URL: #483
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
@bnoordhuis
Copy link
Member

Thanks Roman, landed in 1b1cd1c!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants