indygreg / indygreg/PyOxidizer

Feature request: environmentally determined load paths

Open
#332 1 comment 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
6.2k
Forks
256
PR merge metrics
No merged PRs in 30d

Description

@indygreg closed #328 by committing 2dbe0d2, thereby adding support for Starlark's [`load`] statement. This makes it easier to share target definitions, leaving minimal configuration to downstream projects. A major difficulty remains in using the shared target definitions: we **have to hard code the locations of the shared `.bzl` files**. I'll explain with a schematic description of my intended workflow.

# Example use case

My Python library `pylib` defines `pylib/pylib.bzl`, distributing it as package data so that it's installed and available when my coworkers install `pylib` with Pip. Let's say my coworkers are developing `pyapp`, which requires `pylib`. `pyapp` will be distributed to customers as a pyoxidized `pyapp.exe` generated according to defaults set in `pylib.bzl`. According to `pylib.bzl`'s API, all they need to do is write the following in their `pyoxidizer.bzl`. (See "Current workaround" at indygreg/PyOxidizer#331 for an explanation of passing `CWD` and `BUILD_TARGET_TRIPLE`.)

```bzl
def project_settings():
return dict(
project_name="pyapp",
foo="bar",
# See "Current workaround" at https://github.com/indygreg/PyOxidizer/issues/331
BUILD_TARGET_TRIPLE=BUILD_TARGET_TRIPLE, CWD=CWD,
)
register_target("project_settings", project_settings)
load("venv/lib/python3.8/site-packages/pylib/pylib.bzl", "pylib_install")
resolve_targets()
```

The problematic line is the [`load`] statement. If one person names their virtual environment something other than `venv`, they're already in trouble. I can (and do) solve that particular issue by using [Tox](https://tox.readthedocs.io/en/latest/). But on Windows the `site-packages` directory is in `Lib\site-packages` rather than `lib/python3.X/site-pacakges`. The essential issue is that [`load`] **requires its first argument be a string literal**, so you can't compute that load path.

`pyapp` could vendor `pylib.bzl` and change the [`load`] line to
```bzl
load("vendor/pylib/pylib.bzl", "pylib_install")
```
But then obtaining and upgrading `pylib.bzl` is no longer as easy as Pip installing pylib.

# Potential solutions

A quick and easy option would be to perform environment variable expansion on the `load` path. I would suggest requiring the environment variable name to be surrounded by `$(` and `)` or `${` and `}` [as in Make](https://www.gnu.org/software/make/manual/make.html#Reference) ([Pip supports](https://pip.pypa.io/en/stable/reference/pip_install/#id10) only the latter; [Bazel supports](https://docs.bazel.build/versions/3.7.0/be/make-variables.html#use) only the former; `ProgramFiles(x86)` is a commonly defined environment variable on Windows, but both POSIX and Windows support almost any environment variable name not containing a literal `=`).

A step further is Bazel's ["Make" variables](https://docs.bazel.build/versions/3.7.0/be/make-variables.html) expansion. Bazel's own solution (though going beyond what I would need) would be to make the first argument to [`load`] be a [label](https://docs.bazel.build/versions/3.7.0/build-ref.html#labels).

**If you're down with initially just offering environment variable expansion, I can take a crack at a PR.**

[`load`]: https://github.com/bazelbuild/starlark/blob/master/spec.md#load-statements

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

The issue names no implementation files or tests. Start by locating the Starlark `load` handling and its existing test coverage, then compare the proposed environment-variable expansion forms and cross-platform path behavior. Done means an agreed syntax is implemented, documented, and covered for POSIX and Windows paths.

Written by the indexing model from the issue text.

Assessment

Tech stack
python, rust
Domain
build-system, tooling
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.