Set sccache timeout

[vyasr]

For local builds we don't want sccache timing out so quickly. This number matches what we use for other CI builds.

This PR also adds some documentation of how include/exclude files are handled by rapids-make-conda-env.

[trxcllnt]

See comment

[vyasr]

Builds are currently blocked on rapidsai/cuspatial#1502.

[trxcllnt]

It looks like there's a discrepancy between RAPIDS libs. Some repo wants sphinx 6 and another wants 8. Is this being tracked anywhere @vyasr?

[vyasr]

It looks like there's a discrepancy between RAPIDS libs. Some repo wants sphinx 6 and another wants 8. Is this being tracked anywhere @vyasr?

Hmm I just reran assuming that it was a transient failure and I didn't realize that this bump occurred. Let me see if I can find it.

[vyasr]

I'm not seeing it in any repo's dependencies.yaml...

I found it, it's in cuvs.

[jameslamb]

just re-triggered CI here now that the sphinx pins were removed:

• Remove sphinx pinning cugraph#4839
• Remove sphinx pinning nx-cugraph#60
• Remove sphinx pinning cuspatial#1507
• Remove sphinx pinning cuml#6195

[vyasr]

@jameslamb you'll also need to get cugraph-ops updated. I missed that one in my pass because I've already removed it from my local devcontainer setup and I forgot that we haven't quite removed it altogether yet.

[jameslamb]

Ah ok, put up https://github.com/rapidsai/cugraph-ops/pull/708

[jameslamb]

restarted all CI here now that the cugraph-ops PR is merged (thanks @bdice )

[trxcllnt]

Still seems to be failing due to incompatible breathe version:

  LibMambaUnsatisfiableError: Encountered problems while solving:
    - package breathe-4.35.0-pyhd8ed1ab_0 requires sphinx >=4.0,<6.0.0a, but none of the providers can be installed

[jameslamb]

blegh.

Looking at the logs for conda jobs:

channels:
- rapidsai
- rapidsai-nightly
- dask/label/dev
- pytorch
- pyg
- conda-forge
- nvidia
dependencies:
...
- breathe
- breathe>=4.35.0
- sphinx
- sphinx-autobuild
- sphinx-click
- sphinx-copybutton
- sphinx-markdown-tables
- sphinx-remove-toctrees
- sphinx>=8.0.0
...
- pip:
  ...
  - breathe>=4.35.0
  - nvidia-sphinx-theme

(build link)

The latest breathe on conda-forge has a ceiling of sphinx >=4.0,<7.2.

So we need one of these 2 things to happen:

• this to get merged: Allow using latest Breathe with Sphinx 8.x conda-forge/breathe-feedstock#64
• every RAPIDS repo with the breathe conda package to switch to pip: breathe (and hope that no other dependencies pull it in transitively)

Or, if we could live with not supporting docs builds in devcontainers for a bit (I have no idea if anyone uses devcontainers to build docs), we could add breathe to the list of exclusions here, until that conda-forge PR is updated?

devcontainers/features/src/rapids-build-utils/opt/rapids-build-utils/bin/make-conda-dependencies.sh

