Renamed pip_deps to container_pip_deps #1576

mmikitka · 2020-07-28T17:48:17Z

Related to issue: #1404

Steps to reproduce:

$ ./bazel version
Build label: 3.4.1
Build target: bazel-out/k8-opt/bin/src/main/java/com/google/devtools/build/lib/bazel/BazelServer_deploy.jar
Build time: Tue Jul 14 06:27:53 2020 (1594708073)
Build timestamp: 1594708073
Build timestamp as int: 1594708073

WORKSPACE

load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")

http_archive(
    name = "rules_python",
    url = "https://github.com/bazelbuild/rules_python/releases/download/0.0.2/rules_python-0.0.2.tar.gz",
    strip_prefix = "rules_python-0.0.2",
    sha256 = "b5668cde8bb6e3515057ef465a35ad712214962f0b3a314e551204266c7be90c",
)

load("@rules_python//python:repositories.bzl", "py_repositories")

py_repositories()

load("@rules_python//python:pip.bzl", "pip_repositories", "pip_import", "pip3_import")

pip_repositories()

pip3_import(
    name = "pip_deps",
    requirements = "//:requirements.txt",
)

load("@pip_deps//:requirements.bzl", "pip_install")

pip_install()

http_archive(
    name = "io_bazel_rules_docker",
    sha256 = "4521794f0fba2e20f3bf15846ab5e01d5332e587e9ce81629c7f96c793bb7036",
    strip_prefix = "rules_docker-0.14.4",
    urls = ["https://github.com/bazelbuild/rules_docker/releases/download/v0.14.4/rules_docker-v0.14.4.tar.gz"],
)

load(
    "@io_bazel_rules_docker//repositories:repositories.bzl",
    container_repositories = "repositories",
)
container_repositories()

load("@io_bazel_rules_docker//repositories:deps.bzl", container_deps = "deps")

container_deps()

load("@io_bazel_rules_docker//repositories:pip_repositories.bzl", container_pip_deps = "pip_deps")

container_pip_deps()

load(
    "@io_bazel_rules_docker//python:image.bzl",
    _py_image_repos = "repositories",
)

_py_image_repos()

BUILD

load("@io_bazel_rules_docker//python:image.bzl", "py_image")

py_image(
    name = "test",
    srcs = ["test.py"],
    main = "test.py",
)

Error message:

$ ./bazel build //...
ERROR: /home/mmikitka/.cache/bazel/_bazel_mmikitka/a29d44de5562ad5fee0bf3dd38d43719/external/pip_deps/requirements.bzl:19:9: Traceback (most recent call last):
	File "/home/mmikitka/.cache/bazel/_bazel_mmikitka/a29d44de5562ad5fee0bf3dd38d43719/external/io_bazel_rules_docker/container/BUILD", line 41
		py_library(<5 more arguments>)
	File "/home/mmikitka/.cache/bazel/_bazel_mmikitka/a29d44de5562ad5fee0bf3dd38d43719/external/io_bazel_rules_docker/container/BUILD", line 47, in py_library
		requirement("python-gflags")
	File "/home/mmikitka/.cache/bazel/_bazel_mmikitka/a29d44de5562ad5fee0bf3dd38d43719/external/pip_deps/requirements.bzl", line 19, in requirement
		fail(<1 more arguments>)
Could not find pip-provided dependency: 'python-gflags'
ERROR: /home/mmikitka/src/rules-docker-pip/BUILD:3:9: every rule of type _app_layer implicitly depends upon the target '@io_bazel_rules_docker//container:build_tar', but this target could not be found because of: Target '@io_bazel_rules_docker//container:build_tar' contains an error and its package is in error
ERROR: /home/mmikitka/src/rules-docker-pip/BUILD:3:9: every rule of type _app_layer implicitly depends upon the target '@io_bazel_rules_docker//container:incremental_load_template', but this target could not be found because of: Target '@io_bazel_rules_docker//container:incremental_load_template' contains an error and its package is in error
ERROR: Analysis of target '//:test' failed; build aborted: Analysis failed
INFO: Elapsed time: 64.466s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (33 packages loaded, 363 targets configured)

mmikitka · 2020-07-28T18:42:37Z

/assign @alex1545

mmikitka · 2020-07-28T18:43:28Z

@alex1545 I am seeing the following errors in buildkite, and I don't know what I did to cause them:

