1282293 - configwizard optionalrepo

Status: RESOLVED FIXED

(Developer Services :: Mercurial: configwizard, defect)

Description

[Vlada]

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

Comment 1

[Gregory Szorc [:gps]]

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.

Comment 2

[Gregory Szorc [:gps]]

Attachment: 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/

Comment 3

[Gregory Szorc [:gps]]

Attachment: 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/

Comment 4

[Gregory Szorc [:gps]]

Attachment: 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/

Comment 5

[Gregory Szorc [:gps]]

Attachment: 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 6

[:glob ✱]

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

https://reviewboard.mozilla.org/r/60936/#review57796

Comment 7

[:glob ✱]

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.

Comment 8

[:glob ✱]

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

https://reviewboard.mozilla.org/r/60940/#review57804

Comment 9

[:glob ✱]

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

https://reviewboard.mozilla.org/r/60942/#review57808

Comment 10

[Gregory Szorc [:gps]]

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/

Comment 11

[Gregory Szorc [:gps]]

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 12

[Gregory Szorc [:gps]]

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 13

[Gregory Szorc [:gps]]

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/

Comment 14

[Gregory Szorc [:gps]]

Attachment: 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/

Comment 15

[Gregory Szorc [:gps]]

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 16

[Gregory Szorc [:gps]]

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 17

[Gregory Szorc [:gps]]

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 18

[Gregory Szorc [:gps]]

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 19

[Gregory Szorc [:gps]]

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 20

[Gregory Szorc [:gps]]

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 21

[:glob ✱]

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

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

Comment 22

[Gregory Szorc [:gps]]

Attachment: 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/

Comment 23

[Gregory Szorc [:gps]]

FWIW, RHEL 6 ships Mercurial 1.4 (running from Python 2.6). That's why I care about supporting this configuration.

Comment 24

[:glob ✱]

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

https://reviewboard.mozilla.org/r/61048/#review57886

Comment 25

[:glob ✱]

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

[Pulsebot]

Pushed by [e-mail]:
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