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

[Bug]: venv requirements file path wrong #248

Closed
matzipan opened this issue Jan 31, 2024 · 2 comments · Fixed by #233
Closed

[Bug]: venv requirements file path wrong #248

matzipan opened this issue Jan 31, 2024 · 2 comments · Fixed by #233
Labels
bug Something isn't working untriaged Requires traige

Comments

@matzipan
Copy link

matzipan commented Jan 31, 2024

What happened?

bazel run :test.venv
....
INFO: Running command line: bazel-bin/test.venv_create_venv.sh
WARNING: Requirement 'external/deps_attrs/attrs-21.4.0-py2.py3-none-any.whl' looks like a filename, but the file does not exist
ERROR: Could not install packages due to an OSError: [Errno 2] No such file or directory: '/bazel_cache/output_user_root/61da6a75bcd751b8ee01d9c498363d41/execroot/demo/bazel-out/k8-fastbuild/bin/test.venv_create_venv.sh.runfiles/demo/external/deps_attrs/attrs-21.4.0-py2.py3-none-any.whl'

Version

Development (host) and target OS/architectures: linux, linux

Output of bazel --version: 6.3.2

Version of the Aspect rules, or other relevant rules from your
WORKSPACE or MODULE.bazel file: 43beacc

Language(s) and/or frameworks involved: python

How to reproduce

Workspace file:

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

py_repositories()

python_register_toolchains(
    name = "python39",
    python_version = "3.9",
)

load("@rules_python//python:pip.bzl", "package_annotation", "pip_parse")

PY_WHEEL_RULE_CONTENT = """\
load("@aspect_rules_py//py:defs.bzl", "py_wheel")
py_wheel(
    name = "wheel",
    src = ":whl",
)
"""

PACKAGES = [
    "attrs",
]

ANNOTATIONS = {
    pkg: package_annotation(additive_build_content = PY_WHEEL_RULE_CONTENT)
    for pkg in PACKAGES
}

local_repository(
    name = "aspect_rules_py",
    path = "/workspaces/demo/rules_py",
)

pip_parse(
    name = "deps",
    annotations = ANNOTATIONS,
    python_interpreter_target = "@python39_host//:python",
    requirements_lock = "//:requirements.txt",
)

load("@deps//:requirements.bzl", "install_deps")

install_deps()

load("@aspect_rules_py//py:repositories.bzl", "rules_py_dependencies")

rules_py_dependencies()

Build file:

load("@aspect_rules_py//py:defs.bzl", "py_binary")

py_binary(
    name = "test",
    srcs = ["bla.py"],
    deps = [
        "@deps_attrs//:wheel",
    ],
)

Any other information?

WHL_REQUIREMENTS_FILE contains the following:

external/deps_attrs/attrs-21.4.0-py2.py3-none-any.whl

Obviously, if pip will search this path, it will not find it because it didn't go through runfiles/rlocation. PIP_FIND_LINKS will not change anything because it's the exact same list.

The following patch seems to resolve the issue. We just pass the fully rlocationed paths via requirements.txt:

diff --git a/py/private/venv/venv.tmpl.sh b/py/private/venv/venv.tmpl.sh
index 64c2725..f0f4df0 100644
--- a/py/private/venv/venv.tmpl.sh
+++ b/py/private/venv/venv.tmpl.sh
@@ -89,8 +89,21 @@ if [ "$INSTALL_WHEELS" = true ]; then
   # Call to pip to "install" our dependencies. The `find-links` section in the config points to the external downloaded wheels,
   # while `--no-index` ensures we don't reach out to PyPi
   # We may hit command line length limits if passing a large number of find-links flags, so set them on the PIP_FIND_LINKS env var
