Closed Bug 1437593 Opened 6 years ago Closed 6 years ago

Ability to install and manage python packages with pipenv

Categories

(Firefox Build System :: General, enhancement)

Product:
Firefox Build System
Component:
General
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(firefox61 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla61
Tracking Flags:
Tracking Status
firefox61 --- fixed

People

(Reporter: ahal, Assigned: davehunt)

References

Details

Attachments

(10 files, 19 obsolete files)

Bug 1437593 - Allow pip requirements to be installed from vendored packages;
59 bytes, text/x-review-board-request
gps
: review+
Details
Bug 1437593 - Vendor pipenv 11.10.1;
59 bytes, text/x-review-board-request
Details
Bug 1437593 - Vendor virtualenv-clone 0.3.0;
59 bytes, text/x-review-board-request
ted
: review+
Details
Bug 1437593 - Vendor certifi 2018.1.18;
59 bytes, text/x-review-board-request
ted
: review+
Details
Bug 1437593 - Vendor virtualenv 15.2.0;
59 bytes, text/x-review-board-request
gps
: review+
Details
Bug 1437593 - Add support for using Pipfiles for managing virtual environment dependencies;
59 bytes, text/x-review-board-request
ted
: review+
Details
Bug 1437593 - Move initial virtual environment to _virtualenvs/init;
59 bytes, text/x-review-board-request
ted
: review+
Details
Bug 1437593 - Exclude third_party directory from linting;
59 bytes, text/x-review-board-request
ted
: review+
Details
Bug 1437593 - Update tup backend to support new python virtualenv;
59 bytes, text/x-review-board-request
chmanchester
: review+
Details
Bug 1437593 - Vendor pathlib2 2.3.2;
59 bytes, text/x-review-board-request
ted
: review+
Details 
While fixing bug 1437484, I had to create a requirements.txt with all the correct hashes for our doc dependencies. While 'hashin' certainly makes this process bearable, it's still a pain. You still have to type out all of the sub-dependencies, and figure out what their latest supported version is (which means you need to either install it locally or look it up on pypi).

Instead of using requirements.txt with --require-hashes, we should add the ability to use pipfiles via pipenv. To start, I'd just like to be able to use this from mach commands (via the VirtualenvManager). We could look into deeper build system integrations in a follow-up.

For this bug, I envision these steps:
1) Vendor pipenv to the tree
2) Add some kind of 'install_pipfile' function to VirtualenvManager
3) Replace in-tree requirements.txt with Pipfile/Pipfile.lock as needed

Having this in-tree will also make it a lot easier to implement something in bug 1346026.
See Also: → 1437484
I support this proposal! As we discussed on IRC, when I looked into pipenv previously in the context of that other bug it looked like it was the right tool for the job but it wasn't quite stable yet. If we vendor pipenv itself in tree then that should make its stability not a big problem (as long as the current version works well enough to do the things we need it to do).

Given that we don't vendor Python packages for every Python command in the tree, it might also be nice to provide some way to take a Pipfile.lock and generate the list of packages that would need to be uploaded to our pip mirror so that CI tasks don't wind up pulling from the actual pip repository. (We could even then add a lint-type task that validates that all the packages listed in Pipfile.lock files in-tree are available on the mirror, so we would get advance notice of potential CI instability!)
(In reply to Andrew Halberstadt [:ahal] from comment #0)
> While fixing bug 1437484, I had to create a requirements.txt with all the
> correct hashes for our doc dependencies. While 'hashin' certainly makes this
> process bearable, it's still a pain.

As mentioned in #releng, there's dephash https://pypi.org/project/dephash/ , which is probably obsoleted by pip-tools'

    pip-compile --generate-hashes --output-file requirements.txt requirements.in

> Instead of using requirements.txt with --require-hashes, we should add the
> ability to use pipfiles via pipenv. To start, I'd just like to be able to
> use this from mach commands (via the VirtualenvManager). We could look into
> deeper build system integrations in a follow-up.

I think this works; reading up on pipfiles, it looks like they're supposed to be a new and improved requirements.txt.
I was also thinking that since pipenv can handle its own virtualenvs (even on Windows), this might be a good approach to gps' long standing goal of multiple different virtualenvs in the tree.

p.s, wish I knew about dephash earlier this morning :)
Bug 985141 has my (rather old) vision for how I always wanted to integrate virtualenvs into mach. I think pipenv is newer than when I wrote that bug. It may just be the right tool for the job!
Blocks: 985141
I probably won't have enough time to finish this, but I did look into it a little. It is possible to get pipenv to use the current python environment rather than its own virtualenvs by passing in --system. However if we did this, it might cause issues.

For example we have requests==2.9.1 vendored into the tree. If we added a Pipfile.lock that specified a different version of requests, then running `pipenv install --system` would uninstall our vendored copy and replace it with the new one. This might cause problems for other mach commands.

So I think the goal of this bug needs to change slightly. We don't want to add the ability to use pipenv *with* VirtualenvManager, we want the ability to use pipenv *instead* of VirtualenvManager.
Summary: Ability to install and manage python packages with pipenv from VirtualenvManager → Ability to install and manage python packages with pipenv
Assignee: nobody → dave.hunt
Status: NEW → ASSIGNED
Comment on attachment 8950760 [details]
Bug 1437593 - Add support for using Pipfiles for managing virtual environment dependencies.

https://reviewboard.mozilla.org/r/220008/#review225842

Thanks, this looks like a good first approach to me! Consider this an f+, we'll mostly need to figure out the vendoring situation and agree on method names/locations.

::: python/mozbuild/mozbuild/base.py:751
(Diff revision 1)
>  
>  
>      def _set_log_level(self, verbose):
>          self.log_manager.terminal_handler.setLevel(logging.INFO if not verbose else logging.DEBUG)
>  
> +    def activate(self, path):

We'll probably want to call this activate_pipfile or some such.

::: python/mozbuild/mozbuild/virtualenv.py:527
(Diff revision 1)
> +    def activate_pipenv(self, path):
> +        """Install a Pipfile located at path and activate environment"""

Gps or another build peer should have final say, but I think this should be a static or module method that returns a new VirtualenvManager instance. This will subvert the existing instance (which was originally created to manage the build system virtualenv).

I don't know how exactly this will look from an API standpoint. Maybe callers can just choose to ignore the returned instance if they don't need it.

::: python/mozbuild/mozbuild/virtualenv.py:527
(Diff revision 1)
>          # It /might/ be possible to cheat and set sys.executable to
>          # self.python_path. However, this seems more risk than it's worth.
>          subprocess.check_call([os.path.join(self.bin_path, 'pip')] + args,
>              stderr=subprocess.STDOUT)
>  
> +    def activate_pipenv(self, path):

What happens if path doesn't contain a Pipfile?

::: python/mozbuild/mozbuild/virtualenv.py:530
(Diff revision 1)
> +            [os.path.join(self.bin_path, 'pipenv'), 'install'],
> +            stderr=subprocess.STDOUT, cwd=path)

We could set the WORKON_HOME environment variable to  ~/.mozbuild/virtualenvs so the virtualenvs will be created in the mozilla statedir. Or maybe they should also go in the objdir?

::: tools/docs/mach_commands.py:54
(Diff revision 1)
>          try:
>              jsdoc = which.which('jsdoc')
>          except which.WhichError:
>              return die('jsdoc not found - please install from npm.')
>  
> -        self._activate_virtualenv()
> +        self.activate(os.path.join(here))

nit: unnecessary join
Attachment #8950760 - Flags: review?(ahalberstadt)
Comment on attachment 8950760 [details]
Bug 1437593 - Add support for using Pipfiles for managing virtual environment dependencies.

https://reviewboard.mozilla.org/r/220008/#review225952

