Skip to content

Commit

Permalink
warn if dependencies file does not exist (#33)
Browse files Browse the repository at this point in the history
## Description

Contributes to rapidsai/build-planning#31.

Related to #24.

This proposes the following changes:

* if the file indicated by `dependencies-file` does not exist, emit a
warning

## Notes for Reviewers

### Why add this warning?

While testing rapidsai/cudf#15245, I observed
`rapids-build-backend==0.1.1` failing to update the build dependencies
of wheels.

This showed up with errors like the following:

```text
ERROR: Could not find a version that satisfies the requirement rmm==24.6.* (from versions: 0.0.1)
```

It took me maybe an hour to figure out what was going on.... I wasn't
setting `dependencies-file` in the `[tool.rapids-build-backend]`
configuration.

`cudf` is laid out like this:

```text
cudf/
|___python/
       |_____cudf/
                  |_________pyproject.toml
       |_____dask_cudf/
                  |_________pyproject.toml
```

Not setting `dependencies-file` meant that `rapids-build-backend`
defaulted to `"dependencies.yaml"` (e.g.
`cudf/python/cudf/dependencies.yaml`).

That file doesn't exist, and so it just silently skipped all
`dependencies.yaml`-related stuff via this:


https://github.com/rapidsai/rapids-build-backend/blob/97a19504201f18cfae1864c8be99c1d73cbcc0f9/rapids_build_backend/impls.py#L170-L175

With the warning added in this PR, I would have at least had a hint from
CI logs pointing me at the potential problem.

### ~Why change the default?~

<details><summary>edited: removed (click me)</summary>

`"./dependencies.yaml"` (e.g., `dependencies.yaml` located next to the
wheel's `pyproject.toml`) is not a working default for any RAPIDS
project, as far as I know.

In my experience, most of them are either laid out like `cuml`:

```text
cuml/
|____dependencies.yaml
|____python/
         |_____pyproject.toml
```

Or like `rmm`

```text
rmm/
|____dependencies.yaml
|____python/
         |_____librmm
                    |_____pyproject.toml
         |_____rmm
                    |______pyproject.toml
```

See
https://github.com/search?q=%22%5Bproject%5D%22+org%3Arapidsai+path%3Apyproject.toml&type=code.

Changing to `../dependencies.yaml` would at least automatically work for
some projects (like `cuml`) with minimal risk of reaching up outside of
the repo to some other project's `dependencies.yaml` (which would be
more of a risk with, say, `../../dependencies.yaml`.

</details>

### Could we just make this a hard error instead of a warning?

I think we should!

I'd personally support being even stricter at this point in development
and making an existing `dependencies.yaml` file a hard requirement for
running `rapids-build-backend`, meaning:

* no default value for `dependencies-file`
* loud runtime error if the file pointed to by `dependencies-file`
doesn't exist

If we can't think of an existing case that requires running this without
a `dependencies.yaml`, then I think we should implement this stricter
interface. Making something more flexible later is always easier than
making it stricter, and this strictness right now will save some
developer time (e.g. would have saved me some time working on `cudf`
today).
  • Loading branch information
jameslamb authored May 15, 2024
1 parent dccdf0d commit 1dd6a83
Showing 1 changed file with 9 additions and 0 deletions.
9 changes: 9 additions & 0 deletions rapids_build_backend/impls.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import re
import shutil
import subprocess
import warnings
from contextlib import contextmanager
from functools import lru_cache
from importlib import import_module
Expand Down Expand Up @@ -141,11 +142,19 @@ def _edit_pyproject(config):
if not config.disable_cuda:
cuda_version_major, cuda_version_minor = _get_cuda_version()

# "dependencies.yaml" might not exist in sdists and wouldn't need to... so don't
# raise an exception if that file can't be found when this runs
try:
parsed_config = rapids_dependency_file_generator.load_config_from_file(
config.dependencies_file
)
except FileNotFoundError:
msg = (
f"File not found: '{config.dependencies_file}'. If you want "
"rapids-build-backend to consider dependencies from a dependencies file, ",
"supply an existing file via config setting 'dependencies-file'.",
)
warnings.warn(msg, stacklevel=2)
parsed_config = None

try:
Expand Down

0 comments on commit 1dd6a83

Please sign in to comment.