Skip to content

Commit

Permalink
fbcode_builder support for using Python virtualenv (#76)
Browse files Browse the repository at this point in the history
Summary:
Needs to be enabled by option PYTHON_VENV in the config.

shell_builder.py sets up the venv and uses it once; calling activate

For docker we set ENV; resulting in the virtual environment being present
when the resulting container is run as well as at build time. This is also cleaner
and easier to follow than re-asserting on each RUN step.

For Lego builder we need to source activate on each command as environment
will not persist between commands.

While man on the posts say it makes no sense to use virtualenv within docker
container, this method simplifies the process considerably as we can rely on the
name pip being valid and we don't need to either ensure we are root or pass the
--user flag to pip and setuptools.

Pull Request resolved: facebookarchive/LogDevice#76

Reviewed By: wez

Differential Revision: D14875633

Pulled By: calebmarchent

fbshipit-source-id: aabbcdd509d2a59fa36f8004032a052f014ce1ba
  • Loading branch information
Caleb Marchent authored and facebook-github-bot committed Apr 11, 2019
1 parent a118f8f commit 7c83600
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 3 deletions.
16 changes: 14 additions & 2 deletions build/fbcode_builder/docker_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@

from fbcode_builder import FBCodeBuilder
from shell_quoting import (
raw_shell, shell_comment, shell_join, ShellQuoted
raw_shell, shell_comment, shell_join, ShellQuoted, path_join
)
from utils import recursively_flatten_list, run_command

Expand All @@ -47,7 +47,19 @@ def setup(self):
ShellQuoted('FROM {}'.format(self.option('os_image'))),
# /bin/sh syntax is a pain
ShellQuoted('SHELL ["/bin/bash", "-c"]'),
] + self.install_debian_deps() + [self._change_user()])
] + self.install_debian_deps() + [self._change_user()]
+ [self.workdir(self.option('prefix')),
self.create_python_venv(),
self.python_venv()])

def python_venv(self):
# To both avoid calling venv activate on each RUN command AND to ensure
# it is present when the resulting container is run add to PATH
actions = []
if self.option("PYTHON_VENV", "OFF") == "ON":
actions = ShellQuoted('ENV PATH={p}:$PATH').format(
p=path_join(self.option('prefix'), "venv", "bin"))
return(actions)

def step(self, name, actions):
assert '\n' not in name, 'Name {0} would span > 1 line'.format(name)
Expand Down
15 changes: 15 additions & 0 deletions build/fbcode_builder/fbcode_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ def debian_deps(self):
'sudo',
'unzip',
'wget',
'python3-venv',
]

#
Expand Down Expand Up @@ -246,6 +247,20 @@ def install_debian_deps(self):

return self.step('Install packages for Debian-based OS', actions)

def create_python_venv(self):
action = []
if self.option("PYTHON_VENV", "OFF") == "ON":
action = self.run(ShellQuoted("python3 -m venv {p}").format(
p=path_join(self.option('prefix'), "venv")))
return(action)

def python_venv(self):
action = []
if self.option("PYTHON_VENV", "OFF") == "ON":
action = ShellQuoted("source {p}").format(
p=path_join(self.option('prefix'), "venv", "bin", "activate"))
return(action)

def debian_ccache_setup_steps(self):
return [] # It's ok to ship a renderer without ccache support.

Expand Down
2 changes: 1 addition & 1 deletion build/fbcode_builder/shell_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def step(self, name, actions):
def setup(self):
steps = [
ShellQuoted('set -exo pipefail'),
]
] + [self.create_python_venv(), self.python_venv()]
if self.has_option('ccache_dir'):
ccache_dir = self.option('ccache_dir')
steps += [
Expand Down

0 comments on commit 7c83600

Please sign in to comment.