(MozBootstrap) Allow users to bootstrap without any interactive prompts.

RESOLVED FIXED in Firefox 40

Status

defect
RESOLVED FIXED
4 years ago
a year ago

People

(Reporter: mrrrgn, Assigned: mrrrgn)

Tracking

unspecified
mozilla40
x86_64
Linux

Firefox Tracking Flags

(firefox40 fixed)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

4 years ago
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

4 years ago
Assignee: nobody → winter2718
(Assignee)

Updated

4 years ago
Blocks: 1151508
(Assignee)

Updated

4 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

4 years ago
Posted patch choices-1-of-2 (obsolete) — Splinter Review
(Assignee)

Comment 2

4 years ago
After this patch, it will also be necessary to allow users to run apt-get with the '-y' option.
(Assignee)

Updated

4 years ago
Attachment #8589108 - Attachment description: choices → choices-1-of-2
(Assignee)

Comment 3

4 years ago
Posted patch choices-2-of-2 (obsolete) — Splinter Review
Attachment #8589130 - Flags: review?(gps)
(Assignee)

Updated

4 years ago
Attachment #8589108 - Flags: review?(gps)
(Assignee)

Updated

4 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

4 years ago
Summary: (MozBootstrap) Allow users to bootstrap with any interactive prompts. → (MozBootstrap) Allow users to bootstrap without any interactive prompts.
Should this be under Release Engineering?
Component: Developer Tools → Tools
Product: Developer Documentation → Release Engineering
QA Contact: hwine
(Assignee)

Comment 5

4 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

4 years ago
*ping*
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
Attachment #8589108 - Attachment is patch: true
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 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

4 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

4 years ago
Posted patch mozboot.total.diff (obsolete) — Splinter Review
Attachment #8589108 - Attachment is obsolete: true
Attachment #8589130 - Attachment is obsolete: true
Attachment #8594607 - Flags: review?(gps)
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

4 years ago
Attachment #8594607 - Flags: checkin+
(Assignee)

Comment 13

4 years ago
Attachment #8594607 - Attachment is obsolete: true
Attachment #8596054 - Flags: checkin+
(Assignee)

Updated

4 years ago
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
(Assignee)

Updated

4 years ago
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Updated

4 years ago
Attachment #8596054 - Flags: checkin+
Comment on attachment 8596054 [details] [diff] [review]
mozboot.total.diff

Review of attachment 8596054 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM!
Attachment #8596054 - Flags: review+
(Assignee)

Updated

4 years ago
Attachment #8596054 - Flags: checkin?(gps)
Attachment #8596054 - Flags: checkin?(gps) → checkin+
https://hg.mozilla.org/mozilla-central/rev/5da7e0d7f32e
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40

Updated

a year ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.