[Bug]: building chapel-py-venv removes sphinx-build binary

[mppf]

I'm observing a problem where make chplcheck-venv removes parts of the chpl-venv used for chpldoc.

$ make docs
$ ls third-party/chpl-venv/install/chpldeps/bin/
   # output includes sphinx-build
$ make chplcheck
$ ls third-party/chpl-venv/install/chpldeps/bin/
   # no sphinx-build

Here is a shorter way to reproduce it:

$ rm -Rf third-party/chpl-venv/{build,install}
$ make third-party-chpldoc-venv
$ ls third-party/chpl-venv/install/chpldeps/bin/
  # output includes sphinx-build
$ make chapel-py-venv
$ ls third-party/chpl-venv/install/chpldeps/bin/
  # no sphinx-build

A side issue is that make chapel-py-venv interleaved with make seems to always rebuild libChplFrontendShared.so.

It is unclear to me if this behavior has changed recently or if I'm just running into it now.

[mppf]

@DanilaFe - do you know why this might be?

[DanilaFe]

No idea, this is a surprise. I don't think I have any explicitly deletes / uninstalls in that code. I'll have to check it out.

[bradcray]

This is a wild stab in the dark, but IIRC, there is an aspect of Makefiles where, it it builds something that it considers an intermediate file, it will delete it when it's done with it. Could that be the case here? Doing a quick Google to make sure I didn't hallucinate that memory, I'm finding https://unix.stackexchange.com/questions/517190/what-causes-make-to-delete-intermediate-files

[jabraham17]

I think this is an instance of pypa/pip#8063. --upgrade and --target do not work well, and --target has a bug.

As suggested by that issue, I tried just removing --upgrade from the chapel-py-venv command and got it to work.

Specifically, the following works.

1. Apply this patch

diff --git a/third-party/chpl-venv/Makefile b/third-party/chpl-venv/Makefile
index b5b8694ff0..bfedfa528f 100644
--- a/third-party/chpl-venv/Makefile
+++ b/third-party/chpl-venv/Makefile
@@ -108,7 +108,7 @@ chapel-py-venv: FORCE $(CHPL_VENV_VIRTUALENV_DIR_OK)
        @# directory structure at $(CHPL_VENV_CHPLDEPS)
        export PATH="$(CHPL_VENV_VIRTUALENV_BIN):$$PATH" && \
        export VIRTUAL_ENV=$(CHPL_VENV_VIRTUALENV_DIR) && \
-       CHPL_HOME=$(CHPL_MAKE_HOME) $(PIP) install --upgrade $(CHPL_PIP_INSTALL_PARAMS) $(LOCAL_PIP_FLAGS) \
+       CHPL_HOME=$(CHPL_MAKE_HOME) $(PIP) install $(CHPL_PIP_INSTALL_PARAMS) $(LOCAL_PIP_FLAGS) \
          --target $(CHPL_VENV_CHPLDEPS) \
          $(CHPL_MAKE_HOME)/tools/chapel-py \
          -r $(CHPL_VENV_CHAPEL_PY_REQUIREMENTS_FILE

2. Make the normal chpldeps venv ((cd third-party/chpl-venv/ && make clobber && make))
3. Build chapel-py-venv (make chapel-py-venv)

This gets some warnings, but at least works and doesn't clobber bin

WARNING: Target directory ...../chapel/third-party/chpl-venv/install/chpldeps/__pycache__ already exists. Specify --upgrade to force replacement.
WARNING: Target directory...../chapel/third-party/chpl-venv/install/chpldeps/packaging already exists. Specify --upgrade to force replacement.
WARNING: Target directory ...../chapel/third-party/chpl-venv/install/chpldeps/packaging-24.0.dist-info already exists. Specify --upgrade to force replacement.
WARNING: Target directory ...../chapel/third-party/chpl-venv/install/chpldeps/bin already exists. Specify --upgrade to force replacement.

Ideally, I think we should use proper venvs instead of target installs.

[DanilaFe]

I thought we moved away from proper venvs for a reason. I think I heard that from @mppf, maybe he recalls the reason a bit better.

[jabraham17]

Note, as of #25025, this will no longer occur. Instead, building the CLS test venv (either with CHPL_ALWAYS_BUILD_CHAPEL_PY_TEST set or make test-cls) will clobber sphinx-build.

So while this is still a problem that building the CLS test venv breaks chpldoc, it should not affect most users who are only building the tools themselves