==================== Test output for //container:image_test:
.........................F.........................
======================================================================
FAIL: test_py_image_complex (__main__.ImageTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/var/lib/buildkite-agent/.cache/bazel/_bazel_buildkite-agent/ec321eb2cc2d0f8f91b676b6d4c66c29/sandbox/linux-sandbox/857/execroot/io_bazel_rules_docker/bazel-out/k8-py2-fastbuild/bin/container/image_test.runfiles/io_bazel_rules_docker/container/image_test.py", line 707, in test_py_image_complex
    '/app/testdata/py_image_complex.binary.runfiles/container_pip_deps_pypi__addict_2_1_2/addict-2.1.2.dist-info/top_level.txt',
  File "/var/lib/buildkite-agent/.cache/bazel/_bazel_buildkite-agent/ec321eb2cc2d0f8f91b676b6d4c66c29/sandbox/linux-sandbox/857/execroot/io_bazel_rules_docker/bazel-out/k8-py2-fastbuild/bin/container/image_test.runfiles/io_bazel_rules_docker/container/image_test.py", line 54, in assertLayerNContains
    self.assertTarballContains(layer, paths)
  File "/var/lib/buildkite-agent/.cache/bazel/_bazel_buildkite-agent/ec321eb2cc2d0f8f91b676b6d4c66c29/sandbox/linux-sandbox/857/execroot/io_bazel_rules_docker/bazel-out/k8-py2-fastbuild/bin/container/image_test.runfiles/io_bazel_rules_docker/container/image_test.py", line 49, in assertTarballContains
    self.assertEqual(paths, tar.getnames())
AssertionError: Lists differ: ['.', '/app', '/app/testdata',... != ['.', '/app', '/app/testdata',...
 
First differing element 8:
'/app/testdata/py_image_complex.binary.runfiles/container_pip_deps_pypi__addict_2_1_2/addict-2.1.2.dist-info'
'/app/testdata/py_image_complex.binary.runfiles/container_pip_deps_pypi__addict_2_1_2/addict-2.1.2.dist-info/'
 
  ['.',
   '/app',
   '/app/testdata',
   '/app/testdata/py_image_complex.binary.runfiles',
   '/app/testdata/py_image_complex.binary.runfiles/container_pip_deps_pypi__addict_2_1_2',
   '/app/testdata/py_image_complex.binary.runfiles/container_pip_deps_pypi__addict_2_1_2/addict',
   '/app/testdata/py_image_complex.binary.runfiles/container_pip_deps_pypi__addict_2_1_2/addict/__init__.py',
   '/app/testdata/py_image_complex.binary.runfiles/container_pip_deps_pypi__addict_2_1_2/addict/addict.py',
-  '/app/testdata/py_image_complex.binary.runfiles/container_pip_deps_pypi__addict_2_1_2/addict-2.1.2.dist-info',
+  '/app/testdata/py_image_complex.binary.runfiles/container_pip_deps_pypi__addict_2_1_2/addict-2.1.2.dist-info/',
?                                                                                                              +
 
   '/app/testdata/py_image_complex.binary.runfiles/container_pip_deps_pypi__addict_2_1_2/addict-2.1.2.dist-info/DESCRIPTION.rst',
   '/app/testdata/py_image_complex.binary.runfiles/container_pip_deps_pypi__addict_2_1_2/addict-2.1.2.dist-info/METADATA',
   '/app/testdata/py_image_complex.binary.runfiles/container_pip_deps_pypi__addict_2_1_2/addict-2.1.2.dist-info/RECORD',
   '/app/testdata/py_image_complex.binary.runfiles/container_pip_deps_pypi__addict_2_1_2/addict-2.1.2.dist-info/WHEEL',
   '/app/testdata/py_image_complex.binary.runfiles/container_pip_deps_pypi__addict_2_1_2/addict-2.1.2.dist-info/metadata.json',
   '/app/testdata/py_image_complex.binary.runfiles/container_pip_deps_pypi__addict_2_1_2/addict-2.1.2.dist-info/top_level.txt']

pvcnt · 2020-08-04T18:04:04Z

I was about to submit a similar PR, thank you for doing this!

smukherj1 · 2020-08-04T20:56:16Z

Thanks for making this PR! Not really sure where that extra slash at the end of /app/testdata/py_image_complex.binary.runfiles/container_pip_deps_pypi__addict_2_1_2/addict-2.1.2.dist-info/ is coming from. I'm not sure if repo named pip_deps mean something special to the Bazel python rules and whether this renaming will break things. @brandjon any idea if renaming the pip_deps repository is advisable? The repository naming conflict issue for pip_deps likely affects other users of pip_deps as well.

aaliddell · 2020-10-05T11:11:26Z

Could this be named under the rules_docker 'namespace', rather than taking another name that is non-specific and still liable to clashes? i.e rules_docker_pip_deps. This is how it is done in other repos to prevent similar clashes and since the user doesn't typically see this name it doesn't need to be kept artificially short.

Regarding the test failures, the name pip_deps does not mean anything special; so using any other name is fine so long as you are consistent.

smukherj1 · 2020-10-06T19:15:35Z

Ok please update the image_test.py file with the new expected name that includes the slash. I do like the idea of naming the deps with rules_docker in the name. Maybe call it io_bazel_rules_docker_pip_deps because io_bazel_rules_docker is the name of the rules_docker Bazel workspace?

mmikitka · 2020-10-14T17:21:06Z

@aaliddell and @smukherj1 I renamed container_pip_deps to io_bazel_rules_docker_pip_deps and fixed the unit test error.

Anything else required to complete the PR?

smukherj1

Thanks a lot!

k8s-ci-robot · 2020-10-14T20:11:50Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mmikitka, smukherj1

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [smukherj1]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

smukherj1 · 2020-10-14T20:11:52Z

/gcbrun

mikedanese · 2020-10-15T02:09:53Z

README.md

@@ -167,9 +167,9 @@ load("@io_bazel_rules_docker//repositories:deps.bzl", container_deps = "deps")

 container_deps()

-load("@io_bazel_rules_docker//repositories:pip_repositories.bzl", "pip_deps")
+load("@io_bazel_rules_docker//repositories:pip_repositories.bzl", "io_bazel_rules_docker_pip_deps")


This snippet is using rules_docker-0.14.4 which doesn't have your rename so copy and pasting now fails.

ERROR: Traceback (most recent call last): File "/usr/local/google/home/mikedanese/code/cloud-provider-gcp/WORKSPACE", line 70, column 89, in <toplevel> load("@io_bazel_rules_docker//repositories:pip_repositories.bzl", container_pip_deps = "io_bazel_rules_docker_pip_deps") Error: file '@io_bazel_rules_docker//repositories:pip_repositories.bzl' does not contain symbol 'io_bazel_rules_docker_pip_deps' ERROR: error loading package 'external': Package 'external' contains errors

@smukherj1 and @aaliddell Tagging another release seems like a good idea, and the README.md could be updated at the same time.

Ack. Made a note of this and will get around to this in the next week or so hopefully.

friendly ping :)

Not sure if this is in your attention-set anymore now that it merged

In particular rules_docker started failing. there have been several fixes for gflags being removed, I think we need a release with bazelbuild/rules_docker#1576 Signed-off-by: Elliot Murphy <statik@users.noreply.github.com>

Renamed pip_deps to container_pip_deps

b01189b

mmikitka requested review from alex1545 and smukherj1 as code owners July 28, 2020 17:48

googlebot added the cla: yes label Jul 28, 2020

k8s-ci-robot added the size/L label Jul 28, 2020

mmikitka mentioned this pull request Jul 28, 2020

Clashing of Python and Docker workspace rules #1404

Closed

mmikitka added 3 commits July 28, 2020 13:54

Fixed buildifier lint errors

0400ae6

Attempt to fix test failure

551b28b

Reverted

6c86115

k8s-ci-robot assigned alex1545 Jul 28, 2020

mmikitka added 5 commits October 14, 2020 12:29

Renamed container_pip_deps to io_bazel_rules_docker_pip_deps

d646e0c

Fixed buildifier error

4e005c9

Merge remote-tracking branch 'upstream/master' into rename-pip-deps

b5f54db

Fixed lint error

ffa8b38

Fix image_test error

de05a07

smukherj1 approved these changes Oct 14, 2020

View reviewed changes

k8s-ci-robot assigned smukherj1 Oct 14, 2020

k8s-ci-robot added the lgtm label Oct 14, 2020

k8s-ci-robot added the approved label Oct 14, 2020

rbe-toolchains-pr-bot merged commit 7c705c2 into bazelbuild:master Oct 14, 2020

mikedanese reviewed Oct 15, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Renamed pip_deps to container_pip_deps #1576

Renamed pip_deps to container_pip_deps #1576

mmikitka commented Jul 28, 2020

mmikitka commented Jul 28, 2020

mmikitka commented Jul 28, 2020 •

edited

Loading

pvcnt commented Aug 4, 2020

smukherj1 commented Aug 4, 2020

aaliddell commented Oct 5, 2020

smukherj1 commented Oct 6, 2020

mmikitka commented Oct 14, 2020

smukherj1 left a comment

k8s-ci-robot commented Oct 14, 2020

smukherj1 commented Oct 14, 2020

mikedanese Oct 15, 2020

mmikitka Oct 15, 2020

smukherj1 Oct 16, 2020

dmattia Oct 30, 2020 •

edited

Loading

Renamed pip_deps to container_pip_deps #1576

Renamed pip_deps to container_pip_deps #1576

Conversation

mmikitka commented Jul 28, 2020

mmikitka commented Jul 28, 2020

mmikitka commented Jul 28, 2020 • edited Loading

pvcnt commented Aug 4, 2020

smukherj1 commented Aug 4, 2020

aaliddell commented Oct 5, 2020

smukherj1 commented Oct 6, 2020

mmikitka commented Oct 14, 2020

smukherj1 left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Oct 14, 2020

smukherj1 commented Oct 14, 2020

mikedanese Oct 15, 2020

Choose a reason for hiding this comment

mmikitka Oct 15, 2020

Choose a reason for hiding this comment

smukherj1 Oct 16, 2020

Choose a reason for hiding this comment

dmattia Oct 30, 2020 • edited Loading

Choose a reason for hiding this comment

mmikitka commented Jul 28, 2020 •

edited

Loading

dmattia Oct 30, 2020 •

edited

Loading