Bug 215292 - [webkitpy] Pick a reasonable auto-install location on NFS mounts
Summary: [webkitpy] Pick a reasonable auto-install location on NFS mounts
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jonathan Bedard
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-08-07 13:42 PDT by Jonathan Bedard
Modified: 2020-08-08 13:31 PDT (History)
7 users (show)

See Also:


Attachments
Patch (1.46 KB, patch)
2020-08-07 13:51 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (1.34 KB, patch)
2020-08-07 14:03 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (1.47 KB, patch)
2020-08-07 14:28 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Bedard 2020-08-07 13:42:50 PDT
On occasion, webkitpy is hosted on an NFS mount. It would be wrong for clients which do not own the checkout to write auto-installed libraries to the NFS mount, since they do not own the mount.
Comment 1 Radar WebKit Bug Importer 2020-08-07 13:43:12 PDT
<rdar://problem/66698141>
Comment 2 Jonathan Bedard 2020-08-07 13:51:44 PDT
Created attachment 406208 [details]
Patch
Comment 3 Jonathan Bedard 2020-08-07 14:03:59 PDT
Created attachment 406209 [details]
Patch
Comment 4 Dean Johnson 2020-08-07 14:09:12 PDT
Comment on attachment 406209 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=406209&action=review

LGTM /w comments. Unofficial r+.

> Tools/Scripts/webkitpy/__init__.py:22
> +if sys.platform == 'darwin' and (not os.getuid() or os.stat(libraries).st_uid != os.getuid()):

This is a bit difficult to read; can we split some of the logic into variables to make it clearer? e.g.

is_root_user = os.getuid() == 0
user_owns_local_libraries = os.stat(libraries).st_uid == os.getuid()
if sys.platform == 'darwin' and (is_root_user or not user_owns_local_libraries):
    libraries = os.path.expanduser('~/Library/webkitpy')
Comment 5 Jonathan Bedard 2020-08-07 14:28:06 PDT
Created attachment 406214 [details]
Patch
Comment 6 Alexey Proskuryakov 2020-08-07 20:21:25 PDT
Comment on attachment 406214 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=406214&action=review

> Tools/Scripts/webkitpy/__init__.py:23
> +if sys.platform == 'darwin' and (is_root or not does_own_libraries):

Is it right to autoinstall onto a share even for someone who owns the directory?
Comment 7 Jonathan Bedard 2020-08-08 06:20:13 PDT
(In reply to Alexey Proskuryakov from comment #6)
> Comment on attachment 406214 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=406214&action=review
> 
> > Tools/Scripts/webkitpy/__init__.py:23
> > +if sys.platform == 'darwin' and (is_root or not does_own_libraries):
> 
> Is it right to autoinstall onto a share even for someone who owns the
> directory?

Probably not, although I'm not sure a reliable way to determine if a given directory is a share. In practice, in all of the cases I'm aware of a share being used, it would be a configuration error if an individual user owned a directory anyways.
Comment 8 EWS 2020-08-08 06:43:58 PDT
Committed r265408: <https://trac.webkit.org/changeset/265408>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 406214 [details].
Comment 9 Fujii Hironori 2020-08-08 12:57:40 PDT
WinCairo buildbot is failing since this change.

https://build.webkit.org/builders/WinCairo%2064-bit%20WKL%20Debug%20%28Tests%29/builds/3130


Traceback (most recent call last):
  File "./Tools/Scripts/run-webkit-tests", line 36, in <module>
    from webkitpy.common import multiprocessing_bootstrap
  File "C:\WebKit-BuildWorker\wincairo-wkl-debug-tests\build\Tools\Scripts\webkitpy\__init__.py", line 21, in <module>
    is_root = not os.getuid()
AttributeError: 'module' object has no attribute 'getuid'
Comment 10 Fujii Hironori 2020-08-08 13:31:29 PDT
Committed r265409: <https://trac.webkit.org/changeset/265409>