1255585 - Python virtualenv is new, and build backend is regenerated, with every |mach build mobile/android| Fennec artifact build

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Nick Alexander :nalexander [he/him]]

This is a Fennec artifact mozconfig (i.e., with --enable-artifact-builds).  I constantly see "New python executable in /Users/nalexander/Mozilla/gecko/objdir-droid/_virtualenv/bin/python2.7" when running the (automatic) artifact install; and the build backend is regenerated each build as well.

Comment 1

[Nick Alexander :nalexander [he/him]]

glandium: is there a way to figure out what is triggering the build-backend?

Comment 2

[Nick Alexander :nalexander [he/him]]

With the diff at the end of this comment, I see that the offending file is /Users/nalexander/Mozilla/gecko/objdir-droid/_virtualenv/lib/python2.7/sitecustomize.py.

nalexander@chocho ~/M/gecko> ./mach build mobile/android -v
 0:00.20 /usr/bin/make -C /Users/nalexander/Mozilla/gecko/objdir-droid -j8 -s backend
 0:00.24 Build configuration changed. Regenerating backend.
 0:00.24 /Users/nalexander/Mozilla/gecko/objdir-droid/_virtualenv/lib/python2.7/sitecustomize.py
 0:00.24 Done.
 0:00.39 Reticulating splines...

nalexander@chocho ~/M/gecko> hg diff
diff --git a/Makefile.in b/Makefile.in
--- a/Makefile.in
+++ b/Makefile.in
@@ -109,16 +109,18 @@ backend: $(BUILD_BACKEND_FILES)
 #     backend%RecursiveMakeBackend backend%FasterMakeBackend:
 #         @echo do stuff
 #     backend: backend.RecursiveMakeBackend backend.FasterMakeBackend
 # would only execute the command once.
 #
 # Credit where due: http://stackoverflow.com/questions/2973445/gnu-makefile-rule-generating-a-few-targets-from-a-single-source-file/3077254#3077254
 $(subst .,%,$(BUILD_BACKEND_FILES)):
        @echo 'Build configuration changed. Regenerating backend.'
+       @echo $?
+       @echo 'Done.'
        $(PYTHON) config.status

 Makefile: $(BUILD_BACKEND_FILES)
        @$(TOUCH) $@

 define build_backend_rule
 $(1)_files := $$(shell cat $(1).in)
 $(1): $$($(1)_files)

Comment 3

[Chris Manchester (limited bugmail, email directly)]

I'm not seeing this on a desktop build.

Comment 4

[Nick Alexander :nalexander [he/him]]

Attachment: out

nalexander@chocho ~/M/gecko> rm -rf ; and ./mach build > out

