Closed Bug 1282293 Opened 8 years ago Closed 8 years ago

configwizard optionalrepo

Categories

(Developer Services :: Mercurial: configwizard, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: adaml2, Assigned: gps)

Details

Attachments

(6 files)

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
Component: Untriaged → Build Config
Product: Firefox → Core
Component: Build Config → Mercurial: configwizard
Product: Core → Developer Services
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
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)
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/
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/
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+
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)
Comment on attachment 8765684 [details]
configwizard: implement safehasattr (bug 1282293);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/60938/diff/1-2/
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/
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/
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)
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/
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/
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/
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/
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/
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+
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)
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
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
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: