AcademySoftwareFoundation / rez

An integrated package configuration, build and deployment system for software
https://rez.readthedocs.io
Apache License 2.0
921 stars 329 forks source link

Add more logic in the shell detection code to handle a case where the parent PID is zero #1735

Closed JeanChristopheMorinPerso closed 2 months ago

JeanChristopheMorinPerso commented 2 months ago

Fixes #1732

Add more logic in the shell detection code to handle a case where the parent PID is zero. This can happen when run from within a docker container with a non-interactive shell.

This was not necessarily a hard error. rez would just print an annoying error.

Before:

$ docker run --rm -it rocky_rez rez-env python -- python --version
os.getpid() 0
error: process ID out of range

Usage:
 ps [options]

 Try 'ps --help <simple|list|output|threads|misc|all>'
  or 'ps --help <s|l|o|t|m|a>'
 for additional help text.

For more details see ps(1).
Python 3.9.18

After:

$ docker run --rm -it rocky_rez rez-env python -- python --version
Python 3.9.18
codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 90.00000% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 58.30%. Comparing base (f63031e) to head (3f97be0).

Files Patch % Lines
src/rez/system.py 90.00% 0 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1735 +/- ## ========================================== - Coverage 58.30% 58.30% -0.01% ========================================== Files 126 126 Lines 17159 17161 +2 Branches 3505 3506 +1 ========================================== + Hits 10004 10005 +1 Misses 6492 6492 - Partials 663 664 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

darkvertex commented 2 months ago

I confirm this resolved #1732 for me.

ps: Someone needs to check out the macos py3.7 CI environment. It seems pretty broken.

JeanChristopheMorinPerso commented 2 months ago

Thanks @darkvertex for confirming! As for the macos builds, we'll have to fix them. The failures are caused by the new default macos-latest, which is now macos-14 which themselves are M1 runners. And 3.7 was not compatible with macOS arm64.

Anyway, I'll probably fix the macos thing in another PR.