[Core] Vendor setproctitle

[jjyao]

Why are these changes needed?

Vendor setproctitle by copying its C source code and expose it to Python via Cython (the original C source code is exposed via Python C extension).

The original setproctitle has a side effect when you import it due to dvarrazzo/py-setproctitle#114 (it will change psutil.Process().cmdline() from ["python", "script.py", "--flag"] to ["python script.py --flag", "", ""]) and this PR avoids that by only initializing and changing process title when setproctitle() is called.

Related issue number

Checks

• I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
• I've run scripts/format.sh to lint the changes in this PR.
• I've included any doc changes needed for https://docs.ray.io/en/master/.
  • I've added any new APIs to the API Reference. For example, if I added a
    method in Tune, I've added it in doc/source/tune/api/ under the
    corresponding .rst file.
• I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
• Testing Strategy
  • Unit tests
  • Release tests
  • This PR is not tested :(

[jjyao]

Actually why not just declare setproctitle as a default dependency of Ray? What's the drawback of it? https://discuss.python.org/t/can-vendoring-dependencies-in-a-build-be-officially-supported

Case 1: I maintain a package which is a building-block for many of our internal applications and some of our customers’ applications, so maximal compatibility is a goal. This leads to us not wanting to use many libraries, as there are conflicts this would or could introduce with the downstream applications. e.g. I don’t want to use jsonschema because we have applications which use specific versions of it.

That makes sense, but unless you depend on packages with very constrained dependencies, or you yourself depend on a very constrained version of that package, it should not be an issue.
The example you gave, jsonschema, does not have any runtime dependencies, so unless you need a very constrained version of it, depending on it should not really cause any issues.

setproctitle doesn't have dependencies so dpending on it should be just fine?

[jjyao]

@aslonnie thoughts?

[jjyao]

Even if I don’t need a constrained version of it today, any time it does a breaking change (major release if you use semver), I need to consider it. jsonschema recently released 4.0, so using it would put us in the position of having to support jsonschema 3.x and 4.x in the same codebase.

well, this can be a reason for vendoring.

[jjyao]

The main reason is that, if one of those dependencies has a security
vulnerability, a new or patched version of that dependency needs to
be re-vendored and a new version of the “immutable artifact”
published/installed even if your actual project has no new changes
at all. SBoM efforts go some way toward alleviating this concern,
but it’s still an added challenge and possible delay for the end
user who is otherwise left with a steaming pile of compromised
systems. Multiply it by all of the different things you’ve installed
each of which contains vendored dependencies and the situation can
quickly become unmanageable.

This is the reason for not vendoring.

[mattip]

Less vendoring is better in my opinion. conda-forge has been quite successful in patching ray to declare a regular dependency without vendoring.

[aslonnie]

maybe add --no-deps

[aslonnie]

consider put it under not os.getenv("SKIP_THIRDPARTY_INSTALL_CONDA_FORGE"): ? conda forge does not really like these kind of vendoring afaik.

cc @mattip

we really just use setproctitle and getproctitle these two functions. we can put a stub file that redirects to

you can put a stub or wrapper file that redirects to either ray._private.thirdparty or external setproctitle

[pcmoritz]

The north star for Ray Core (pip install ray) is to have zero python dependencies, so declaring it as a dependency is not an option.

The ideal would be to just set the process title in the C++ code / integrate it properly into the build system, is that an option people are considering for this?

[jjyao]

import setproctitle

   @ray.remote
   def f(x):
      assert setproctitle.getproctitle() == "ray::special_f"
      return x + 1

This test now fails because how setproctitle works. setproctitle mainly provides two methods: setproctitle() and getproctitle(). setproctitle() calls syscalls to update the process title (e.g. https://man.freebsd.org/cgi/man.cgi?query=setproctitle&sektion=3&format=html) AND store the new title in a global variable. getproctitle() simply returns the previously set title from the global variable (because there is no system call to get proc title). The issue is that now we vendor setproctitle and it's different from the one inside site-package/ so if you setproctitle via one version of the setproctitle library and try to getproctitle via another version of the setproctitle library, it won't work because they don't share the same global variable.

[aslonnie]

hmm.. do we want ray users to rely on setproctitle.getproctitle() to do things?

[aslonnie]

feels that ray core should provide an api of ray.getproctitle() and ask user to use that instead.

because getproctitle() basically is not really a thing without the global setproctitle package.

or we cannot vendor setproctitle.. we can maybe have an optional/legacy behavior to call the global setproctitle.setproctitle() when it exists, just to grandfather the old behavior.

[jjyao]

No need to enforce the import order for setproctitle since we vendor it and doesn't rely on sys.path anymore.

[jjyao]

This is better checking since it actually makes sure the process title is actually changed underneath while getproctitle() just returns the cached title previously set by setproctitle(). In other words, the previous check only checks that setproctitle() was called without checking if the actual process title was changed or not.

[jjyao]

Same reason as above, checking psutil.Process().cmdline() is better than getproctitle()

[jjyao]

This is basically what setproctitle.setup.py does.

[aslonnie]

pretty cool!