-  PIP_FIND_LINKS=$(tr '\n' ' ' < "${WHL_REQUIREMENTS_FILE}")
-  export PIP_FIND_LINKS
+
+  RLOCATION_REQUIREMENTS_FILE="rlocation_requirements.txt"
+  if [ -f $RLOCATION_REQUIREMENTS_FILE ] ; then
+    rm $RLOCATION_REQUIREMENTS_FILE
+  fi
+  touch $RLOCATION_REQUIREMENTS_FILE
+
+  for WHL_PATH in $(cat "${WHL_REQUIREMENTS_FILE}")
+  do
+    WHL_PATH_FOR_RLOCATION="${WHL_PATH#external/}"
+
+    WHL_RLOCATION=$(rlocation $WHL_PATH_FOR_RLOCATION)
+
+    echo $WHL_RLOCATION >> $RLOCATION_REQUIREMENTS_FILE
+  done
 
   PIP_FLAGS=(
     "--quiet"
@@ -105,7 +118,7 @@ if [ "$INSTALL_WHEELS" = true ]; then
     "--no-index"
   )
 
-  ${VPIP} install "${PIP_FLAGS[@]}" -r "${WHL_REQUIREMENTS_FILE}"
+  ${VPIP} install "${PIP_FLAGS[@]}" -r "$RLOCATION_REQUIREMENTS_FILE"
 
   unset PIP_FIND_LINKS
 fi
@matzipan matzipan added the bug Something isn't working label Jan 31, 2024
@github-actions github-actions bot added the untriaged Requires traige label Jan 31, 2024
@matzipan
Copy link
Author

matzipan commented Jan 31, 2024

With the patch above bazel run :test or bazel test :test does not work anymore.

This patch fixes it:

diff --git a/py/private/venv/venv.tmpl.sh b/py/private/venv/venv.tmpl.sh
index 64c2725..ade8943 100644
--- a/py/private/venv/venv.tmpl.sh
+++ b/py/private/venv/venv.tmpl.sh
@@ -89,8 +89,25 @@ if [ "$INSTALL_WHEELS" = true ]; then
   # Call to pip to "install" our dependencies. The `find-links` section in the config points to the external downloaded wheels,
   # while `--no-index` ensures we don't reach out to PyPi
   # We may hit command line length limits if passing a large number of find-links flags, so set them on the PIP_FIND_LINKS env var
-  PIP_FIND_LINKS=$(tr '\n' ' ' < "${WHL_REQUIREMENTS_FILE}")
-  export PIP_FIND_LINKS
+
+  RLOCATION_REQUIREMENTS_FILE="rlocation_requirements.txt"
+  if [ -f $RLOCATION_REQUIREMENTS_FILE ] ; then
+    rm $RLOCATION_REQUIREMENTS_FILE
+  fi
+  touch $RLOCATION_REQUIREMENTS_FILE
+
+  if [ "$USE_MANIFEST_PATH" = false ]; then
+    cat "${WHL_REQUIREMENTS_FILE}" > $RLOCATION_REQUIREMENTS_FILE
+  else
+    for WHL_PATH in $(cat "${WHL_REQUIREMENTS_FILE}")
+    do
+      WHL_PATH_FOR_RLOCATION="${WHL_PATH#*/}"
+
+      WHL_RLOCATION=$(rlocation $WHL_PATH_FOR_RLOCATION)
+
+      echo $WHL_RLOCATION >> $RLOCATION_REQUIREMENTS_FILE
+    done
+  fi
 
   PIP_FLAGS=(
     "--quiet"
@@ -105,7 +122,7 @@ if [ "$INSTALL_WHEELS" = true ]; then
     "--no-index"
   )
 
-  ${VPIP} install "${PIP_FLAGS[@]}" -r "${WHL_REQUIREMENTS_FILE}"
+  ${VPIP} install "${PIP_FLAGS[@]}" -r $RLOCATION_REQUIREMENTS_FILE
 
   unset PIP_FIND_LINKS
 fi

@mattem
Copy link
Collaborator

mattem commented Feb 7, 2024

Will be fixed by #233

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working untriaged Requires traige
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants