bootstrap.py --help starts with downloading

[bluss]

rustbuild really has a proper --help response, but the first time the user asks for it, it starts working / downloading right away. This is a bad first impression, and a stumbling block for getting to know the new build system.

[hanna-kruppe]

Kinda hard to do anything else, the help message comes from the src/bootstrap crate written in Rust. One could duplicate the usage text in the Python script, but besides being gross and prone to divergence from the ground truth, this increases our reliance on the Python wrapper which ideally should be as minimal as possible (perhaps even some day be replaced with a set of really stupid native platform-specific scripts).

[bluss]

It already parses command line flags before download -- it could catch --help and stop there.

[hanna-kruppe]

It's not hard to catch --help (in fact it already treats that flag specially), but what should it do with that? Outputting the real help message requires knowledge that's not in bootstrap.py, only in bootstrap.rs (for lack of a better name). And simply saying something like

Can't show --help because [...]

doesn't seem like a big improvement to me. It gives you no useful information and the first impression is just as bad.

[bluss]

When I tried to get to know the new build system, my impression was that bootstrap had no --help, because it just started working. So I never found it until much later.

I know, not every misunderstanding is a bug, but it seems like this could be improved? The user just needs to know what they need to do to reach the real --help. Even better of course if that could just be shown, but we can't have it all.

[hanna-kruppe]

You have a point. Perhaps instead of stopping dead, a bootstrap.py invocation that needs to download the nightlies could output something like this before downloading?

NOTE: downloading bootstrap requirements before processing bootstrap ${given command line arguments}

[0xmohit]

OTOH, the script should probably create the build directory in the repo root. Currently it creates the build directory in the location where the script is invoked. For example, if one happens to invoke it from within the src directory: python bootstrap/bootstrap.py ... then build, build/cache, ... are created inside src.

Not sure if there is a reason for making the build directory relative to pwd. The following would seem more desirable:

diff --git a/src/bootstrap/bootstrap.py b/src/bootstrap/bootstrap.py
index 2c2260a..056bd36 100644
--- a/src/bootstrap/bootstrap.py
+++ b/src/bootstrap/bootstrap.py
@@ -370,7 +370,7 @@ def main():
     rb.config_toml = ''
     rb.config_mk = ''
     rb.rust_root = os.path.abspath(os.path.join(__file__, '../../..'))
-    rb.build_dir = os.path.join(os.getcwd(), "build")
+    rb.build_dir = os.path.join(rb.rust_root, "build")
     rb.verbose = args.verbose
     rb.clean = args.clean

[hanna-kruppe]

@0xmohit

I think the intent is to allow out-of-source builds (including multiple independent builds).

Aside: this seems completely unrelated to this issue? If you want to discuss this further please open a new issue.

[xen0n]

FYI the relevant part is non-trivial and not easily refactored out. Maybe printing a note before proceeding to download is the only reasonable thing we can do without too much effort.

[ranma42]

And even after the download, the --help output starts with some cruft (as it checks that the download/build is fine):

    Finished debug [unoptimized + debuginfo] target(s) in 0.0 secs
unknown command: --help
Usage: x.py --help [options] [<args>...]

Options:
    -v, --verbose       use verbose output
...

Would it be an acceptable compromise to keep the output of ./build/bootstrap/debug/bootstrap --help in a text file that can be written back to the console when the ./x.py --help command is run?

That file would mostly be static and it would be easy to add a (tidy?) check that compares it to the output from ./build/bootstrap/debug/bootstrap --help to ensure consistency when the set of options and flags is changed.

[xen0n]

@ranma42 I'm not quite sure but your suggestion may be best implemented with automation, i.e. a script that invokes bootstrap multiple times to obtain all the possible help messages. The script would be run by people developing rustbuild to sync their changes. However that's still quite a bit of complexity and tedious to do not to mention easy to forget, I'm not even sure we should go that far for those impatient people.

@alexcrichton What's your opinion on that, since you authored the whole system?

[xen0n]

I've hacked together such a script:

#!/usr/bin/env python
# -*- coding: utf-8 -*-

from __future__ import unicode_literals, absolute_import

import functools
import os
import subprocess
import sys


def extract_usage(bootstrap_path, args):
    argv = [bootstrap_path, ]
    argv.extend(args)
    proc = subprocess.Popen(argv, stdout=subprocess.PIPE)
    stdout, stderr = proc.communicate()
    return stdout


USAGES_TO_EXTRACT = {
        'cmds': [],
        'build': ['build', '-h'],
        'test': ['test', '-h'],
        'doc': ['doc', '-h'],
        'clean': ['clean', '-h'],
        'dist': ['dist', '-h'],
        }


def main():
    rust_root = os.path.abspath(os.path.join(__file__, '../../..'))
    bootstrap_path = os.path.join(rust_root, 'build/bootstrap/debug/bootstrap')

    usage = functools.partial(extract_usage, bootstrap_path)

    for category, args in USAGES_TO_EXTRACT.items():
        out_filename = 'usage-{}.txt'.format(category)
        out_path = os.path.join(rust_root, 'src/bootstrap', out_filename)
        with open(out_path, 'wb') as fp:
            fp.write(usage(args))


if __name__ == '__main__':
    main()

In case bootstrap is invoked without args or an unknown command, it outputs a line of error, which should be discarded but I didn't include it. Anyway it's still undecided whether rustbuild maintainers are expected to sync usage messages while working on it, so let's discuss.

[ranma42]

@xen0n I had assumed that only the top-level --help option was to be provided without the download (i.e. the generic usage syntax, not the command-specific options). I was suggesting to add a check to the CI infrastructure to ensure the consistency specifically to prevent contributors from forgetting to update it when needed.

[xen0n]

@ranma42 Oh that would be much easier to do! However AFAIK the CI infrastructure only deals with testing rustc itself right now? I don't know how much work it'll take extending rustbuild to self-test as well, as all heavy-lifting are done in Cargo.