bootstrap.py --help starts with downloading #37305
Comments
bluss
added
the
T-bootstrap
|
Kinda hard to do anything else, the help message comes from the |
|
It already parses command line flags before download -- it could catch --help and stop there. |
|
It's not hard to catch
doesn't seem like a big improvement to me. It gives you no useful information and the first impression is just as bad. |
|
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. |
|
You have a point. Perhaps instead of stopping dead, a
|
|
OTOH, the script should probably create the Not sure if there is a reason for making the 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 |
|
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. |
|
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. |
|
And even after the download, the Would it be an acceptable compromise to keep the output of That file would mostly be static and it would be easy to add a (tidy?) check that compares it to the output from |
|
@ranma42 I'm not quite sure but your suggestion may be best implemented with automation, i.e. a script that invokes @alexcrichton What's your opinion on that, since you authored the whole system? |
|
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 |
|
@xen0n I had assumed that only the top-level |
|
@ranma42 Oh that would be much easier to do! However AFAIK the CI infrastructure only deals with testing |
|
I think some non- (Side note: I thought the auto-generated files were already verified in some way, but I could not find any such check; would it make sense to implement this and open a PR for it?) |
|
Unfortunately I don't really see a solution to this. This is to me is just a fact of how rustbuild works, we can't really work around it. While committing help messages would be possible, I'd personally prefer to avoid putting any form of generated file in the repository as then we have to keep it up to date, add CI for it, ... |
|
It doesn't need to be perfect, there's always solutions. Would it be ok to add a provisional message that explains that it must build itself before the real --help is displayed? |
|
I'd personally consider that a non-starter because it'd be a pretty bad user experience of typing |
|
@alexcrichton If build system is initialized then Starting arbitrary long build process on |
|
I basically think that this is "bad UI" no matter what we do. Either:
It seems like it's just a matter of weighing these and there's no clear winner |
|
I don't understand why |
|
So let's say we make |
|
@alexcrichton I believe what he means is just printing out why a build is started when being invoked uninitialized. |
|
To provide anything more than a simple hint needs extracting the generated output somehow, all the associated maintenance costs apply. I think we shouldn't go that far unless majority of people see it annoying. (Being more friendly to drive-by contributors by providing immediate help is another topic, let's not discuss that here.) |
By following whatever recommendations uninitialized
|
|
@petrochenkov that's exactly what I meant by |
|
(Writing the edit as a new message in case you are using email) More concrete, uninitialized |
|
I mean, |
|
@alexcrichton I don't see why a provisional message means that --help-really is needed? Can we please focus on solutions. Example future user experience: |
Sorry if I miscommunicated but this is what I meant by my first option above. A two-step print-the-help-message seems a bad UI just as much as download information to print the help is. It seems to me that newbies would far more often prefer to just get a help message rather than go through a multi-stage process just to get a help message.
I'm sorry if I've miscommunicated as well, but this is precisely what I'm trying to do. @petrochenkov is proposing a two-stage help message where first you get a message saying you have to run something else. You then run that something else and then you get the help message. This is what I intended to communicate many comments back as what I believe is not better than the current situation. I'm also confused by the code block you had as an example. It seems like that's exactly what happens today beyond an informational message that something is being compiled. Do you think that's sufficient to close this issue? |
|
@alexcrichton It's not perfect, but even a simple thing like that is a noticeable improvement in user experience. @rkruppe said:
But I don't agree. If you just tell the user why something is happening and that it's intentional, that's much better. |
|
FWIW I've come around and agree that simply saying that and why it's downloading and building stuff before printing the help message is an improvement. |
|
@petrochenkov do you agree that an informational message on the first build would be sufficient? |
|
@alexcrichton (Regarding #37817, this issue is not an absolute blocker, it's just a problem magnified by rustbuild being a default and |
|
@petrochenkov sorry just trying to clarify. You commented on #37817 with
which I interpreted as "#37305 must be fixed before merging this", as in an absolute blocker. I can certainly implement an information message, and we can leave this open if we feel it's still not fixed. |
|
@alexcrichton |
|
I've added a sample message to #37817 |
|
I've stumbled across this today, and I think this UX is really bad.
As there are already rustbuild related dependencies through cargo vendor (#37524), there isn't much more to mess up here by somehow autogenerating the help text and saving it in a form python can understand. It can be generated every time after rustbuild is built, and CI can simply pass a flag to the python script to verify that the file didn't change, so that a PR always needs to check in the change of the file. Is that a good solution? About the sample message: I think it can be improved if it includes a pointer to some document that doesn't require the internet, like |
|
FWIW, I think that printing an informative message should suffice for rolling out the deprecation process. If you're going to use the build system, you're going to be bootstrapping anyway -- so the main issue is being informed about what's happening. That said, I'd suggest specifically mentioning |
|
[Hopefully, this issue also concerns With Rust 1.16.0 I get the message below. Running |
Mark-Simulacrum
added
the
C-enhancement
… r=petrochenkov Improve --help performance for x.py Since compiling the bootstrap command doesn't require any submodules, we can skip updating submodules when a --help command is passed in. On my machine, this saves 1 minute if the submodules are already downloaded, and 10 minutes if run on a clean repo. This commit also adds a message before compiling/downloading anything when a --help command is passed in, to tell the user WHY --help takes so long to complete. It also points the user to the bootstrap README.md for faster help. Finally, this fixes one warning message that still referenced using make instead of x.py, even though x.py is now the standard way of building rust. Closes rust-lang#37305
… r=petrochenkov Improve --help performance for x.py Since compiling the bootstrap command doesn't require any submodules, we can skip updating submodules when a --help command is passed in. On my machine, this saves 1 minute if the submodules are already downloaded, and 10 minutes if run on a clean repo. This commit also adds a message before compiling/downloading anything when a --help command is passed in, to tell the user WHY --help takes so long to complete. It also points the user to the bootstrap README.md for faster help. Finally, this fixes one warning message that still referenced using make instead of x.py, even though x.py is now the standard way of building rust. Closes rust-lang#37305
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.
The text was updated successfully, but these errors were encountered: