indygreg / indygreg/PyOxidizer
Global constants not available in Starlark files that pyoxidizer.bzl loads
- 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](https://github.com/bazelbuild/starlark/blob/master/spec.md#load-statements). This makes it easier to share target definitions, leaving minimal configuration to downstream projects. Ideally loaded modules would have access to the same global symbols as the main `pyoxidizer.bzl` that `pyoxidizer build` is executing. Currently, [global constants](https://pyoxidizer.readthedocs.io/en/latest/config_globals.html#global-constants) don't work, even though [global functions](https://pyoxidizer.readthedocs.io/en/latest/config_globals.html#global-functions) do. Per https://github.com/indygreg/PyOxidizer/issues/328#issuecomment-731657497 I'm opening this ticket for further discussion.
Current installed version of PyOxidizer: 6875c5fd1f5c6df3c1a24734868393a1ae19cb57
```console
$ pyoxidizer --version
PyOxidizer 0.11.0-pre
commit: 6875c5fd1f5c6df3c1a24734868393a1ae19cb57
source: /Users/wks/.cargo/git/checkouts/pyoxidizer-04b02466db45c615/6875c5f
pyembed crate location: path = "/Users/wks/.cargo/git/checkouts/pyoxidizer-04b02466db45c615/6875c5f/pyembed"
```
# Failing Test Case
Place the following snippets into an otherwise empty directory.
**`pyoxidizer.bzl`**
```bzl
load("helper.bzl", "test")
resolve_targets()
```
**`helper.bzl`**
```bzl
def test():
cwd = CWD
triple = BUILD_TARGET_TRIPLE
config_path = CONFIG_PATH
print(cwd, triple, config_path)
register_target("test", test, default=True)
```
I would expect `pyoxidizer build` to print the values of the global constants. This is the **`pyoxidizer build` output**.
```console
$ pyoxidizer build
resolving 1 targets
resolving target test
error[CM01]: Variable 'CWD' not found
--> helper.bzl:2:11
|
2 | cwd = CWD
| ^^^ Variable was not found
error: Variable 'CWD' not found
```
Commenting out the first and the second lines of `test()` show that each of the three global constants are unavailable.
# Current Workaround
The following modified files work as I was hoping the example above would.
**`pyoxidizer.bzl`**
```bzl
def project_settings():
return dict(
CWD = CWD,
BUILD_TARGET_TRIPLE = BUILD_TARGET_TRIPLE,
CONFIG_PATH = CONFIG_PATH,
)
register_target("project_settings", project_settings)
load("helper.bzl", "test")
resolve_targets()
```
**`helper.bzl`**
```bzl
def test(project_settings):
cwd = project_settings["CWD"]
triple = project_settings["BUILD_TARGET_TRIPLE"]
config_path = project_settings["CONFIG_PATH"]
print(cwd, triple, config_path)
register_target("test", test, depends=["project_settings"], default=True)
```
**Output from `pyoxidizer build`**
```console
$ pyoxidizer build
resolving 1 targets
resolving target test
resolving target project_settings
/Users/wks/Documents/pyox-test/. x86_64-apple-darwin ./pyoxidizer.bzl
error: NoneType does not implement build()
```
# Discussion
I'll respond to points from https://github.com/indygreg/PyOxidizer/issues/328#issuecomment-731657497, intentionally out of order.
> However, any variable mutations within the `load()`ed file will not be reflected outside that file's `Environment`.
That's good. I think other behavior would surprise Python programmers. Certainly it would surprise me.
> Furthermore, if we attempt to clone and pass the child environment containing the variables to the loaded file's context, we get an error due to mutably borrowing a frozen variable. I'm unsure what is up here. It is possible we might be able to work around it.
The main `pyoxidizer.bzl` environment appears to be created here:
https://github.com/indygreg/PyOxidizer/blob/6875c5fd1f5c6df3c1a24734868393a1ae19cb57/pyoxidizer/src/starlark/eval.rs#L64-L79
> The `Environment` (which is the thing containing variables) is different for the file calling `load()` and the file be loaded. These can't be the same exact object: the best we can do here is clone the current environment: that should make the variables from the calling file available to the file being loaded.
Correct me if I'm wrong—the cloning appears to happen here (omitting the error handling):
https://github.com/indygreg/PyOxidizer/blob/6875c5fd1f5c6df3c1a24734868393a1ae19cb57/pyoxidizer/src/starlark/eval.rs#L99-L111
Perhaps one solution would be to apply [`crate::starlark::env::populate_environment`](https://github.com/indygreg/PyOxidizer/blob/6875c5fd1f5c6df3c1a24734868393a1ae19cb57/pyoxidizer/src/starlark/env.rs#L185-L232) at [line 73](https://github.com/indygreg/PyOxidizer/blob/6875c5fd1f5c6df3c1a24734868393a1ae19cb57/pyoxidizer/src/starlark/eval.rs#L73) above to `file_loader_env`? Or would we need to apply to `file_loader_env` just the few lines of `populate_environment` shown below?
https://github.com/indygreg/PyOxidizer/blob/6875c5fd1f5c6df3c1a24734868393a1ae19cb57/pyoxidizer/src/starlark/env.rs#L211-L219
Contributor guide
No contributing guide indexed for this repository
Research direction
Start in pyoxidizer/src/starlark/eval.rs around environment creation and loading, then read populate_environment in pyoxidizer/src/starlark/env.rs. Reproduce the issue with the supplied pyoxidizer.bzl and helper.bzl files using pyoxidizer build. Done means the loaded helper can access CWD, BUILD_TARGET_TRIPLE, and CONFIG_PATH and the build prints their values without the missing-variable error.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- build-system, tooling
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 45/100