Lines 161 to 170 in 71649b5

	local conda_noinstall=($(rapids-python-pkg-names) $(rapids-python-conda-pkg-names));
	# Generate a combined conda env yaml file.
	conda-merge "${conda_env_yamls[@]}" \
	| (grep -v '^name:' || [ "$?" == "1" ]) \
	| (grep -v -P '^[ ]*?\- (\.git\@[^(main|master)])(.*?)$' || [ "$?" == "1" ]) \
	| (grep -v -P "^[ ]*?\- ($(tr -d '[:blank:]' <<< "${conda_noinstall[@]/%/ |}"))(=.*|>.*|<.*)?$" || [ "$?" == "1" ]) \
	| ( if test ${#_exclude[@]} -gt 0; then grep -E -v "${_exclude[@]}" || [ "$?" == "1" ]; else cat -; fi ) \
	| ( if test ${#_include[@]} -gt 0; then grep -E "${_include[@]}" || [ "$?" == "1" ]; else cat -; fi ) \
	;

[bdice]

Or, if we could live with not supporting docs builds in devcontainers for a bit (I have no idea if anyone uses devcontainers to build docs), we could add breathe to the list of exclusions here, until that conda-forge PR is updated?

I am okay with this for a short-term solution as long as we add comments explaining it. I do use devcontainers to build docs, but it's rare for me. We do need to get that conda-forge PR merged to fix cuVS docs builds so they're not relying on pip: breathe ...

[trxcllnt]

@jameslamb if you want to exclude breathe, we can do it as an argument at the callsite rather than inside this file. These exclusions are special, since they are either patterns or transform the dependency somehow.

The change I'd suggest is replacing this line with the one below:

devcontainers/features/src/rapids-build-utils/opt/rapids-build-utils/bin/make-conda-env.sh

Line 38 in 71649b5

rapids-make-conda-dependencies "${OPTS[@]}" > "${new_env_path}";

    rapids-make-conda-dependencies --exclude <(echo breathe) "${OPTS[@]}" > "${new_env_path}";

[jameslamb]

Thanks @trxcllnt .

It originally looked to me, based on these lines, that the docs for these --include / --exclude arguments were wrong, and that those really referred to packages, not files.

devcontainers/features/src/rapids-build-utils/opt/rapids-build-utils/bin/make-conda-dependencies.sh

Line 12 in 71649b5

# -e,--exclude <file> Path(s) to requirement files of packages to exclude.

devcontainers/features/src/rapids-build-utils/opt/rapids-build-utils/bin/make-conda-dependencies.sh

Lines 49 to 52 in 71649b5

	local -a _exclude=();
	local exc; for exc in "${exclude[@]}"; do
	_exclude+=(-f "${exc}");
	done

devcontainers/features/src/rapids-build-utils/opt/rapids-build-utils/bin/make-conda-dependencies.sh

Lines 163 to 170 in 71649b5

	# Generate a combined conda env yaml file.
	conda-merge "${conda_env_yamls[@]}" \
	| (grep -v '^name:' || [ "$?" == "1" ]) \
	| (grep -v -P '^[ ]*?\- (\.git\@[^(main|master)])(.*?)$' || [ "$?" == "1" ]) \
	| (grep -v -P "^[ ]*?\- ($(tr -d '[:blank:]' <<< "${conda_noinstall[@]/%/ |}"))(=.*|>.*|<.*)?$" || [ "$?" == "1" ]) \
	| ( if test ${#_exclude[@]} -gt 0; then grep -E -v "${_exclude[@]}" || [ "$?" == "1" ]; else cat -; fi ) \
	| ( if test ${#_include[@]} -gt 0; then grep -E "${_include[@]}" || [ "$?" == "1" ]; else cat -; fi ) \
	;

That led me to b55f366 .

But I see now that those -f being appended aren't for e.g. conda-merge, but for like git grep -E -f {file}, and I guess the <(echo breathe) thing is to provide that single pattern as a file descriptor without touching the filesystem, like git grep -E -f <(echo breathe). Pretty powerful!!!

I just pushed 7f93231 reverting my misguided changes, but including comments and expanding the docs to make it clearer that this is how that's working.

[jameslamb]

It looks to me like skipping breathe is sufficient to get CI working here again. If we merge this as-is, I'll open an issue tracking the work to revert that breathe exclusion whenever conda-forge/breathe-feedstock#64 is merged.

@trxcllnt whenever you have time, can you re-review?

[bdice]

conda-forge/breathe-feedstock#64 has been merged, so let’s try reverting the breathe-related changes.

[vyasr]

lol I just came here to say the same. I'll update.

[jameslamb]

Can we please keep the docs and comments I'd added? They would've saved me some time investigating this and I think they'll help the next person looking at something like this.

I really really don't think it's obvious that --exclude / --include can also take file descriptors, or that the those files are being treated as files of patterns for grep.

[vyasr]

Sure I'll just remove my commits and make the change Bradley suggested instead then.

We need conda-forge/breathe-feedstock#66 merged to get new Breathe packages up before CI will pass here.

[vyasr]

Great we're finally in the clear on the Breathe front here. The conda environments solved on the latest runs.

[vyasr]

I expect that the builds here will still fail though because I started merging rapids-logger changes 😮‍💨

The review was addressed