Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add pip3_import to run pip with python 3. #82

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 36 additions & 14 deletions python/pip.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
# limitations under the License.
"""Import pip requirements into Bazel."""

def _pip_import_impl(repository_ctx):
def _shared_pip_import_impl(python_interpreter, repository_ctx):
"""Core implementation of pip_import."""

# Add an empty top-level BUILD file.
Expand All @@ -24,7 +24,8 @@ def _pip_import_impl(repository_ctx):

# To see the output, pass: quiet=False
result = repository_ctx.execute([
"python", repository_ctx.path(repository_ctx.attr._script),
python_interpreter, repository_ctx.path(repository_ctx.attr._script),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about make python_interpreter as a string attribute instead? I was happily using your branch, until I suddenly want to upgrade to python3.6/python3.7 and don't want to overwrite the system's python3 which is pointing to python3.5. I think this approach is better than adding extra pip36_import() and pip37_import(), etc. One original pip_import with python_interpreter attribute is enough.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good idea. I won't have time to update this in the near future though, but if you send me a PR on top of this I'll merge it into this PR.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Please take a look at #158 . Thanks!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! I like it, it's much simpler than this PR!

"--python_interpreter", python_interpreter,
"--name", repository_ctx.attr.name,
"--input", repository_ctx.path(repository_ctx.attr.requirements),
"--output", repository_ctx.path("requirements.bzl"),
Expand All @@ -34,22 +35,43 @@ def _pip_import_impl(repository_ctx):
if result.return_code:
fail("pip_import failed: %s (%s)" % (result.stdout, result.stderr))

_shared_attrs = {
"requirements": attr.label(
allow_files = True,
mandatory = True,
single_file = True,
),
"_script": attr.label(
executable = True,
default = Label("//tools:piptool.par"),
cfg = "host",
),
}

def _pip_import_impl(repository_ctx):
_shared_pip_import_impl("python", repository_ctx)

pip_import = repository_rule(
attrs = {
"requirements": attr.label(
allow_files = True,
mandatory = True,
single_file = True,
),
"_script": attr.label(
executable = True,
default = Label("//tools:piptool.par"),
cfg = "host",
),
},
attrs = _shared_attrs,
implementation = _pip_import_impl,
)

def _pip2_import_impl(repository_ctx):
_shared_pip_import_impl("python2", repository_ctx)

pip2_import = repository_rule(
attrs = _shared_attrs,
implementation = _pip2_import_impl,
)

def _pip3_import_impl(repository_ctx):
_shared_pip_import_impl("python3", repository_ctx)

pip3_import = repository_rule(
attrs = _shared_attrs,
implementation = _pip3_import_impl,
)

"""A rule for importing <code>requirements.txt</code> dependencies into Bazel.

This rule imports a <code>requirements.txt</code> file and generates a new
Expand Down
3 changes: 2 additions & 1 deletion python/whl.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def _whl_impl(repository_ctx):
"""Core implementation of whl_library."""

args = [
"python",
repository_ctx.attr.python_interpreter,
repository_ctx.path(repository_ctx.attr._script),
"--whl", repository_ctx.path(repository_ctx.attr.whl),
"--requirements", repository_ctx.attr.requirements,
Expand All @@ -35,6 +35,7 @@ def _whl_impl(repository_ctx):

whl_library = repository_rule(
attrs = {
"python_interpreter": attr.string(),
"whl": attr.label(
allow_files = True,
mandatory = True,
Expand Down
5 changes: 5 additions & 0 deletions rules_python/piptool.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,9 @@ def pip_main(argv):
parser = argparse.ArgumentParser(
description='Import Python dependencies into Bazel.')

parser.add_argument('--python_interpreter', action='store',
help=('The python python interpreter to use '))

parser.add_argument('--name', action='store',
help=('The namespace of the import.'))

Expand Down Expand Up @@ -175,10 +178,12 @@ def whl_library(wheel):
if "{repo_name}" not in native.existing_rules():
whl_library(
name = "{repo_name}",
python_interpreter = "{python_interpreter}",
whl = "@{name}//:{path}",
requirements = "@{name}//:requirements.bzl",
extras = [{extras}]
)""".format(name=args.name, repo_name=wheel.repository_name(),
python_interpreter=args.python_interpreter,
path=wheel.basename(),
extras=','.join([
'"%s"' % extra
Expand Down
Binary file modified tools/piptool.par
Binary file not shown.
Binary file modified tools/whltool.par
Binary file not shown.