895995 - py27_mercurial DMG installs to a different location than RPM

Status: RESOLVED FIXED

(Infrastructure & Operations :: RelOps: Puppet, task, P2)

Description

[Dustin J. Mitchell [:dustin] (he/him)]

This is kind of a mess.

On Darwin, it installs to /tools/mercurial, with a link from /usr/local/bin
On CentOS, it installs to /tools/python27_mercurial, with a link from /usr/local/bin
On Ubuntu, /tools/python27_mercurial contains a symlink *to* /usr/bin/hg

Will changing the location on Darwin break anything?

Comment 1

[Justin Wood (:Callek)]

(In reply to Dustin J. Mitchell [:dustin] from comment #0)
> This is kind of a mess.
> 
> On Darwin, it installs to /tools/mercurial, with a link from /usr/local/bin

Eww

> On CentOS, it installs to /tools/python27_mercurial, with a link from
> /usr/local/bin

This is the ideal (imo)

> On Ubuntu, /tools/python27_mercurial contains a symlink *to* /usr/bin/hg

This was done after chatting with :rail (who created the package for me), because the way it was setup on deb atm would have made using /tools very hard (or so he said). I was unable to figure out how to build the DEB at the time.

> Will changing the location on Darwin break anything?

I don't *think* so, shall we give it a shot and see how it plays in staging? We might need to update the PATH stuff a bit, but I'd rather try to have a standard here :-)

Comment 2

[Dustin J. Mitchell [:dustin] (he/him)]

Attachment: bug895995.patch

Simple change, plus puppet code to deploy it.  We should probably run some tests on this in staging before deploying?

Comment 3

[Dustin J. Mitchell [:dustin] (he/him)]

And, I should add, I've already rebuilt this for 10.6, 10.7, and 10.8.

Comment 4

[Justin Wood (:Callek)]

Comment on attachment 781168 [details] [diff] [review]
bug895995.patch

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

This is a r+ pending staging it, I can't commit to testing it this week for sure, but possibly someone else can. (if it fails, I still think we should do it, but workaround with a symlink until we fix other related codes paths.)

Comment 5

[Dustin J. Mitchell [:dustin] (he/him)]

Deploying this will not remove the old location (/tools/mercurial), so I think it's safe.  We'll only see problems, if any, on newly-imaged hosts.

Callek, what do you think, and what does your time look like this week?

Comment 6

[Justin Wood (:Callek)]

