Closed Bug 1166065 Opened 6 years ago Closed 6 years ago

All hg hooks should use a #!/usr/bin/env python shebang line

Categories

(Developer Services :: Mercurial: hg.mozilla.org, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: bkero, Assigned: bkero)

Details

Attachments

(3 files, 1 obsolete file)

Some hooks still use #!/usr/bin/python, which should not be the case, especially since it will be technically incorrect once virtualenvs are used.
Attached file MozReview Request: bz://1166065/gps (obsolete) —
/r/8969 - ansible: add copy of peep 2.4.1
/r/8971 - hghooks: use env in shebang
/r/8973 - ansible: create a virtualenv for running hgweb

Pull down these commits:

hg pull -r 118686bdcacfe8e77ad611295493f6be1c3b0543 https://reviewboard-hg.mozilla.org/version-control-tools/
Attachment #8607228 - Flags: review?(bkero)
https://reviewboard.mozilla.org/r/8973/#review7619

::: ansible/roles/hg-web/tasks/main.yml:7
(Diff revision 1)
> +    # Machines are CentOS 6, which has Python 2.6 installed by default.

These are going to cause our docker version of hgweb to fail, since these do not have access to mrepo.

If we do so, it's going to require VPN access to mrepo, which our users (especially external) might not have.

I know it can work for production, but is there a globally accessible upstream version of python 2.7 that we can use here?

::: ansible/roles/hg-web/tasks/main.yml:38
(Diff revision 1)
> +  hg: repo=http://selenic.com/repo/hg dest=/repo/hg/hg revision=612ed41ae359f458dbdc74f03ce9542d3816859d purge=yes force=yes

Where is the 'hg' resource defined? I looked through vct/ansible and couldn't find it
https://reviewboard.mozilla.org/r/8973/#review7625

> These are going to cause our docker version of hgweb to fail, since these do not have access to mrepo.
> 
> If we do so, it's going to require VPN access to mrepo, which our users (especially external) might not have.
> 
> I know it can work for production, but is there a globally accessible upstream version of python 2.7 that we can use here?

We don't use this role in Docker yet. So I'm willing to punt the problem until another day.
Comment on attachment 8607228 [details]
MozReview Request: bz://1166065/gps

/r/8969 - ansible: add copy of peep 2.4.1; r=bkero
/r/8971 - hghooks: use env in shebang; r=bkero
/r/8973 - ansible: create a virtualenv for running hgweb

Pull down these commits:

hg pull -r 197aff05f575249da7ed7cc4fb52b1bd1bfe86cc https://reviewboard-hg.mozilla.org/version-control-tools/
https://reviewboard.mozilla.org/r/8973/#review8139

::: ansible/roles/hg-web/defaults/main.yml:2
(Diff revision 2)
> +venv_hgweb: /repo/hg/venv_hgweb

I know this is in /repo/hg, but since this is a shared NFS dir on hgssh, I worry that this could have weird behavior if run against both hgssh1.dmz.scl3 and hgssh2.dmz.scl3

::: ansible/roles/hg-web/tasks/main.yml:32
(Diff revision 2)
> +  copy: src=requirements.txt dest=/repo/hg/requirements.hgweb.txt mode=0644 owner=root group=root

Why is this in /repo/hg instead of venv_hgweb?
https://reviewboard.mozilla.org/r/8973/#review8159

> I know this is in /repo/hg, but since this is a shared NFS dir on hgssh, I worry that this could have weird behavior if run against both hgssh1.dmz.scl3 and hgssh2.dmz.scl3

This Ansible role only applies to hgweb. So NFS concerns, while valid, don't come into play.
Comment on attachment 8607228 [details]
MozReview Request: bz://1166065/gps

/r/8969 - ansible: add copy of peep 2.4.1; r=bkero
/r/8971 - hghooks: use env in shebang; r=bkero
/r/8973 - ansible: create a virtualenv for running hgweb

Pull down these commits:

hg pull -r 78132edcb332c490ceadfc4b64024d472318a8a4 https://reviewboard-hg.mozilla.org/version-control-tools/
Comment on attachment 8607228 [details]
MozReview Request: bz://1166065/gps

https://reviewboard.mozilla.org/r/8965/#review8265

::: ansible/roles/hg-web/tasks/main.yml:29
(Diff revision 3)
> +           replace='#!{{ venv_hgweb }}/bin/python2.7'

Although idempotent, this is going to be run every time. Thankfully it's quick, although not ideal.

::: ansible/roles/hg-web/tasks/main.yml:44
(Diff revision 3)
> +  command: "{{ venv_hgweb }}/bin/python setup.py install chdir=/repo/hg/hg"

Is it a good idea to be installing this outside of the virtualenv?
Attachment #8607228 - Flags: review?(bkero)
ansible: add copy of peep 2.4.1; r=bkero

File obtained from commit 9a9c535dbcfb1e401ac39c10a057f8597d3f92ec
of erikrose/peep on GitHub.

Adding peep to the repository will enable us to use peep from a secure
source. (Installing peep over the Internet exposes us to MiTM attacks.)
Attachment #8612413 - Flags: review?(bkero)
hghooks: use env in shebang (bug 1166065); r=bkero

We're transitioning to run Mercurial from Python 2.7. If we hardcode
/usr/bin/python, we'll get the system Python, which on CentOS 6 machines
is Python 2.6. Use /usr/bin/env to find the appropriate Python
interpreter for the environment.

Of course, hooks should all be using the "python:" prefix so they are
loaded as Python modules and we don't incur avoidable overhead running
new processes.
Attachment #8612414 - Flags: review?(bkero)
ansible: create a virtualenv for running hgweb; r?bkero

We want to run hgweb from Python 2.7 and from a virtualenv. This commit
introduces basic Ansible support for creating a 2.7-based virtualenv to
run hgweb.

It isn't complete. But it is better than nothing.
Attachment #8612415 - Flags: review?(bkero)
Comment on attachment 8612413 [details]
MozReview Request: ansible: add copy of peep 2.4.1; r=bkero

File didn't change since last time.
Attachment #8612413 - Flags: review?(bkero) → review+
Comment on attachment 8612414 [details]
MozReview Request: hghooks: use env in shebang (bug 1166065); r=bkero

File didn't change since last time.
Attachment #8612414 - Flags: review?(bkero) → review+
Comment on attachment 8612415 [details]
MozReview Request: ansible: create a virtualenv for running hgweb; r?bkero

https://reviewboard.mozilla.org/r/8973/#review8459

Ship It!
Attachment #8612415 - Flags: review?(bkero) → review+
Will deploy this shortly, just so hg.mo is current.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Attachment #8607228 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.