::: python/mozbuild/mozbuild/virtualenv.py:527
(Diff revision 1)
>          # It /might/ be possible to cheat and set sys.executable to
>          # self.python_path. However, this seems more risk than it's worth.
>          subprocess.check_call([os.path.join(self.bin_path, 'pip')] + args,
>              stderr=subprocess.STDOUT)
>  
> +    def activate_pipenv(self, path):

Address in followup, where I now raise an exception if there is no Pipfile.

::: python/mozbuild/mozbuild/virtualenv.py:527
(Diff revision 1)
> +    def activate_pipenv(self, path):
> +        """Install a Pipfile located at path and activate environment"""

I like this idea, however VirtualenvManager currently expects a manifest file.

::: python/mozbuild/mozbuild/virtualenv.py:530
(Diff revision 1)
> +            [os.path.join(self.bin_path, 'pipenv'), 'install'],
> +            stderr=subprocess.STDOUT, cwd=path)

I've addressed this by using the objdir as it was easy to get a reference to this.
Blocks: 1388013
Comment on attachment 8950855 [details]
Bug 1437593 - Add Pipfile and Pipfile.lock for tools/docs.

https://reviewboard.mozilla.org/r/220090/#review226216

::: tools/docs/Pipfile:10
(Diff revision 1)
> +recommonmark = "*"
> +sphinx = "<1.7.0"
> +sphinx-js = "*"
> +sphinx_rtd_theme = "*"
> +typing = "*"

There's a good chance I just don't know how this works, but are we not supposed to pin the versions in here? Otherwise wouldn't running `pipenv install` cause the Pipfile.lock to be regenerated with the latest package versions of all these? Or would we run `pipenv --ignore-pipfile` to stop that from happening.
Attachment #8950855 - Flags: review?(ahalberstadt) → review-
Comment on attachment 8950856 [details]
Bug 1437593 - Addressing review feedback and abort if Pipfile.lock is out of date.

https://reviewboard.mozilla.org/r/220092/#review226226

These will require a build peer's review (though gps may want to tackle it). So these comments are still just feedback.

::: python/mozbuild/mozbuild/virtualenv.py:531
(Diff revision 1)
>  
>      def activate_pipenv(self, path):
>          """Install a Pipfile located at path and activate environment"""
> -        subprocess.check_call(
> -            [os.path.join(self.bin_path, 'pipenv'), 'install'],
> -            stderr=subprocess.STDOUT, cwd=path)
> +        pipenv = os.path.join(self.bin_path, 'pipenv')
> +        env = os.environ.copy()
> +        env['WORKON_HOME'] = os.path.join(self.topobjdir, 'virtualenvs')

Fyi, you can find ~/.mozbuild with this:
https://searchfox.org/mozilla-central/source/python/mozboot/mozboot/util.py#10

Though the objdir is probably the better place anyway. This will create both `virtualenvs` and the usual `_virtualenv` directories which would be a little weird. We should move the latter into the former as part of this.
Attachment #8950856 - Flags: review?(ahalberstadt)
Comment on attachment 8950855 [details]
Bug 1437593 - Add Pipfile and Pipfile.lock for tools/docs.

https://reviewboard.mozilla.org/r/220090/#review226230

::: tools/docs/Pipfile:3
(Diff revision 1)
> +[[source]]
> +
> +url = "https://pypi.python.org/simple"

We may want to point this at our internal pypi, though we'd likely need to get a lot of new packages added and I have no idea what the process for that is these days (or if it's still just file a bug and wait for releng to do it).

I also saw that it's possible to have multiple sources defined and you can specify which packages should be downloaded from which sources.
Comment on attachment 8950855 [details]
Bug 1437593 - Add Pipfile and Pipfile.lock for tools/docs.

https://reviewboard.mozilla.org/r/220090/#review226216

> There's a good chance I just don't know how this works, but are we not supposed to pin the versions in here? Otherwise wouldn't running `pipenv install` cause the Pipfile.lock to be regenerated with the latest package versions of all these? Or would we run `pipenv --ignore-pipfile` to stop that from happening.

The way I understand it, Pipfile contains all of your direct dependencies and any specific version identifiers if necessary. Pipfile.lock contains the entire dependency graph, along with hashes for all packages. When you `pipenv install [package]` the Pipfile and Pipfile.lock are updated. You can also run `pipenv lock` to (re)create the Pipfile.lock or `pipenv update` to check for newer versions of dependencies that match the version specifiers, and update the Pipfile.lock.

When you run `pipenv install --deploy` a virtual environment if created (if it doesn't exist) and the dependencies listed in Pipfile.lock are installed. If there have been changes to Pipfile, then the install will abort as the Pipfile.lock will be out of date.

The workflow would therefore be that we use `pipenv install --deploy` indirectly when we're running the mach command. If we become aware of a reason to update the dependencies we can change to the directory containing the Pipfile, run `pipenv update [package]`, test the results of using the new version(s), and then submit a patch with the new Pipfile.lock.

In the case of this Pipfile, the latest version of Sphinx causes a failure. I've specified to use the latest version below 1.7.0 as we know this works, and we have a Pipfile.lock that pins to 1.6.7. If a 1.6.8 was released, we could run `pipenv update sphinx` and just Pipfile.lock will be updated. If we resolve the issue with 1.7.0 then we should be able to remove the version specifier and run `pipenv update`.

Also note that 'typing' is not a direct dependency, but is not being picked up by pipenv as a conditional dependency of sphinx, so I've had to include it in the Pipfile for now. See https://github.com/sphinx-doc/sphinx/blob/2602be13bc986efa4401ea40ba9591dc8f5ac259/setup.py#L38-L40, and I suspect this is related to https://github.com/pypa/pipenv/issues/1229.
Comment on attachment 8950855 [details]
Bug 1437593 - Add Pipfile and Pipfile.lock for tools/docs.

https://reviewboard.mozilla.org/r/220090/#review226230

> We may want to point this at our internal pypi, though we'd likely need to get a lot of new packages added and I have no idea what the process for that is these days (or if it's still just file a bug and wait for releng to do it).
> 
> I also saw that it's possible to have multiple sources defined and you can specify which packages should be downloaded from which sources.

This is a good idea. Is this preferred over vendoring? Do you know who I can ask about the internal PyPI mirror, such as location and process for adding packages?
Comment on attachment 8950855 [details]
Bug 1437593 - Add Pipfile and Pipfile.lock for tools/docs.

https://reviewboard.mozilla.org/r/220090/#review226230

> This is a good idea. Is this preferred over vendoring? Do you know who I can ask about the internal PyPI mirror, such as location and process for adding packages?

I guess I should have made it a comment rather than an issue, I think it's something we can probably tackle in a follow-up. But to answer the question, I'm not sure who handles this anymore. I'd ask around in #releng.
Comment on attachment 8950855 [details]
Bug 1437593 - Add Pipfile and Pipfile.lock for tools/docs.

https://reviewboard.mozilla.org/r/220090/#review226216

