This is important because often, the first script a bot runs will be installing something with sudo, which means that the autoinstall directory will be owned by root.
<rdar://problem/66342996>
Created attachment 405597 [details] Patch
Comment on attachment 405597 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=405597&action=review > Tools/Scripts/libraries/webkitcorepy/webkitcorepy/autoinstall.py:278 > + owner = cls.owner() > + if owner: > + os.chown(creation_root, *owner) > + for root, directories, files in os.walk(creation_root): > + for directory in directories: > + os.chown(os.path.join(root, directory), *owner) > + for file in files: > + os.chown(os.path.join(root, file), *owner) > + This seems a repeating twice, maybe create a helper which can also help with nested indentation: @classmethod def change_ownership_for_path(cls, path, recursive=False): # Windows doesn't have sudo if not hasattr(os, "geteuid"): return # If we aren't root, the default behavior is correct if os.geteuid() != 0: return # If running as sudo, we really want the caller of sudo to own the autoinstall directory uid = os.environ.get('SUDO_UID', -1)) gid = int(os.environ.get('SUDO_GID', -1)) os.chown(path, uid, gid) if not recursive: return if not os.path.isdir(path): return for root, directories, files in os.walk(path): for directory in directories: os.chown(os.path.join(root, directory), uid, gid) for file in files: os.chown(os.path.join(root, file), uid, gid)
Created attachment 405633 [details] Patch
Comment on attachment 405633 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=405633&action=review > Tools/Scripts/libraries/webkitcorepy/webkitcorepy/autoinstall.py:238 > + return None Even thought it's equivalent,, may be just `return` to keep the consistency.
Created attachment 405636 [details] Patch
Committed r265123: <https://trac.webkit.org/changeset/265123> All reviewed patches have been landed. Closing bug and clearing flags on attachment 405636 [details].