Closed
Bug 1151834
Opened 9 years ago
Closed 9 years ago
(MozBootstrap) Allow users to bootstrap without any interactive prompts.
Categories
(Firefox Build System :: General, defect)
Tracking
(firefox40 fixed)
RESOLVED
FIXED
mozilla40
Tracking | Status | |
---|---|---|
firefox40 | --- | fixed |
People
(Reporter: mrrrgn, Assigned: mrrrgn)
References
Details
Attachments
(1 file, 3 obsolete files)
18.62 KB,
patch
|
gps
:
review+
|
Details | Diff | Splinter Review |
Right now users are given an interactive prompt, to choose between bootstrapping a system for FF Desktop or FF Android. This makes it more difficult to use bootstrap.py in an automated fashion. To remedy this, users should also have the option to pass in their choice as a script argument.
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → winter2718
Assignee | ||
Updated•9 years ago
|
Summary: (MozBootstrap) Allow users to choose a bootstrap environment without an interactive prompt. → (MozBootstrap) Allow users to choose a bootstrap application without an interactive prompt.
Assignee | ||
Comment 1•9 years ago
|
||
Assignee | ||
Comment 2•9 years ago
|
||
After this patch, it will also be necessary to allow users to run apt-get with the '-y' option.
Assignee | ||
Updated•9 years ago
|
Attachment #8589108 -
Attachment description: choices → choices-1-of-2
Assignee | ||
Comment 3•9 years ago
|
||
Attachment #8589130 -
Flags: review?(gps)
Assignee | ||
Updated•9 years ago
|
Attachment #8589108 -
Flags: review?(gps)
Assignee | ||
Updated•9 years ago
|
Summary: (MozBootstrap) Allow users to choose a bootstrap application without an interactive prompt. → (MozBootstrap) Allow users to bootstrap with any interactive prompts.
Assignee | ||
Updated•9 years ago
|
Summary: (MozBootstrap) Allow users to bootstrap with any interactive prompts. → (MozBootstrap) Allow users to bootstrap without any interactive prompts.
Comment 4•9 years ago
|
||
Should this be under Release Engineering?
Component: Developer Tools → Tools
Product: Developer Documentation → Release Engineering
QA Contact: hwine
Assignee | ||
Comment 5•9 years ago
|
||
Release Engineering works: it's a little tricky, just because I had thought of bootstrap as being a developer tool first and foremost.
Assignee | ||
Comment 6•9 years ago
|
||
*ping*
Comment 7•9 years ago
|
||
This should be tracked as part of the Firefox build system. Sorry for not reviewing - you hit me up for review just as I was about to go to Montreal for PyCon and a Mercurial Sprint. I'm back now. Will make my way through email and review backlog today.
Component: Tools → Build Config
Product: Release Engineering → Core
QA Contact: hwine
Updated•9 years ago
|
Attachment #8589108 -
Attachment is patch: true
Comment 8•9 years ago
|
||
Comment on attachment 8589108 [details] [diff] [review] choices-1-of-2 Review of attachment 8589108 [details] [diff] [review]: ----------------------------------------------------------------- This patch has CRLF characters. Please normalize line endings to LF before committing. Please change your editor on Windows to save LF endings. r+ on the feature. r- on the implementation. So f+. ::: python/mozboot/bin/bootstrap.py @@ +131,5 @@ > + help='The type of the repository. This defines how we fetch file ' > + 'content. Like --repo, you should not need to set this.') > + parser.add_option('--application-choice', dest='application_choice', type=int, > + help='Pass in an application choice (instead of using the ' > + 'default interactive prompt)') Passing integers as CLI arguments is opaque and not very use friendly. Please rewrite this so the CLI takes strings like "desktop" and "android" for the argument.
Attachment #8589108 -
Flags: review?(gps) → feedback+
Comment 9•9 years ago
|
||
Comment on attachment 8589130 [details] [diff] [review] choices-2-of-2 Review of attachment 8589130 [details] [diff] [review]: ----------------------------------------------------------------- This is fine except for the bikeshed. Next review should be a pretty straightforward rubber stamp r+. ::: python/mozboot/bin/bootstrap.py @@ +136,5 @@ > parser.add_option('--application-choice', dest='application_choice', type=int, > help='Pass in an application choice (instead of using the ' > 'default interactive prompt)') > + parser.add_option('--assume-yes', dest='assume_yes', action='store_true', > + help='Answer yes to any (Y/n) interactive prompts') Let's make this --no-interactive, since that's what the feature does.
Attachment #8589130 -
Flags: review?(gps) → feedback+
Assignee | ||
Comment 10•9 years ago
|
||
(In reply to Gregory Szorc [:gps] from comment #8) > Comment on attachment 8589108 [details] [diff] [review] > choices-1-of-2 > > Review of attachment 8589108 [details] [diff] [review]: > ----------------------------------------------------------------- > > This patch has CRLF characters. Please normalize line endings to LF before > committing. Please change your editor on Windows to save LF endings. > > r+ on the feature. r- on the implementation. So f+. > > ::: python/mozboot/bin/bootstrap.py > @@ +131,5 @@ > > + help='The type of the repository. This defines how we fetch file ' > > + 'content. Like --repo, you should not need to set this.') > > + parser.add_option('--application-choice', dest='application_choice', type=int, > > + help='Pass in an application choice (instead of using the ' > > + 'default interactive prompt)') > > Passing integers as CLI arguments is opaque and not very use friendly. > Please rewrite this so the CLI takes strings like "desktop" and "android" > for the argument. Oddly, I don't use Windows. Not sure how the strange endings got in there. I'll make sure they are normalized.
Assignee | ||
Comment 11•9 years ago
|
||
Attachment #8589108 -
Attachment is obsolete: true
Attachment #8589130 -
Attachment is obsolete: true
Attachment #8594607 -
Flags: review?(gps)
Comment 12•9 years ago
|
||
Comment on attachment 8594607 [details] [diff] [review] mozboot.total.diff Review of attachment 8594607 [details] [diff] [review]: ----------------------------------------------------------------- Almost... ::: python/mozboot/mozboot/base.py @@ +156,5 @@ > print('Executing as root:', subprocess.list2cmdline(command)) > subprocess.check_call(command, stdin=sys.stdin) > def yum_install(self, *packages): > command = ['yum', 'install'] > + if self.no_interactive is True: "is True" is considered an anti-pattern in Firefox since "if foo" almost always does the same thing. Please nix this here and throughout the patch. ::: python/mozboot/mozboot/bootstrap.py @@ +5,5 @@ > from __future__ import print_function > import platform > import sys > import os.path > +import collections Sadly, collections was introduced in Python 2.7 and this file must be able to run on Python 2.6. That being said, we may want to audit and see what is actually holding up back to Python 2.6. It was originally OS X 10.6. I can make an argument we can drop 10.6 as a *development* platform. But that's for a follow-up bug.
Attachment #8594607 -
Flags: review?(gps) → feedback+
Assignee | ||
Updated•9 years ago
|
Attachment #8594607 -
Flags: checkin+
Assignee | ||
Comment 13•9 years ago
|
||
Attachment #8594607 -
Attachment is obsolete: true
Attachment #8596054 -
Flags: checkin+
Assignee | ||
Updated•9 years ago
|
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•9 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Updated•9 years ago
|
Attachment #8596054 -
Flags: checkin+
Comment 14•9 years ago
|
||
Comment on attachment 8596054 [details] [diff] [review] mozboot.total.diff Review of attachment 8596054 [details] [diff] [review]: ----------------------------------------------------------------- LGTM!
Attachment #8596054 -
Flags: review+
Assignee | ||
Updated•9 years ago
|
Attachment #8596054 -
Flags: checkin?(gps)
Updated•9 years ago
|
Attachment #8596054 -
Flags: checkin?(gps) → checkin+
Comment 16•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/5da7e0d7f32e
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
status-firefox40:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Updated•6 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•