> The way I understand it, Pipfile contains all of your direct dependencies and any specific version identifiers if necessary. Pipfile.lock contains the entire dependency graph, along with hashes for all packages. When you `pipenv install [package]` the Pipfile and Pipfile.lock are updated. You can also run `pipenv lock` to (re)create the Pipfile.lock or `pipenv update` to check for newer versions of dependencies that match the version specifiers, and update the Pipfile.lock.
> 
> When you run `pipenv install --deploy` a virtual environment if created (if it doesn't exist) and the dependencies listed in Pipfile.lock are installed. If there have been changes to Pipfile, then the install will abort as the Pipfile.lock will be out of date.
> 
> The workflow would therefore be that we use `pipenv install --deploy` indirectly when we're running the mach command. If we become aware of a reason to update the dependencies we can change to the directory containing the Pipfile, run `pipenv update [package]`, test the results of using the new version(s), and then submit a patch with the new Pipfile.lock.
> 
> In the case of this Pipfile, the latest version of Sphinx causes a failure. I've specified to use the latest version below 1.7.0 as we know this works, and we have a Pipfile.lock that pins to 1.6.7. If a 1.6.8 was released, we could run `pipenv update sphinx` and just Pipfile.lock will be updated. If we resolve the issue with 1.7.0 then we should be able to remove the version specifier and run `pipenv update`.
> 
> Also note that 'typing' is not a direct dependency, but is not being picked up by pipenv as a conditional dependency of sphinx, so I've had to include it in the Pipfile for now. See https://github.com/sphinx-doc/sphinx/blob/2602be13bc986efa4401ea40ba9591dc8f5ac259/setup.py#L38-L40, and I suspect this is related to https://github.com/pypa/pipenv/issues/1229.

Ok I think this approach makes sense, thanks for clarfying. I thought `pipenv install` would automatically update the Pipfile.lock with the latest versions, but sounds like you're saying it doesn't and there's a separate command (`pipenv update`) for that.
Comment on attachment 8950855 [details]
Bug 1437593 - Add Pipfile and Pipfile.lock for tools/docs.

https://reviewboard.mozilla.org/r/220090/#review226590
Attachment #8950855 - Flags: review-
Comment on attachment 8950855 [details]
Bug 1437593 - Add Pipfile and Pipfile.lock for tools/docs.

https://reviewboard.mozilla.org/r/220090/#review229154
Comment on attachment 8950855 [details]
Bug 1437593 - Add Pipfile and Pipfile.lock for tools/docs.

https://reviewboard.mozilla.org/r/220090/#review226216

> Ok I think this approach makes sense, thanks for clarfying. I thought `pipenv install` would automatically update the Pipfile.lock with the latest versions, but sounds like you're saying it doesn't and there's a separate command (`pipenv update`) for that.

This has changed in the latest version of pipenv. There's now a `pipenv sync` command that installs all packages specified in the `Pipfile.lock`. As far as I can tell, this changes the workflow to using `pipenv install` to add/update packages in `Pipfile` (and `Pipfile.lock`), and `pipenv sync` to prepare an environment for use. The pipenv project still appears to be quite active, so I may already be out of date!
Comment on attachment 8950855 [details]
Bug 1437593 - Add Pipfile and Pipfile.lock for tools/docs.

https://reviewboard.mozilla.org/r/220090/#review226230

> I guess I should have made it a comment rather than an issue, I think it's something we can probably tackle in a follow-up. But to answer the question, I'm not sure who handles this anymore. I'd ask around in #releng.

It appears that if we use an index other than PyPI then we'd lose support for hashes. See https://github.com/pypa/pipenv/issues/988#issuecomment-346204212
Comment on attachment 8950760 [details]
Bug 1437593 - Add support for using Pipfiles for managing virtual environment dependencies.

https://reviewboard.mozilla.org/r/220008/#review229404

::: python/mozbuild/mozbuild/base.py:753
(Diff revision 2)
>      def _set_log_level(self, verbose):
>          self.log_manager.terminal_handler.setLevel(logging.INFO if not verbose else logging.DEBUG)
>  
> +    def activate(self, path):
> +        self._activate_virtualenv()
> +        self.virtualenv_manager.install_pip_package('pipenv==9.0.3')

This will use the Internet to download pipenv. We can't have that.

Please vendor pipenv first and install from the source checkout.

::: python/mozbuild/mozbuild/virtualenv.py:527
(Diff revision 2)
> +    def activate_pipenv(self, path):
> +        """Install a Pipfile located at path and activate environment"""
> +        subprocess.check_call(
> +            [os.path.join(self.bin_path, 'pipenv'), 'install'],
> +            stderr=subprocess.STDOUT, cwd=path)

What is `path` here? I thought it was the path to a `Pipfile`, but it apparently isn't?

Will the virtualenv get created in the source directory? We don't want that. (We want extra state to be in the objdir.)
Attachment #8950760 - Flags: review-
Comment on attachment 8950855 [details]
Bug 1437593 - Add Pipfile and Pipfile.lock for tools/docs.

https://reviewboard.mozilla.org/r/220090/#review229408

::: tools/docs/Pipfile.lock:6
(Diff revision 2)
> +        "host-environment-markers": {
> +            "implementation_name": "cpython",
> +            "implementation_version": "0",
> +            "os_name": "posix",
> +            "platform_machine": "x86_64",
> +            "platform_python_implementation": "CPython",
> +            "platform_release": "17.4.0",
> +            "platform_system": "Darwin",
> +            "platform_version": "Darwin Kernel Version 17.4.0: Sun Dec 17 09:19:54 PST 2017; root:xnu-4570.41.2~1/RELEASE_X86_64",
> +            "python_full_version": "2.7.13",
> +            "python_version": "2.7",
> +            "sys_platform": "darwin"

How does this work in multi-platform environments? Is this just debugging info or what? If it isn't important, can we strip it so it doesn't churn every time someone changes a `Pipfile.lock` file?
Comment on attachment 8954058 [details]
Bug 1437593 - Vendor pipenv and dependencies.

https://reviewboard.mozilla.org/r/223204/#review229416

First, we vendor Python packages without their version component.

Second, there are multiple packages being vendored here. We want one commit per package.

Third, we don't want to make non-vendoring changes along with the vendoring, if possible.

Fourth, is there any way to prune the vendored packages vendored as part of pipenv? The set of vendored packages overlaps with packages we already have in the repo. I don't like taking duplicates, especially if pipenv is doing `sys.path` muckery and this results in multiple versions of these packages available on `sys.path`. I'm also skeptical of pipenv vendoring its own (apparently patched) copy of pip. What's going on here and do we have any control to not have so much duplication?
Attachment #8954058 - Flags: review?(gps) → review-
Comment on attachment 8950760 [details]
Bug 1437593 - Add support for using Pipfiles for managing virtual environment dependencies.

https://reviewboard.mozilla.org/r/220008/#review229488

::: python/mozbuild/mozbuild/base.py:753
(Diff revision 2)
>      def _set_log_level(self, verbose):
>          self.log_manager.terminal_handler.setLevel(logging.INFO if not verbose else logging.DEBUG)
>  
> +    def activate(self, path):
> +        self._activate_virtualenv()
> +        self.virtualenv_manager.install_pip_package('pipenv==9.0.3')

Addressed in a later commit where I vendor pipenv.

::: python/mozbuild/mozbuild/virtualenv.py:527
(Diff revision 2)
> +    def activate_pipenv(self, path):
> +        """Install a Pipfile located at path and activate environment"""
> +        subprocess.check_call(
> +            [os.path.join(self.bin_path, 'pipenv'), 'install'],
> +            stderr=subprocess.STDOUT, cwd=path)

This is the path containing the Pipfile. I've updated this in a later commit to set an environment variable controlling the path to the Pipfile.
Comment on attachment 8950855 [details]
Bug 1437593 - Add Pipfile and Pipfile.lock for tools/docs.

https://reviewboard.mozilla.org/r/220090/#review229494

::: tools/docs/Pipfile.lock:6
(Diff revision 2)
> +        "host-environment-markers": {
> +            "implementation_name": "cpython",
> +            "implementation_version": "0",
> +            "os_name": "posix",
> +            "platform_machine": "x86_64",
> +            "platform_python_implementation": "CPython",
> +            "platform_release": "17.4.0",
> +            "platform_system": "Darwin",
> +            "platform_version": "Darwin Kernel Version 17.4.0: Sun Dec 17 09:19:54 PST 2017; root:xnu-4570.41.2~1/RELEASE_X86_64",
> +            "python_full_version": "2.7.13",
> +            "python_version": "2.7",
> +            "sys_platform": "darwin"

Apparently this is for auditing purposes. It's been discussed here: https://github.com/pypa/pipenv/issues/753. If we removed it, pipenv would just add it back in when the Pipfile.lock is regenerated, so we'd still have the churn.
Comment on attachment 8954058 [details]
Bug 1437593 - Vendor pipenv and dependencies.

https://reviewboard.mozilla.org/r/223204/#review229416

Thanks for your feedback!

> First, we vendor Python packages without their version component.

How do we manage when we have different version requirements? At one point I thought I was going to need a more recent version of requests to satisfy pipenv. If that had been the case would we just block on updating requests?

> Second, there are multiple packages being vendored here. We want one commit per package.

+1

> Third, we don't want to make non-vendoring changes along with the vendoring, if possible.

+1

> Fourth, is there any way to prune the vendored packages vendored as part of pipenv? The set of vendored packages overlaps with packages we already have in the repo. I don't like taking duplicates, especially if pipenv is doing sys.path muckery and this results in multiple versions of these packages available on sys.path.

I'm not sure. It seems pipenv is moving fast and breaking things. I suspect once things settle there will be less need for them to patch the dependencies. Maybe we should hold off using it until then? There could be good reasons not to, but I could see that we could reduce our initial virtual environment to just pipenv and dependencies, and for all mach commands to provide Pipfiles that satisfy their specific requirements.

> I'm also skeptical of pipenv vendoring its own (apparently patched) copy of pip. What's going on here and do we have any control to not have so much duplication?

Here's a summary of the pip changes:

 * Pip is modified, to make it work with Pipenv's custom virtualenv locations.
 * Pip is modified, to make it work with pip-tool modifications.
 * Pip is modified, to make it resolve deep extras links.
In addition to the outstanding questions in comment 32, I was wondering if we have a document that explains our policy/approach to vendoring vs using our internal PyPI mirror? If not, perhaps I can write one (with guidance on what that policy/approach is).
Flags: needinfo?(gps)
I don't know that we have that written down anywhere, but I would summarize it as "you must be able to build Firefox with only a source checkout". That is, doing a build must not require installing additional packages from the internet. Beyond that it gets a little murky. I think that for very common things like running test harnesses, we should have dependencies vendored so those don't require an internet connection. For less common things we have allowed installing packages from a package manager. `mach lint` will install required packages on-the-fly, for example, and `mach vendor rust` will install `cargo-vendor` if it's not already present.

For things that we're going to run in CI, the ideal state is that we don't rely on external resources, so their dependencies should either be vendored or we should have them available on our PyPI mirror. (We don't currently have any way to enforce this, though.)
Product: Core → Firefox Build System
The maintainer tweeted this recently:
https://twitter.com/kennethreitz/status/972832442708123648

So it's definitely not stable (despite being v11). I'm also mildly worried about the maintainer(s) attitudes towards new feature requests. I've seen many examples of issues being closed without adequate explanation, so it's worth keeping in mind upstreaming features for our needs could be difficult.

But I guess as long as this is more of a side-capability to start, and the most recent version (11.4) works well enough for our needs, I don't see much benefit in waiting. I can't imagine an upgrade in the future causing *that* much trouble.
Comment on attachment 8950760 [details]
Bug 1437593 - Add support for using Pipfiles for managing virtual environment dependencies.

I think mozreview/bugzilla re-flagged me alongside gps.

Though just in case the review requests were intentional, I did look at the updated patches. They look good to me and I don't have anything to add on top of gps' comments.
Attachment #8950760 - Flags: review?(ahalberstadt)
Attachment #8950855 - Flags: review?(ahalberstadt)
Attachment #8950856 - Flags: review?(ahalberstadt)
I discovered the nascent https://github.com/kennethreitz/pipenvlib today, which could be an alternative to spawning a process for pipenv.
I spent a decent amount of time looking into this again yesterday, hoping the some of the many new releases of pipenv had made this easier. Unfortunately it appears that the latest versions have a lot more dependencies, even though some of these are vendored within pipenv itself, which doesn't make a lot of sense to me.

I have a working set of patches, but this includes 14 new vendored pacakges, and one upgraded package. The vendored packages also includes cryptography, which I'm unable to install on macOS without using openssl installed via brew and following the suggestion here: https://github.com/pyca/cryptography/issues/3489#issuecomment-312607156 so this might be undesirable.

I'll push my changes later today for feedback.
Attachment #8950760 - Attachment is obsolete: true
Attachment #8950855 - Attachment is obsolete: true
Attachment #8950856 - Attachment is obsolete: true
Attachment #8954058 - Attachment is obsolete: true
Attachment #8954059 - Attachment is obsolete: true
Attachment #8954059 - Flags: review?(gps)
Comment on attachment 8963076 [details]
Bug 1437593 - Vendor pyOpenSSL 17.5.0;

https://reviewboard.mozilla.org/r/231946/#review237484


Code analysis found 1 defect in this patch:
 - 1 defect found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: third_party/python/pyOpenSSL/tox.ini:26
(Diff revision 1)
> +    coverage run --parallel -m OpenSSL.debug
> +    coverage run --parallel -m pytest -v {posargs}
> +
> +[testenv:py27-twistedMaster]
> +deps =
> +    # [tls,conch] syntax doesn't work here so we enumerate all dependencies.

Error: "use 'disabled=<reason>' to disable a test instead of a comment"
 [no-comment-disable]
Comment on attachment 8963076 [details]
Bug 1437593 - Vendor pyOpenSSL 17.5.0;

https://reviewboard.mozilla.org/r/231946/#review237484

> Error: "use 'disabled=<reason>' to disable a test instead of a comment"
>  [no-comment-disable]

There's a bug on file to get all linters to ignore all third party paths. But for now you can fix this by adding 'third_party' to this list:
https://searchfox.org/mozilla-central/source/tools/lint/test-disable.yml#8
Comment on attachment 8963084 [details]
Bug 1437593 - Add support for using Pipfiles for managing virtual environment dependencies;

https://reviewboard.mozilla.org/r/231962/#review237568

::: python/mozbuild/mozbuild/base.py:752
(Diff revision 1)
>  
>      def _set_log_level(self, verbose):
>          self.log_manager.terminal_handler.setLevel(logging.INFO if not verbose else logging.DEBUG)
>  
> +    def activate_pipfile(self, path):
> +        pipfile = os.path.join(path, 'Pipfile')

Might as well wrap this in an `if not path.endswith('Pipfile')` so consumers can pass in either a directory or a pipfile.

::: python/mozbuild/mozbuild/virtualenv.py:553
(Diff revision 1)
> +        self.virtualenv_root = subprocess.check_output(
> +            [pipenv, '--venv'],
> +            stderr=subprocess.STDOUT,
> +            env=env).rstrip()
> +
> +        self.activate()

I doubt there's a use case for this now, but is there a way to re-activate the initial virtualenv? Probably fine to leave as follow-up fodder.
Comment on attachment 8963086 [details]
Bug 1437593 - Move initial virtual environment to _virtualenvs/init;

https://reviewboard.mozilla.org/r/231966/#review237572

::: build/moz.configure/init.configure:236
(Diff revision 1)
> -            os.path.join(topobjdir, '_virtualenv'), out,
> +            os.path.join(topobjdir, '_virtualenvs', 'init'), out,
>              os.path.join(topsrcdir, 'build', 'virtualenv_packages.txt'))

Definitely follow-up fodder, but maybe we could even turn `build/virtualenv_packages.txt` itself into a Pipfile.
Thanks for pushing on this Dave, excited to see this and bug 1388013 land! This all looks good to me, f+.
Comment on attachment 8963068 [details]
Bug 1437593 - Allow pip requirements to be installed from vendored packages;

https://reviewboard.mozilla.org/r/231930/#review239060
Attachment #8963068 - Flags: review?(gps) → review+
Comment on attachment 8963074 [details]
Bug 1437593 - Vendor virtualenv 15.2.0;

https://reviewboard.mozilla.org/r/231942/#review239062
Attachment #8963074 - Flags: review?(gps) → review+
Comment on attachment 8963076 [details]
Bug 1437593 - Vendor pyOpenSSL 17.5.0;

https://reviewboard.mozilla.org/r/231946/#review243118


Code analysis found 1 defect in this patch:
 - 1 defect found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: third_party/python/pyOpenSSL/tox.ini:26
(Diff revision 2)
> +    coverage run --parallel -m OpenSSL.debug
> +    coverage run --parallel -m pytest -v {posargs}
> +
> +[testenv:py27-twistedMaster]
> +deps =
> +    # [tls,conch] syntax doesn't work here so we enumerate all dependencies.

Error: "use 'disabled=<reason>' to disable a test instead of a comment"
 [no-comment-disable]
Comment on attachment 8963069 [details]
Bug 1437593 - Vendor pipenv 11.10.1;

https://reviewboard.mozilla.org/r/231932/#review245032

Sorry for the delay, last week's work week threw a wrench in my code reviewing!

I assume you manually vendored this? If we want to update to a newer version in the future will we have to repeat that manual process, or will we be able to do something simpler? If we will need to repeat the manual process it wouldn't hurt to write it down somewhere. (Even in the commit message here would be OK.)
Attachment #8963069 - Flags: review?(ted) → review+
Comment on attachment 8963070 [details]
Bug 1437593 - Vendor ordereddict 1.1;

https://reviewboard.mozilla.org/r/231934/#review245034

I assume something depends on this, but weird. (OrderedDict only showed up in Python 2.7??)
Attachment #8963070 - Flags: review?(ted) → review+
Comment on attachment 8963071 [details]
Bug 1437593 - Vendor pathlib 1.0.1;

https://reviewboard.mozilla.org/r/231936/#review245038
Attachment #8963071 - Flags: review?(ted) → review+
Comment on attachment 8963072 [details]
Bug 1437593 - Vendor virtualenv-clone 0.3.0;

https://reviewboard.mozilla.org/r/231938/#review245044
Attachment #8963072 - Flags: review?(ted) → review+
Comment on attachment 8963073 [details]
Bug 1437593 - Vendor certifi 2018.1.18;

https://reviewboard.mozilla.org/r/231940/#review245046

::: third_party/python/certifi/LICENSE:9
(Diff revision 2)
> +
> +Certificate data from Mozilla as of: Thu Nov  3 19:04:19 2011#
> +This is a bundle of X.509 certificates of public Certificate Authorities
> +(CA). These were automatically extracted from Mozilla's root certificates
> +file (certdata.txt).  This file can be found in the mozilla source tree:
> +http://mxr.mozilla.org/mozilla/source/security/nss/lib/ckfw/builtins/certdata.txt?raw=1#

OK, this is pretty hilarious. :) We're vendoring a copy of a Python library that has a vendored copy of  the list of CA certificates from our repo.
Attachment #8963073 - Flags: review?(ted) → review+
Comment on attachment 8963075 [details]
Bug 1437593 - Vendor ndg-httpsclient 0.4.4;

https://reviewboard.mozilla.org/r/231944/#review245052
Attachment #8963075 - Flags: review?(ted) → review+
Comment on attachment 8963076 [details]
Bug 1437593 - Vendor pyOpenSSL 17.5.0;

https://reviewboard.mozilla.org/r/231946/#review245054
Attachment #8963076 - Flags: review?(ted) → review+
Comment on attachment 8963077 [details]
Bug 1437593 - Vendor cryptography 2.2.2;

https://reviewboard.mozilla.org/r/231948/#review245056
Attachment #8963077 - Flags: review?(ted) → review+
Comment on attachment 8963078 [details]
Bug 1437593 - Vendor ipaddress 1.0.19;

https://reviewboard.mozilla.org/r/231950/#review245058
Attachment #8963078 - Flags: review?(ted) → review+
Comment on attachment 8963079 [details]
Bug 1437593 - Vendor enum34 1.1.6;

https://reviewboard.mozilla.org/r/231952/#review245060
Attachment #8963079 - Flags: review?(ted) → review+
Comment on attachment 8963080 [details]
Bug 1437593 - Vendor cffi 1.11.5;

https://reviewboard.mozilla.org/r/231954/#review245062
Attachment #8963080 - Flags: review?(ted) → review+
Comment on attachment 8963081 [details]
Bug 1437593 - Vendor asn1crypto 0.24.0;

https://reviewboard.mozilla.org/r/231956/#review245064
Attachment #8963081 - Flags: review?(ted) → review+
Comment on attachment 8963083 [details]
Bug 1437593 - Vendor pycparser 2.18;

https://reviewboard.mozilla.org/r/231960/#review245068
Attachment #8963083 - Flags: review?(ted) → review+
Comment on attachment 8963084 [details]
Bug 1437593 - Add support for using Pipfiles for managing virtual environment dependencies;

https://reviewboard.mozilla.org/r/231962/#review245074

::: python/mozbuild/mozbuild/base.py:751
(Diff revision 2)
>  
>  
>      def _set_log_level(self, verbose):
>          self.log_manager.terminal_handler.setLevel(logging.INFO if not verbose else logging.DEBUG)
>  
> +    def activate_pipfile(self, path):

This method is called `activate_pipfile` but the method on `VirtualenvManager` is `activate_pipenv`. I don't know that one is better than the other, but I think they should probably be consistent. (I might slightly prefer pipenv?)

Additionally: is there a reason this method takes a path without the `Pipfile` appended? That seems somewhat unintuitive.

::: python/mozbuild/mozbuild/base.py:756
(Diff revision 2)
> +    def activate_pipfile(self, path):
> +        pipfile = os.path.join(path, 'Pipfile')
> +        if not os.path.exists(pipfile):
> +            raise Exception('Pipfile not found: %s.' % pipfile)
> +        self._activate_virtualenv()
> +        pipenv_reqs = os.path.join(os.path.abspath(os.path.dirname(__file__)), 'pipenv.txt')

It might be shorter and clearer just to explicitly say `os.path.join(self.topsrcdir, 'python/mozbuild/mozbuild/pipenv.txt')`

::: python/mozbuild/mozbuild/virtualenv.py:537
(Diff revision 2)
>  
> +    def activate_pipenv(self, pipfile):
> +        """Install a Pipfile located at path and activate environment"""
> +        pipenv = os.path.join(self.bin_path, 'pipenv')
> +        env = os.environ.copy()
> +        env.update({

It looks like there are a few other things we might want to tweak, maybe only in CI?
https://docs.pipenv.org/advanced/#configuration-with-environment-variables

We can always fix those later, though.
Attachment #8963084 - Flags: review?(ted) → review+
Comment on attachment 8963085 [details]
Bug 1437593 - Use pipenv for |mach doc| environment;

https://reviewboard.mozilla.org/r/231964/#review245084

::: tools/docs/Pipfile:4
(Diff revision 2)
> +[[source]]
> +url = "https://pypi.python.org/simple"
> +verify_ssl = true
> +name = "pypi"

Boy, that's some annoying boilerplate.
Attachment #8963085 - Flags: review?(ted) → review+
Comment on attachment 8963086 [details]
Bug 1437593 - Move initial virtual environment to _virtualenvs/init;

https://reviewboard.mozilla.org/r/231966/#review245086
Attachment #8963086 - Flags: review?(ted) → review+
This is failing on try when running setup.py install for cffi:

fatal error: ffi.h: No such file or directory

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8722d076cb1d2a4a5ed7c02afe7a5a3d548a6ed3&selectedJob=175416753

Do you have any suggestions?
Flags: needinfo?(ted)
Comment on attachment 8970715 [details]
Bug 1437593 - Exclude third_party directory from linting;

https://reviewboard.mozilla.org/r/239462/#review245296
Attachment #8970715 - Flags: review?(ted) → review+
From https://cffi.readthedocs.io/en/latest/installation.html:
"on CPython, on non-Windows platforms, you also need to install libffi-dev in order to compile CFFI itself."

So it's looking for a system libffi, which is annoying. Maybe patching Pipenv to remove this dependency for now is the right way to go until we figure out a better way to deal with this?
Flags: needinfo?(ted)
It turns out that pipenv used to only depend on requests[security] and ordereddict if the Python version was 2.6. I suspect this was an unintentional change, so I have filed an issue [0] and opened a pull request [1]. In the meantime I have patched pipenv.

:ted if you're okay with the pipenv patch then I'll mark the patches that vendor the now unused packages as obsolete and push a final series for review.
(In reply to Dave Hunt [:davehunt] ⌚️UTC+0 from comment #126)
> It turns out that pipenv used to only depend on requests[security] and
> ordereddict if the Python version was 2.6. I suspect this was an
> unintentional change, so I have filed an issue [0] and opened a pull request
> [1]. In the meantime I have patched pipenv.

I forgot to include the links:
[0] https://github.com/pypa/pipenv/issues/2055
[1] https://github.com/pypa/pipenv/pull/2056
Comment on attachment 8970881 [details]
Bug 1437593 - Patch vendored pipenv to remove unused dependencies;

https://reviewboard.mozilla.org/r/239648/#review245346

Great, this makes things a bit simpler! Hopefully we can find a good resolution with upstream. Do you want to document this patch somewhere in the source tree just so we don't accidentally revert it? I'm not incredibly worried about this scenario since you're only just introducing pipenv with these patches and it's unlikely that someone else will go updating it without your knowledge.
Attachment #8970881 - Flags: review?(ted) → review+
Attachment #8963070 - Attachment is obsolete: true
Attachment #8963075 - Attachment is obsolete: true
Attachment #8963076 - Attachment is obsolete: true
Attachment #8963077 - Attachment is obsolete: true
Attachment #8963078 - Attachment is obsolete: true
Attachment #8963079 - Attachment is obsolete: true
Attachment #8963080 - Attachment is obsolete: true
Attachment #8963081 - Attachment is obsolete: true
Attachment #8963082 - Attachment is obsolete: true
Attachment #8963083 - Attachment is obsolete: true
> Hopefully we can find a good resolution with upstream. Do you want to document this patch somewhere in the source tree just so we don't accidentally revert it?

My patch has been merged upstream, so with the next release of pipenv we shouldn't need the patch applied separately.
Flags: needinfo?(gps)
Pushed by dhunt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/828caab5032d
Allow pip requirements to be installed from vendored packages; r=gps
https://hg.mozilla.org/integration/autoland/rev/c7fa833eff1d
Vendor pipenv 11.9.0; r=ted
https://hg.mozilla.org/integration/autoland/rev/cf026c071b73
Vendor pathlib 1.0.1; r=ted
https://hg.mozilla.org/integration/autoland/rev/19b7273e375c
Vendor virtualenv-clone 0.3.0; r=ted
https://hg.mozilla.org/integration/autoland/rev/2f83246cb0b1
Vendor certifi 2018.1.18; r=ted
https://hg.mozilla.org/integration/autoland/rev/d12eec17c51c
Vendor virtualenv 15.2.0; r=gps
https://hg.mozilla.org/integration/autoland/rev/60114a93b6b1
Add support for using Pipfiles for managing virtual environment dependencies; r=ted
https://hg.mozilla.org/integration/autoland/rev/c90a0961f52a
Use pipenv for |mach doc| environment; r=ted
https://hg.mozilla.org/integration/autoland/rev/ab27411c53c2
Move initial virtual environment to _virtualenvs/init; r=ted
https://hg.mozilla.org/integration/autoland/rev/8cb34c11ad45
Exclude third_party directory from linting; r=ted
https://hg.mozilla.org/integration/autoland/rev/e4b2357330b4
Patch vendored pipenv to remove unused dependencies; r=ted
Backed out 11 changesets (bug 1437593) for SpiderMonkey failures on a CLOSED TREE 

Backout link:https://hg.mozilla.org/integration/autoland/rev/787d7f8d7a5d35675639e03cd8a59a1f91e88f00

Push with failures:https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=e4b2357330b4d5f20b306f57aa34e8f74dd45869

Log link: https://treeherder.mozilla.org/logviewer.html#?job_id=175846602&repo=autoland&lineNumber=1700

Log snippet:
[task 2018-04-26T22:59:25.665Z] make[1]: Entering directory '/builds/worker/workspace/build/src/obj-spider'
[task 2018-04-26T22:59:25.680Z] make[2]: Entering directory '/builds/worker/workspace/build/src/obj-spider/config'
[task 2018-04-26T22:59:25.680Z] backend.mk:2160: warning: overriding recipe for target '../dist/system_wrappers/pixman.h'
[task 2018-04-26T22:59:25.681Z] backend.mk:1278: warning: ignoring old recipe for target '../dist/system_wrappers/pixman.h'
[task 2018-04-26T22:59:25.683Z] make[2]: Nothing to be done for 'check'.
[task 2018-04-26T22:59:25.683Z] make[2]: Leaving directory '/builds/worker/workspace/build/src/obj-spider/config'
[task 2018-04-26T22:59:25.845Z] make[2]: Entering directory '/builds/worker/workspace/build/src/obj-spider/js/src'
[task 2018-04-26T22:59:25.845Z] (cd /builds/worker/workspace/build/src && /builds/worker/workspace/build/src/obj-spider/_virtualenvs/init/bin/python /builds/worker/workspace/build/src/config/check_js_msg_encoding.py);
[task 2018-04-26T22:59:25.920Z] Traceback (most recent call last):
[task 2018-04-26T22:59:25.920Z]   File "/builds/worker/workspace/build/src/config/check_js_msg_encoding.py", line 68, in <module>
[task 2018-04-26T22:59:25.921Z]     main()
[task 2018-04-26T22:59:25.921Z]   File "/builds/worker/workspace/build/src/config/check_js_msg_encoding.py", line 62, in main
[task 2018-04-26T22:59:25.921Z]     if not check_files():
[task 2018-04-26T22:59:25.921Z]   File "/builds/worker/workspace/build/src/config/check_js_msg_encoding.py", line 50, in check_files
[task 2018-04-26T22:59:25.921Z]     with get_repository_from_env() as repo:
[task 2018-04-26T22:59:25.921Z]   File "/builds/worker/workspace/build/src/python/mozversioncontrol/mozversioncontrol/__init__.py", line 446, in get_repository_from_env
[task 2018-04-26T22:59:25.921Z]     return get_repository_from_build_config(buildconfig)
[task 2018-04-26T22:59:25.921Z]   File "/builds/worker/workspace/build/src/python/mozversioncontrol/mozversioncontrol/__init__.py", line 422, in get_repository_from_build_config
[task 2018-04-26T22:59:25.921Z]     raise MissingConfigureInfo('could not find VCS_CHECKOUT_TYPE '
[task 2018-04-26T22:59:25.921Z] mozversioncontrol.MissingConfigureInfo: could not find VCS_CHECKOUT_TYPE in build config; check configure output and verify it could find a VCS binary
[task 2018-04-26T22:59:25.925Z] Makefile:61: recipe for target 'check-js-msg' failed
[task 2018-04-26T22:59:25.925Z] make[2]: *** [check-js-msg] Error 1
[task 2018-04-26T22:59:25.926Z] make[2]: Leaving directory '/builds/worker/workspace/build/src/obj-spider/js/src'
[task 2018-04-26T22:59:25.926Z] /builds/worker/workspace/build/src/config/recurse.mk:100: recipe for target 'js/src/check' failed
[task 2018-04-26T22:59:25.926Z] make[1]: *** [js/src/check] Error 2
[task 2018-04-26T22:59:25.926Z] make[1]: Leaving directory '/builds/worker/workspace/build/src/obj-spider'
[task 2018-04-26T22:59:25.926Z] /builds/worker/workspace/build/src/config/recurse.mk:32: recipe for target 'check' failed
[task 2018-04-26T22:59:25.926Z] make: *** [check] Error 2
Flags: needinfo?(dave.hunt)
Comment on attachment 8963086 [details]
Bug 1437593 - Move initial virtual environment to _virtualenvs/init;

Could you take another look over this patch :ted? I had to change an assumption that the virtualenv is a direct child of the objdir. Thanks!
Flags: needinfo?(dave.hunt)
Attachment #8963086 - Flags: review+ → review?(ted)
Comment on attachment 8963086 [details]
Bug 1437593 - Move initial virtual environment to _virtualenvs/init;

https://reviewboard.mozilla.org/r/231966/#review246154

::: python/mozbuild/mozbuild/base.py:154
(Diff revision 5)
>              if os.path.isfile(our_path):
>                  topsrcdir = dir_path
>                  break
>  
>          # See if we're running from a Python virtualenv that's inside an objdir.
> -        mozinfo_path = os.path.join(os.path.dirname(sys.prefix), "mozinfo.json")
> +        for dir_path in ancestors(os.path.dirname(sys.prefix)):

A bit of a quibble, but recursively traversing directories upward shouldn't be necessary here--we know we're always putting virtualenvs at `$objdir/_virtualenvs/$x` so we just need to look up two levels instead of one like the code is currently doing. I'd say just use `os.path.join(sys.prefix, '../../mozinfo.json')`.
Attachment #8963086 - Flags: review?(ted) → review+
Pushed by dhunt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/88954bcf6ca2
Allow pip requirements to be installed from vendored packages; r=gps
https://hg.mozilla.org/integration/autoland/rev/afaeac146ff8
Vendor pipenv 11.9.0; r=ted
https://hg.mozilla.org/integration/autoland/rev/ce9e3951357a
Vendor pathlib 1.0.1; r=ted
https://hg.mozilla.org/integration/autoland/rev/e024b3d775fd
Vendor virtualenv-clone 0.3.0; r=ted
https://hg.mozilla.org/integration/autoland/rev/4842e4b68d82
Vendor certifi 2018.1.18; r=ted
https://hg.mozilla.org/integration/autoland/rev/d6fe456bb39c
Vendor virtualenv 15.2.0; r=gps
https://hg.mozilla.org/integration/autoland/rev/900f32450a74
Add support for using Pipfiles for managing virtual environment dependencies; r=ted
https://hg.mozilla.org/integration/autoland/rev/dfafbe7770ad
Use pipenv for |mach doc| environment; r=ted
https://hg.mozilla.org/integration/autoland/rev/36c66615886f
Move initial virtual environment to _virtualenvs/init; r=ted
https://hg.mozilla.org/integration/autoland/rev/64c84cf90e8b
Exclude third_party directory from linting; r=ted
https://hg.mozilla.org/integration/autoland/rev/0f3dbbd73ed4
Patch vendored pipenv to remove unused dependencies; r=ted
Tier-2 Linux x64 opt failures : https://treeherder.mozilla.org/logviewer.html#?job_id=176004047&repo=autoland&lineNumber=1504

[task 2018-04-27T19:02:06.675Z] 19:02:06     INFO -  tup error: Expected to write to file 'buildid.h' from cmd 339171 but didn't
[task 2018-04-27T19:02:06.675Z] 19:02:06     INFO -  tup error: Expected to write to file 'buildid.h.pp' from cmd 339171 but didn't
[task 2018-04-27T19:02:06.675Z] 19:02:06     INFO -  *   0%    17) obj-firefox: PYTHONDONTWRITEBYTECODE=1 ./_virtualenv/bin/python -m mozbuild.action.file_generate /builds/worker/workspace/build/src/build/variables.py source_repo_header source-repo.h source-repo.h.pp
[task 2018-04-27T19:02:06.675Z] 19:02:06     INFO -  /bin/sh: 1: ./_virtualenv/bin/python: not found
[task 2018-04-27T19:02:06.675Z] 19:02:06     INFO -   *** tup errors ***
[task 2018-04-27T19:02:06.675Z] 19:02:06     INFO -   *** Command ID=339174 failed with return value 127
[task 2018-04-27T19:02:06.675Z] 19:02:06     INFO -  tup error: Expected to write to file 'source-repo.h' from cmd 339174 but didn't
[task 2018-04-27T19:02:06.675Z] 19:02:06     INFO -  tup error: Expected to write to file 'source-repo.h.pp' from cmd 339174 but didn't
[task 2018-04-27T19:02:06.675Z] 19:02:06     INFO -   *** tup: 2 jobs failed.
[task 2018-04-27T19:02:06.716Z] 19:02:06     INFO -  0 compiler warnings present.
[task 2018-04-27T19:02:06.774Z] 19:02:06    ERROR - Return code: 1
[task 2018-04-27T19:02:06.774Z] 19:02:06  WARNING - setting return code to 2
[task 2018-04-27T19:02:06.774Z] 19:02:06    FATAL - 'mach build -v' did not run successfully. Please check log for errors.
[task 2018-04-27T19:02:06.774Z] 19:02:06    FATAL - Running post_fatal callback...
[task 2018-04-27T19:02:06.775Z] 19:02:06    FATAL - Exiting -1
[task 2018-04-27T19:02:06.775Z] 19:02:06     INFO - [mozharness: 2018-04-27 19:02:06.775138Z] Finished build step (failed)
[task 2018-04-27T19:02:06.775Z] 19:02:06     INFO - Running post-run listener: _summarize
[task 2018-04-27T19:02:06.775Z] 19:02:06    ERROR - # TBPL FAILURE #
[task 2018-04-27T19:02:06.775Z] 19:02:06     INFO - [mozharness: 2018-04-27 19:02:06.775406Z] FxDesktopBuild summary:
[task 2018-04-27T19:02:06.775Z] 19:02:06    ERROR - # TBPL FAILURE #

@chmanchester , @mshal can you take a look?
Flags: needinfo?(mshal)
Flags: needinfo?(cmanchester)
Looks like we just need to update the PYTHON path in tup.py (it should be using the value from the subst anyway). Want me to send a patch here or in a separate bug?
Flags: needinfo?(mshal)
Flags: needinfo?(cmanchester)
Attachment #8971726 - Flags: review?(core-build-config-reviews) → review?(cmanchester)
Comment on attachment 8971726 [details]
Bug 1437593 - Update tup backend to support new python virtualenv;

https://reviewboard.mozilla.org/r/240492/#review246238
Attachment #8971726 - Flags: review?(cmanchester) → review+
Backed out 11 changesets (bug 1437593) for Doc linting failure. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer.html#?job_id=176096083&repo=autoland&lineNumber=699

ImportError: No module named commandinfo
[task 2018-04-28T07:39:14.372Z] /builds/worker/checkouts/gecko/docs-out/html/main/_staging/python/mach.commands.rst:18: WARNING: autodoc: failed to import module u'mach.commands.settings'; the following exception was raised:
[task 2018-04-28T07:39:14.372Z] Traceback (most recent call last):
[task 2018-04-28T07:39:14.372Z]   File "/builds/worker/checkouts/gecko/obj-x86_64-pc-linux-gnu/_virtualenvs/docs-y-qLfVwz/lib/python2.7/site-packages/sphinx/ext/autodoc.py", line 658, in import_object
[task 2018-04-28T07:39:14.372Z]     __import__(self.modname)
[task 2018-04-28T07:39:14.372Z]   File "/builds/worker/checkouts/gecko/build/mach_bootstrap.py", line 364, in __call__
[task 2018-04-28T07:39:14.372Z]     module = self._original_import(name, globals, locals, fromlist, level)
[task 2018-04-28T07:39:14.372Z] ImportError: No module named settings
[task 2018-04-28T07:39:14.372Z] /builds/worker/checkouts/gecko/python/mozbuild/mozbuild/android_version_code.py:docstring of mozbuild.android_version_code.android_version_code_v1:22: WARNING: Unexpected indentation.
[task 2018-04-28T07:39:14.373Z] /builds/worker/checkouts/gecko/python/mozbuild/mozbuild/artifacts.py:docstring of mozbuild.artifacts.ArtifactPersistLimit:12: WARNING: Unexpected indentation.
[task 2018-04-28T07:39:14.373Z] /builds/worker/checkouts/gecko/python/mozbuild/mozbuild/artifacts.py:docstring of mozbuild.artifacts.ArtifactPersistLimit:14: WARNING: Block quote ends without a blank line; unexpected unindent.
[task 2018-04-28T07:39:14.373Z] /builds/worker/checkouts/gecko/docs-out/html/main/_staging/python/mozbuild.rst:81: WARNING: autodoc: failed to import module u'mozbuild.faster_daemon'; the following exception was raised:
[task 2018-04-28T07:39:14.373Z] Traceback (most recent call last):
[task 2018-04-28T07:39:14.373Z]   File "/builds/worker/checkouts/gecko/obj-x86_64-pc-linux-gnu/_virtualenvs/docs-y-qLfVwz/lib/python2.7/site-packages/sphinx/ext/autodoc.py", line 658, in import_object
[task 2018-04-28T07:39:14.373Z]     __import__(self.modname)
[task 2018-04-28T07:39:14.373Z]   File "/builds/worker/checkouts/gecko/build/mach_bootstrap.py", line 364, in __call__
[task 2018-04-28T07:39:14.373Z]     module = self._original_import(name, globals, locals, fromlist, level)
[task 2018-04-28T07:39:14.373Z]   File "/builds/worker/checkouts/gecko/python/mozbuild/mozbuild/faster_daemon.py", line 30, in <module>
[task 2018-04-28T07:39:14.373Z]     import pywatchman
[task 2018-04-28T07:39:14.373Z]   File "/builds/worker/checkouts/gecko/build/mach_bootstrap.py", line 364, in __call__
[task 2018-04-28T07:39:14.374Z]     module = self._original_import(name, globals, locals, fromlist, level)
[task 2018-04-28T07:39:14.374Z] ImportError: No module named pywatchman

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=0f3dbbd73ed424a47974388b0cf97db84a5c756c

Backout:
https://hg.mozilla.org/integration/autoland/rev/f28612f1c310b3e2663d891a0acff3a0beafb765
Flags: needinfo?(dave.hunt)
Pushed by mshal@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7f92f91fa85d
Update tup backend to support new python virtualenv; r=chmanchester
:dluca I'm seeing many of the same errors on mozilla-central builds, such as https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&filter-searchStr=doc&selectedJob=176031105. The difference appears to be a "Resource temporarily unavailable" that my patch series intermittently introduces.

:ahal any ideas why my patch would cause this?
Flags: needinfo?(dave.hunt) → needinfo?(ahalberstadt)
https://hg.mozilla.org/mozilla-central/rev/7f92f91fa85d
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox61: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
This is not resolved, as my patch series was backed out as per comment 155.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
It looks like the sphinx.autodoc extension is having problems importing the modules its trying to generate documentation for. I guess it makes sense this patch could cause that given the |mach doc| command is now running inside of a pipenv. I have no idea how it could be intermittent though :/. We may end up needing to make sure new pipenvs are copied from the initial virtualenv (rather than starting fresh). I'm not sure if pipenv would make this easy or not.

Dave, I'd suggest dropping the |mach doc| commit from this series and moving it to a follow-up. That way we can get all this landed before too much bit rot and move forward on the python 3 work without needing to block on this |mach doc| problem.
Flags: needinfo?(ahalberstadt)
Attachment #8963071 - Attachment is obsolete: true
Attachment #8970881 - Attachment is obsolete: true
Attachment #8963069 - Flags: review+ → review?(ted)
(In reply to Andrew Halberstadt [:ahal] from comment #161)
> It looks like the sphinx.autodoc extension is having problems importing the
> modules its trying to generate documentation for. I guess it makes sense
> this patch could cause that given the |mach doc| command is now running
> inside of a pipenv. I have no idea how it could be intermittent though :/.
> We may end up needing to make sure new pipenvs are copied from the initial
> virtualenv (rather than starting fresh). I'm not sure if pipenv would make
> this easy or not.

I see the same errors without this patch series, it's just the errors appear to be ignored and the job completes "successfully". See https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&filter-searchStr=doc&selectedJob=176422405 for example.

> Dave, I'd suggest dropping the |mach doc| commit from this series and moving
> it to a follow-up. That way we can get all this landed before too much bit
> rot and move forward on the python 3 work without needing to block on this
> |mach doc| problem.

I've updated to the latest pipenv release, which has the benefit of dropping the patch for the uneccessary dependency. Unfortunately the intermittent is still present. I'll spend a little bit of time investigating this tomorrow, but if I don't get anywhere then I'll do as you suggest and split the |mach doc| changes out to another bug.
Blocks: 1458461
Attachment #8963085 - Attachment is obsolete: true
With the |mach doc| patch removed the Doc job is now passing on try. I've split this out to bug 1458461.
Comment on attachment 8972413 [details]
Bug 1437593 - Vendor pathlib2 2.3.2;

https://reviewboard.mozilla.org/r/241020/#review246940
Attachment #8972413 - Flags: review?(ted) → review+
Attachment #8972544 - Attachment is obsolete: true
Attachment #8972544 - Flags: review?(ted)
Pushed by dhunt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/961cb8855eb9
Vendor pipenv 11.10.1; r=ted
https://hg.mozilla.org/integration/autoland/rev/c235820f3d76
Vendor virtualenv-clone 0.3.0; r=ted
https://hg.mozilla.org/integration/autoland/rev/b5b8d6b5923c
Vendor certifi 2018.1.18; r=ted
https://hg.mozilla.org/integration/autoland/rev/b7e922e95a70
Vendor virtualenv 15.2.0; r=gps
https://hg.mozilla.org/integration/autoland/rev/a2e0263a144e
Add support for using Pipfiles for managing virtual environment dependencies; r=ted
https://hg.mozilla.org/integration/autoland/rev/3bcaf4828c3e
Move initial virtual environment to _virtualenvs/init; r=ted
https://hg.mozilla.org/integration/autoland/rev/ba6d53de04c0
Exclude third_party directory from linting; r=ted
https://hg.mozilla.org/integration/autoland/rev/c1a849d79497
Allow pip requirements to be installed from vendored packages; r=gps
https://hg.mozilla.org/integration/autoland/rev/9e6ec0a26b9a
Vendor pathlib2 2.3.2; r=ted
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: