configwizard optionalrepo

RESOLVED FIXED

Status

Developer Services
Mercurial: configwizard
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: Vlada, Assigned: gps)

Tracking

Trunk

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(6 attachments)

(Reporter)

Description

a year ago
User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Ubuntu Chromium/50.0.2661.102 Chrome/50.0.2661.102 Safari/537.36

Steps to reproduce:

checkout clean firefox (to empty directory) with:
hg clone https://hg.mozilla.org/mozilla-central/ firefox

./mach mercurial-setup



Actual results:

Ensuring https://hg.mozilla.org/hgcustom/version-control-tools is up to date at /home/w/.mozbuild/version-control-tools
requesting all changes
adding changesets
adding manifests
adding file changes
added 4386 changesets with 10524 changes to 1946 files
1419 files updated, 0 files merged, 0 files removed, 0 files unresolved
================================================================================
*** failed to import extension configwizard from /home/w/.mozbuild/version-control-tools/hgext/configwizard: cmd() got an unexpected keyword argument 'optionalrepo'
hg: unknown command 'configwizard'



Expected results:

some flawless setuping :-)

hg --version
Mercurial Distributed SCM (version 2.8.2)

Linux Mint 17.2 ( ubuntu )

2 weeks ago it worked

Updated

a year ago
Component: Untriaged → Build Config
Product: Firefox → Core
(Assignee)

Updated

a year ago
Component: Build Config → Mercurial: configwizard
Product: Core → Developer Services
(Assignee)

Comment 1

a year ago
We rewrote `mach mercurial-setup` to use a Mercurial extension under the hood. Unfortunately, it appears to be using an API not compatible with Mercurial 2.8.2 (which is rather ancient and has known security vulnerabilities, so you should upgrade).

We should error more gracefully. So I'll see about fixing this.
Assignee: nobody → gps
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
(Assignee)

Comment 2

a year ago
Created attachment 8765683 [details]
configwizard: support loading extension on older Mercurial versions (bug 1282293);

See the inline comment for why we do this.

With this change, the extension now loads with Mercurial versions
down to 2.0.

Review commit: https://reviewboard.mozilla.org/r/60936/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/60936/
Attachment #8765683 - Flags: review?(glob)
Attachment #8765684 - Flags: review?(glob)
Attachment #8765685 - Flags: review?(glob)
Attachment #8765686 - Flags: review?(glob)
(Assignee)

Comment 3

a year ago
Created attachment 8765684 [details]
configwizard: implement safehasattr (bug 1282293);

util.safehasattr was added in hg 2.0. Replacing instances during
the extension load path with getattr() allow us to load the extension
on 1.9.

Review commit: https://reviewboard.mozilla.org/r/60938/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/60938/
(Assignee)

Comment 4

a year ago
Created attachment 8765685 [details]
configwizard: handle missing cmdutil.command (bug 1282293);

cmdutil.command was introduced in hg 1.9. Working around it being
missing allows the extension to load on hg 1.7 and 1.8.

It might run on earlier versions. However, I'm unable to run hg
1.6 and earlier on my machine when building hg from source. I
think compatibility down to 1.7 should be fine. So unless someone
complains, I'm inclined to leave this as our cut-off.

Review commit: https://reviewboard.mozilla.org/r/60940/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/60940/
(Assignee)

Comment 5

a year ago
Created attachment 8765686 [details]
configwizard: update error message when running too old Mercurial (bug 1282293);

By adding the version numbers we let people know what they are running
and what is required.

By adding the URL to install instructions, we hopefully increase the
changes of people installing a modern Mercurial.

Review commit: https://reviewboard.mozilla.org/r/60942/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/60942/
Comment on attachment 8765683 [details]
configwizard: support loading extension on older Mercurial versions (bug 1282293);

https://reviewboard.mozilla.org/r/60936/#review57796
Attachment #8765683 - Flags: review?(glob) → review+
Comment on attachment 8765684 [details]
configwizard: implement safehasattr (bug 1282293);

https://reviewboard.mozilla.org/r/60938/#review57802

::: hgext/configwizard/__init__.py:841
(Diff revision 1)
>              fh.write('%s\n' % line)
>  
>  
>  def uisetup(ui):
>      # hasconfig() was added in 3.7. Backport until we require 3.7.
> -    if util.safehasattr(ui, 'hasconfig'):
> +    if getattr(ui, 'hasconfig', None):

i'm concerned that this behaves slightly differently from safehasattr.

given safehasattr is trivial, i think it would be better to monkey patch it in if required.
Attachment #8765684 - Flags: review?(glob)
Attachment #8765685 - Flags: review?(glob) → review+
Comment on attachment 8765685 [details]
configwizard: handle missing cmdutil.command (bug 1282293);

https://reviewboard.mozilla.org/r/60940/#review57804
Comment on attachment 8765686 [details]
configwizard: update error message when running too old Mercurial (bug 1282293);

https://reviewboard.mozilla.org/r/60942/#review57808
Attachment #8765686 - Flags: review?(glob) → review+
(Assignee)

Comment 10

a year ago
Comment on attachment 8765683 [details]
configwizard: support loading extension on older Mercurial versions (bug 1282293);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/60936/diff/1-2/
Attachment #8765684 - Attachment description: configwizard: avoid util.safehasattr during extension load (bug 1282293); → configwizard: implement safehasattr (bug 1282293);
Attachment #8765684 - Flags: review?(glob)
(Assignee)

Comment 11

a year ago
Comment on attachment 8765684 [details]
configwizard: implement safehasattr (bug 1282293);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/60938/diff/1-2/
(Assignee)

Comment 12

a year ago
Comment on attachment 8765685 [details]
configwizard: handle missing cmdutil.command (bug 1282293);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/60940/diff/1-2/
(Assignee)

Comment 13

a year ago
Comment on attachment 8765686 [details]
configwizard: update error message when running too old Mercurial (bug 1282293);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/60942/diff/1-2/
(Assignee)

Comment 14

a year ago
Created attachment 8765936 [details]
configwizard: avoid set literals syntax (bug 1282293);

set literals are added in Python 2.7. If running a Mercurial using
Python 2.6 (as you'll find on some Linux distributions), this will
cause things to break.

Stop using set literals to allow the extension to load.

Review commit: https://reviewboard.mozilla.org/r/61048/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/61048/
Attachment #8765936 - Flags: review?(glob)
(Assignee)

Comment 15

a year ago
Comment on attachment 8765685 [details]
configwizard: handle missing cmdutil.command (bug 1282293);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/60940/diff/2-3/
(Assignee)

Comment 16

a year ago
Comment on attachment 8765686 [details]
configwizard: update error message when running too old Mercurial (bug 1282293);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/60942/diff/2-3/
(Assignee)

Comment 17

a year ago
Comment on attachment 8765936 [details]
configwizard: avoid set literals syntax (bug 1282293);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61048/diff/1-2/
(Assignee)

Comment 18

a year ago
Comment on attachment 8765685 [details]
configwizard: handle missing cmdutil.command (bug 1282293);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/60940/diff/3-4/
(Assignee)

Comment 19

a year ago
Comment on attachment 8765686 [details]
configwizard: update error message when running too old Mercurial (bug 1282293);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/60942/diff/3-4/
(Assignee)

Comment 20

a year ago
Comment on attachment 8765936 [details]
configwizard: avoid set literals syntax (bug 1282293);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61048/diff/2-3/
Comment on attachment 8765684 [details]
configwizard: implement safehasattr (bug 1282293);

https://reviewboard.mozilla.org/r/60938/#review57878
Attachment #8765684 - Flags: review?(glob) → review+
(Assignee)

Comment 22

a year ago
Created attachment 8765952 [details]
configwizard: work around ui.__class__ not working on hg <1.7 (bug 1282293);

See the inline comment for why we do this.

This enables the extension to load on hg 1.4, which is what RHEL 6
ships and is hopefully the oldest version we'll encounter in the wild.
It may work on earlier versions but I haven't tested.

Review commit: https://reviewboard.mozilla.org/r/61062/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/61062/
Attachment #8765952 - Flags: review?(glob)
(Assignee)

Comment 23

a year ago
FWIW, RHEL 6 ships Mercurial 1.4 (running from Python 2.6). That's why I care about supporting this configuration.
Comment on attachment 8765936 [details]
configwizard: avoid set literals syntax (bug 1282293);

https://reviewboard.mozilla.org/r/61048/#review57886
Attachment #8765936 - Flags: review?(glob) → review+
Attachment #8765952 - Flags: review?(glob) → review+
Comment on attachment 8765952 [details]
configwizard: work around ui.__class__ not working on hg <1.7 (bug 1282293);

https://reviewboard.mozilla.org/r/61062/#review57888

Comment 26

a year ago
Pushed by gszorc@mozilla.com:
https://hg.mozilla.org/hgcustom/version-control-tools/rev/fb246026bf31
configwizard: support loading extension on older Mercurial versions ; r=glob
https://hg.mozilla.org/hgcustom/version-control-tools/rev/bb92dc1c2758
configwizard: implement safehasattr ; r=glob
https://hg.mozilla.org/hgcustom/version-control-tools/rev/74963e9d9bc2
configwizard: handle missing cmdutil.command ; r=glob
https://hg.mozilla.org/hgcustom/version-control-tools/rev/f628fbf497b0
configwizard: update error message when running too old Mercurial ; r=glob
https://hg.mozilla.org/hgcustom/version-control-tools/rev/dbc20d5f9b31
configwizard: avoid set literals syntax ; r=glob
https://hg.mozilla.org/hgcustom/version-control-tools/rev/377a4607351e
configwizard: work around ui.__class__ not working on hg <1.7 ; r=glob
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.