desiInstall pip warnings resulting in CRITICAL level errors

[sbailey]

When using pip 21.2.1 and python 3.9.6 in a test conda environment, pip prints a deprecation message that desiInstall interprets as a CRITICAL-level error and exists with a non-zero exit code and leaves the git directory behind. Investigate whether we need to do anything about this warning, and update desiInstall so that it doesn't treat it as a fatal error.

Steps to reproduce at NERSC:

module load python/3.8-anaconda-2020.11
conda create --name testpip --yes python=3.9
source activate testpip
conda install requests numpy astropy --yes

pip install git+https://github.com/desihub/desiutil.git
export LANG=en_US.utf8
desiInstall -r $SCRATCH/temp desispec master

Results in an exit code 1 and the following messages:

WARNING:install.py:183:get_options:2021-07-26T22:19:16: The environment variable DESICONDA is not set!
CRITICAL:install.py:854:install:2021-07-26T22:19:24: Error during installation:   DEPRECATION: A future pip version will change local packages to be built in-place without first copying to a temporary directory. We recommend you use --use-feature=in-tree-build to test your packages with this new behavior before it becomes the default.
   pip 21.3 will remove support for this functionality. You can find discussion regarding this at https://github.com/pypa/pip/issues/7555.

[weaverba137]

This warning can be ignored. I am curious though why you're not using the desiBoostrap script, which is intended to install desiutil and set up the modules environment.

[sbailey]

Thanks for confirming that this can be ignored. Please update desiInstall so that it is ignored (or if there are backwards compatible options to pip to avoid the message in the first place, even better).

More broadly, it appears that desiInstall treats any output (or any stderr output?) from the pip install as a CRITICAL-level failure, even if the pip install was successful and there is nothing actually wrong with the installation. We also tripped on this with fiberassign compiler warning messages being treated as CRITICAL failures. I suggest updating that so that desiInstall only treats them as CRITICAL if pip install exists with a non-zero exit code, otherwise just pass them forward as INFO. i.e. use the exit code of pip to determine success/failure of the install rather than using existence of stderr messages. Any concerns about doing that?

Perhaps stating the obvious, but to be clear: if pip install succeeds (exitcode=0) but desiInstall hits some other problem like directory permissions when creating the module file, that would also be a CRITICAL-level non-zero-exit for desiInstall as well.

Regarding desiBootstrap: I'm trying to simplify our procedures for installing a new software release after a new desiconda base installation. Although desiBootstap works, it requires instructions that I have have to look up every time (https://desi.lbl.gov/trac/wiki/Pipeline/Install#bootstrappingtheenvironment) so I was experimenting with an alternate method that I can remember:

• pip install desiutil (to get desiInstall for installing modules-based versions of everything)
• desiInstall everything including desiutil to get code+module files for every product
• pip uninstall desiutil (to get it out of the base environment, since we now have a modules-based version to use)

I'm not sure if that is the best approach long term, but it was sufficient and simpler for documenting a reproducer case for this PR. I'm pretty sure that is unrelated to the warning-treated-as-critical issue reported here, but if that difference in desiInstall usage is the cause, then that would be a reason not to use this approach (or otherwise refine it).

[weaverba137]

Agreed. However, it is not the case that any output is treated as a failure. The output is filtered and the filter needs to be updated, that's all. Unexpected build errors do need to be passed to the user. Expected errors and warnings can be filtered.

The whole point of documentation is that you can look it up, rather than being forced to remember it!

[sbailey]

Thanks. I agree that unexpected output should be passed on to the user, but I'm also suggesting that unexpected output with a return code of 0 should be INFO-logged and treated as a success (return code 0) instead of CRITICAL-logged with failure return code 1. OTOH if you specifically know of cases where a pip install return code isn't a reliable test of whether the installation actually succeeded, then I would be less inclined to trust it over parsing output (but I would still consider whether that should be special-cased, vs. treating any unexpected output as necessarily an installation failure).