(In reply to Dustin J. Mitchell [:dustin] from comment #5)
> Deploying this will not remove the old location (/tools/mercurial), so I
> think it's safe.  We'll only see problems, if any, on newly-imaged hosts.

Ooo Good point. Personally I'd love to see the old-location removed entirely. Both to keep "newly imaged" matching "previously imaged" and to avoid surprise mismatches.

That said I don't *think* we need to block initial landing on that. I'm not sure if I'd be able to do the staging this week, lets revisit "me or not" later tomorrow. And if I can't I can try and find someone who can.

Comment 7

[Dustin J. Mitchell [:dustin] (he/him)]

It sounds like coop is involved in both this bug and in re-imaging all of the 10.7 talos to use PuppetAgain, so I'm assigning to him to handle scheduling of the deployment.

Comment 8

[Chris Cooper [:coop] (he/him)]

I will try to get this into staging tomorrow.

Comment 9

[Chris Cooper [:coop] (he/him)]

Pinned bld-lion-r5-043 to my own puppet env, and it's currently running jobs fine using the new mercurial location.

Comment 10

[Dustin J. Mitchell [:dustin] (he/him)]

Great!  So we should probably land this fairly soon, and *then* schedule the phased re-imaging of the 10.7 talos hosts?

Comment 11

[Chris Cooper [:coop] (he/him)]

Comment on attachment 781168 [details] [diff] [review]
bug895995.patch

https://hg.mozilla.org/build/puppet/rev/9c5f03635ec0

Comment 12

[Chris Cooper [:coop] (he/him)]

(In reply to Dustin J. Mitchell [:dustin] from comment #10)
> Great!  So we should probably land this fairly soon, and *then* schedule the
> phased re-imaging of the 10.7 talos hosts?

Yes, let's get that handled in bug 891880.

Comment 13

[Dustin J. Mitchell [:dustin] (he/him)]

This fixed the install location, but some of the paths are still wrong.

Now:
On Darwin, it installs to /tools/python27_mercurial, with a link from /usr/local/bin
  $mercurial is "/tools/mercurial/bin/hg" (WRONG)
On CentOS, it installs to /tools/python27_mercurial, with a link from /usr/local/bin
  $mercurial is "/tools/python27-mercurial/bin/hg" (correct)
On Ubuntu, /usr/bin/hg a symlink to /usr/bin/hg (WRONG)
  $mercurial is "/usr/bin/hg" (correct)

Comment 14

[Dustin J. Mitchell [:dustin] (he/him)]

Attachment: bug895995-p2.patch

Comment 15

[Rail Aliiev [:rail]]

Comment on attachment 797881 [details] [diff] [review]
bug895995-p2.patch

I believe you also need to add force => true, because /usr/bin/hg may be a part of installed original mercurial deb package.

Comment 16

[Dustin J. Mitchell [:dustin] (he/him)]

It gets worse.

On Darwin, it installs to /tools/python27_mercurial, with a link from /usr/local/bin
                                         ^
  $mercurial is "/tools/mercurial/bin/hg" (WRONG)
On CentOS, it installs to /tools/python27-mercurial, with a link from /usr/local/bin
                                         ^
  $mercurial is "/tools/python27-mercurial/bin/hg" (correct)
On Ubuntu, /usr/bin/hg a symlink to /usr/bin/hg (WRONG)
  $mercurial is "/usr/bin/hg" (correct)

So, I think we need a new version on Darwin.  I'll land attachment 797881 [details] [diff] [review] with an underscore for now.

Comment 17

[Dustin J. Mitchell [:dustin] (he/him)]

Actually, I'll be on a plane in 2h, so I'll land this Monday.

Comment 18

[Dustin J. Mitchell [:dustin] (he/him)]

Attachment: underscore.patch

ugh.

Comment 19

[[e-mail] (:bhearsum)]

Comment on attachment 800933 [details] [diff] [review]
underscore.patch

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

Hyphen, underscore - what's the difference! They're both just walk a straight line.

Comment 20

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 800933 [details] [diff] [review]
underscore.patch

checked-in almost exactly one year ago (Sept 6, 2013) in http://hg.mozilla.org/build/puppet/rev/e6995f8e0db1

Comment 21

[Dustin J. Mitchell [:dustin] (he/him)]

Now:
On Darwin, it installs to /tools/python27_mercurial, with a link from /usr/local/bin
  $mercurial is "/tools/python27_mercurial/bin/hg"
On CentOS, it installs to /tools/python27-mercurial, with a link from /usr/local/bin
  $mercurial is "/tools/python27-mercurial/bin/hg"
On Ubuntu, /tools/python27-mercurial/bin/hg is a symlink *to* /usr/bin/hg
  $mercurial is "/usr/bin/hg"

So, this is pretty close, but still with the underscores on OS X and the hyphens on CentOS and Ubuntu.  I hate the Ubuntu situation, too, but it's OK for now (hg 2.5.4 across the board).  When we decide to upgrade, we should fix that by actually building for Ubuntu -- I'll add a comment to that effect.

Comment 22

[Dustin J. Mitchell [:dustin] (he/him)]

I rebuild this on a builder (Lion) and installed it on snow:

[[e-mail] ~]# ls -d /tools/python27*
/tools/python27                 /tools/python27_mercurial
[[e-mail] ~]# installer -target / -package /Volumes/mozilla-python27-mercurial-2.5.4-3/python27-mercurial-2.5.4-3.pkg
installer: Package name is root
installer: Upgrading at base path /
installer: The upgrade was successful.
[[e-mail] ~]# ls -d /tools/python27*
/tools/python27                 /tools/python27-mercurial
[[e-mail] ~]# /usr/local/bin/hg --version
Mercurial Distributed SCM (version 2.5.4)
(see http://mercurial.selenic.com for more information)

Copyright (C) 2005-2012 Matt Mackall and others
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
[[e-mail] ~]# ls -l /usr/local/bin/hg
lrwxr-xr-x  1 root  wheel  32 Sep  3 08:35 /usr/local/bin/hg -> /tools/python27-mercurial/bin/hg

everything seems to be in place.

Comment 23

[Dustin J. Mitchell [:dustin] (he/him)]

Attachment: bug895995-p3.patch

I installed and tested (by running hg --version) this on:
 t-snow-r4-0112.test.releng.scl3.mozilla.com
 bld-lion-r5-022.try.releng.scl3.mozilla.com
 talos-mtnlion-r5-023.test.releng.scl3.mozilla.com
mavericks can fend for itself.

Comment 24

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8483558 [details] [diff] [review]
bug895995-p3.patch

Oh, Callek's maybe a better choice - I just looked at the r+ for the last patch.

Comment 25

[Dustin J. Mitchell [:dustin] (he/him)]

Note that during the wait for review, Simone has found the need to build a newer version of mercurial for bug 961279, and will be basing his work off of this patch.

Assuming that project keeps moving ahead, I'd suggest just revising attachment 8483558 [details] [diff] [review] with the new version number and putting it up for review on bug 961279, then duping this bug forward.

Comment 26

[Justin Wood (:Callek)]

Comment on attachment 8483558 [details] [diff] [review]
bug895995-p3.patch

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

if tested on our OSX versions, and tested to not bust existing venv's r+=me

Comment 27

[Dustin J. Mitchell [:dustin] (he/him)]

Thanks -- the question of existing venvs is a good one and should be tested.  Over to simone for comment 25.

Comment 28

[Justin Wood (:Callek)]

Attachment: [puppet] remove the old location

a mxr search is finding no users, this removes it (used this way to ensure we don't hit surprising issues down the line since the symlink would only be missing on newly imaged hosts)

If this sticks I'll remove the whole file{} tomorrow

Comment 29

[Justin Wood (:Callek)]

Comment on attachment 8542607 [details] [diff] [review]
[puppet] remove the old location

https://hg.mozilla.org/build/puppet/rev/62b5984152bb
https://hg.mozilla.org/build/puppet/rev/ac3cfd37cff7

Comment 30

[Justin Wood (:Callek)]

Attachment: [puppet] remove the cruft

Comment 31

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8542831 [details] [diff] [review]
[puppet] remove the cruft

http://hg.mozilla.org/build/puppet/rev/975f10fb873514383a9ed133757842d97b83b1e0

Comment 32

[Dustin J. Mitchell [:dustin] (he/him)]

We now have the same mercurial path on all systems but Windows, which is as good as it's gonna get.  Thanks Callek!