get_deploy_root: Avoid useless extra realpath call.

The comment that tabbott edited into my commit while wimpifying this
function is wrong on multiple levels.

Firstly, the way in which users might be “running our scripts” was
never relevant.  `__file__` is not the script that the user ran, it’s
zulip_tools.py itself.  What matters is not how the user ran the
script, but rather how zulip_tools was imported.  If zulip_tools was
imported as scripts.lib.zulip_tools, then `__file__` must end with
`scripts/lib/zulip_tools.py`, so running dirname three times on it is
fine.  In fact, in Python ≥ 3.4 (we don’t support anything older),
`__file__` in an imported module is always an absolute path, so it
must end with `scripts/lib/zulip_tools.py` in any case.

(At present, there’s one script that imports lib.zulip_tools, and the
installer runs scripts/lib/zulip_tools.py as a script, but those uses
don’t hit this function.)

Secondly, even if we do care about `__file__` being a funny relative
path, there’s still no reason to have two calls to `realpath`.
`realpath(dirname(dirname(dirname(realpath(…)))))` is equivalent to
`dirname(dirname(dirname(realpath(…)))), as the inner `realpath` has
already canonicalized symlinks at every level.

This version also deals with `__file__` being a funny relative
path (assuming none of scripts, lib, and zulip_tools.py are themselves
symlinks), while making fewer `lstat` calls than either of the above
constructions.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
This commit is contained in:
Anders Kaseorg 2019-03-05 14:45:00 -08:00 committed by Tim Abbott
parent 500b161aab
commit a45be467ad
1 changed files with 3 additions and 5 deletions

View File

@ -81,11 +81,9 @@ def parse_cache_script_args(description):
return args return args
def get_deploy_root() -> str: def get_deploy_root() -> str:
# This calls realpath twice to handle both symlinks and users return os.path.realpath(
# running our scripts with relative paths from a current working os.path.normpath(os.path.join(os.path.dirname(__file__), "..", ".."))
# directory of `scripts/`. )
return os.path.realpath(os.path.dirname(os.path.dirname(
os.path.dirname(os.path.realpath(__file__)))))
def get_deployment_version(extract_path): def get_deployment_version(extract_path):
# type: (str) -> str # type: (str) -> str