(With some trivial patches to print when we're writing sitecustomize.py.)

Comment 5

[Nick Alexander :nalexander [he/him]]

OK, the issue under all this is that |mach artifact install| (and probably other mach commands that activate the virtualenv) don't specify if they want sys.executable_path or the virtualenv Python path at https://dxr.mozilla.org/mozilla-central/source/python/mozbuild/mozbuild/base.py#666.  After Bug 1253502, this matters.  This manifests as back-to-back |mach artifact install| invocations writing two virtual environments.  I'm not sure if the correct solution is to always specify the virtual Python path, but that solves the issue locally:

	Modified   python/mozbuild/mozbuild/base.py
diff -r 0837015740ff python/mozbuild/mozbuild/base.py
--- a/python/mozbuild/mozbuild/base.py	Thu Mar 10 21:05:05 2016 +0100
+++ b/python/mozbuild/mozbuild/base.py	Thu Mar 10 14:30:45 2016 -0800
@@ -662,9 +662,9 @@ class MozbuildObject(ProcessExecutionMix
         return cls(self.topsrcdir, self.settings, self.log_manager,
             topobjdir=self.topobjdir)
 
     def _activate_virtualenv(self):
-        self.virtualenv_manager.ensure()
+        self.virtualenv_manager.ensure(python=self.virtualenv_manager.python_path)
         self.virtualenv_manager.activate()
 
 
 class MachCommandBase(MozbuildObject):

Comment 6

[Mike Hommey [:glandium]]

That most definitely looks wrong. The python path to pass there is not sys.executable vs. virtualenv python. It's sys.executable vs. some entirely different python.

Comment 7

[Nick Alexander :nalexander [he/him]]

(In reply to Mike Hommey [:glandium] from comment #6)
> That most definitely looks wrong. The python path to pass there is not
> sys.executable vs. virtualenv python. It's sys.executable vs. some entirely
> different python.

So is the real issue that up_to_date is supposed to be true when we pass sys.executable and the virtualenv Python is installed?  'cuz on my device, it's not -- the system Python and the virtualenv Python have different sizes (after following lots of symlinks).  I can verify that's how up_to_date fails if needed.

Comment 8

[Mike Hommey [:glandium]]

wtf? why are your system python and virtualenv python different?

Comment 9

[Nick Alexander :nalexander [he/him]]

(In reply to Mike Hommey [:glandium] from comment #8)
> wtf? why are your system python and virtualenv python different?

No idea.

nalexander@chocho ~/M/gecko> which python
/usr/local/bin/python
nalexander@chocho ~/M/gecko> ls -al /usr/local/bin/python
lrwxr-xr-x  1 nalexander  admin  34 21 Feb 11:39 /usr/local/bin/python -> ../Cellar/python/2.7.11/bin/python
nalexander@chocho ~/M/gecko> ls -al /usr/local/Cellar/python/2.7.11/bin/python
lrwxr-xr-x  1 nalexander  admin  54 22 Jan 08:29 /usr/local/Cellar/python/2.7.11/bin/python -> ../Frameworks/Python.framework/Versions/2.7/bin/python
nalexander@chocho ~/M/gecko> ls -al /usr/local/Cellar/python/2.7.11/Frameworks/Python.framework/Versions/2.7/bin/python2.7
-rwxr-xr-x  1 nalexander  admin  13316 21 Feb 11:38 /usr/local/Cellar/python/2.7.11/Frameworks/Python.framework/Versions/2.7/bin/python2.7

Also:

nalexander@chocho ~/M/gecko> ls -al /usr/local/bin/python2.7
lrwxr-xr-x  1 nalexander  admin  37 21 Feb 11:39 /usr/local/bin/python2.7 -> ../Cellar/python/2.7.11/bin/python2.7

However:

nalexander@chocho ~/M/gecko> ls -al objdir-droid/_virtualenv/bin/python
lrwxr-xr-x  1 nalexander  staff  9 10 Mar 14:34 objdir-droid/_virtualenv/bin/python -> python2.7
nalexander@chocho ~/M/gecko> ls -al objdir-droid/_virtualenv/bin/python2.7
-rwxr-xr-x  1 nalexander  staff  12568 10 Mar 14:34 objdir-droid/_virtualenv/bin/python2.7
nalexander@chocho ~/M/gecko> python --version
Python 2.7.11
nalexander@chocho ~/M/gecko> ./mach python --version
Python 2.7.11

This is after a fresh clobber.  Is it possible that some other Python is being found, instead of the one I installed via homebrew?

Comment 10

[Mike Hommey [:glandium]]

that is what this suggests...

Comment 11

[Chris Manchester (limited bugmail, email directly)]

I was able to reproduce after upgrading Python on my macbook. Virtualenv is finding itself a different Python, here: https://dxr.mozilla.org/mozilla-central/rev/dd1abe874252e507b825a0a4e1063b0e13578288/python/virtualenv/virtualenv.py#1257

Comment 12

[Chris Manchester (limited bugmail, email directly)]

Attachment: MozReview Request: Bug 1255585 - Prevent Python executable mis-match from constantly clobbering the virtualenv on OS X. r=gps

Virtualenv will sometimes find a different executable from its sys.executable on OS X,
causing a check in the build system comparing filesizes between sys.executable and virtualenv
python to fail, resulting in clobbering and re-building the virtualenv every time the virtualenv
is activated, causing the build backend and more to be re-built.

Instead of checking file sizes, this commit causes us to compare the output of sys.version
to estimate compatibility between interpreters.

Review commit: https://reviewboard.mozilla.org/r/39367/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/39367/

Comment 13

[Chris Manchester (limited bugmail, email directly)]

Nick, can you verify the fix in comment 12?

Comment 14

[Nick Alexander :nalexander [he/him]]

(In reply to Chris Manchester (:chmanchester) from comment #13)
> Nick, can you verify the fix in comment 12?

Yes, this works for me locally.  Thanks!

Comment 15

[Gregory Szorc [:gps]]

There is code in python/virtualenv/virtualenv.py that mucks about with the python binary in the virtualenv. See "mach_o_change" for one example. So yeah, sniffing file size feels fragile.

Comment 16

[Gregory Szorc [:gps]]

Comment on attachment 8729329 [details]
MozReview Request: Bug 1255585 - Prevent Python executable mis-match from constantly clobbering the virtualenv on OS X. r=gps

https://reviewboard.mozilla.org/r/39367/#review36177

I'm not a fan of having to spawn processes to perform this check. This will add dozens of milliseconds of overhead on Windows.

I think what we should do instead is write a file into the virtualenv when it is created giving info about the source and virtualenv python executables, notably path and file size. We can read this state file and compare against filesystem state to see if things have changed and act accordingly.

Will that work?

Comment 17

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8729329 [details]
MozReview Request: Bug 1255585 - Prevent Python executable mis-match from constantly clobbering the virtualenv on OS X. r=gps

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/39367/diff/1-2/

Comment 18

[Chris Manchester (limited bugmail, email directly)]

Comment 17 is updated with gps' suggestion. I'll re-request review once I'm able to test this on OS X.

Comment 19

[Gregory Szorc [:gps]]

Comment on attachment 8729329 [details]
MozReview Request: Bug 1255585 - Prevent Python executable mis-match from constantly clobbering the virtualenv on OS X. r=gps

https://reviewboard.mozilla.org/r/39367/#review36263

Assuming you've verified this actually works, land it!

Comment 20

[Gregory Szorc [:gps]]

Comment on attachment 8729329 [details]
MozReview Request: Bug 1255585 - Prevent Python executable mis-match from constantly clobbering the virtualenv on OS X. r=gps

https://reviewboard.mozilla.org/r/39367/#review36265

::: python/mozbuild/mozbuild/virtualenv.py:107
(Diff revision 2)
> +            print(python, file=fh)
> +            print(os.path.getsize(python), file=fh)

print() here is kinda wonky. Typically fh.write() is used. I /think/ print() will add CRLF on Windows where fh.write() must be told the line ending. But it shouldn't matter for this use case.

Comment 21

[Chris Manchester (limited bugmail, email directly)]

(In reply to Gregory Szorc [:gps] from comment #20)
> Comment on attachment 8729329 [details]
> MozReview Request: Bug 1255585 - Prevent Python executable mis-match from
> constantly clobbering the virtualenv on OS X. r=gps
> 
> https://reviewboard.mozilla.org/r/39367/#review36265
> 
> ::: python/mozbuild/mozbuild/virtualenv.py:107
> (Diff revision 2)
> > +            print(python, file=fh)
> > +            print(os.path.getsize(python), file=fh)
> 
> print() here is kinda wonky. Typically fh.write() is used. I /think/ print()
> will add CRLF on Windows where fh.write() must be told the line ending. But
> it shouldn't matter for this use case.

I don't think this r+ was meant to be cancelled, because an issue wasn't opened with it... I'll just address this comment and land.

Comment 22

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2ed1a0e47b18

Comment 23

[Gregory Szorc [:gps]]

Comment on attachment 8729329 [details]
MozReview Request: Bug 1255585 - Prevent Python executable mis-match from constantly clobbering the virtualenv on OS X. r=gps

https://reviewboard.mozilla.org/r/39367/#review36269

No it wasn't meant to be cancelled. WTF MozReview.

Comment 24

[Ryan VanderMeulen [:RyanVM]]

Backed out for Windows build bustage.
https://hg.mozilla.org/integration/mozilla-inbound/rev/49c270ffc682

https://treeherder.mozilla.org/logviewer.html#?job_id=23611292&repo=mozilla-inbound

Comment 25

[Gregory Szorc [:gps]]

That's a fun failure.

On Windows, you can't overwrite or delete a file/executable that is currently running. So if we're trying to reinstall the virtualenv from a Python process running from that virtualenv, you'll get a permissions error. That might be what happened here. We could work around it with a clobber in automation. But this may be an issue for local developers.

Comment 26

[Ryan VanderMeulen [:RyanVM]]

And Mac.
https://treeherder.mozilla.org/logviewer.html#?job_id=23611299&repo=mozilla-inbound

Comment 27

[Ryan VanderMeulen [:RyanVM]]

Also appears to have left bld-lion-r5-080 in a permanently broken state, so that's nice.

Comment 28

[Chris Manchester (limited bugmail, email directly)]

Windows bustage was just a matter of normalizing paths. On OS X though, somewhere between mozharness' own virtualenv and multiple objdirs a consistent Python is too much to ask for, so we attempt to re-create the virtualenv when we shouldn't, and somehow create a circular symlink. In particular, /tools/python27/bin/python2.7 is used to create the virtualenv, but we see /tools/buildbot/bin/python when we try to run printconfigsetting.py after the build.

In light of this I think we should dump hexversion in the state file instead of using a path, which still mostly checks what we want, and works fine on automation: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e3b6e3762e2a

Comment 29

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8729329 [details]
MozReview Request: Bug 1255585 - Prevent Python executable mis-match from constantly clobbering the virtualenv on OS X. r=gps

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/39367/diff/2-3/

Comment 30

[Gregory Szorc [:gps]]

Comment on attachment 8729329 [details]
MozReview Request: Bug 1255585 - Prevent Python executable mis-match from constantly clobbering the virtualenv on OS X. r=gps

https://reviewboard.mozilla.org/r/39367/#review36411

I still think we could have got the path based approach to work. But let's try this.

Comment 31

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a1a8876174c3

Comment 32

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/a1a8876174c3