Make osutils.ResolveSymlink more obvious |
||||
Issue descriptionI don't see any benefit over os.path.realpath (plus os.path.relpath if you want its 'root' kwarg).
,
Aug 28
it probably isn't useful if |root| isn't specified. which means the mobmonitor/system/systeminfo.py code should be converted to os.path.realpath, and we can prob make it clearer by making the root non-optional. otherwise, if root is specified, i'm not seeing how os.path helps us. the example text in the func explains what this func is good for. os.path.relpath will still resolve absolute paths outside of |root|, and relpath's |start| is purely for relative symlinks.
,
Aug 28
Oh its for resolving absolute chroot symlinks from outside the chroot. That docstring...does not convey that to me.
,
Aug 28
"""Resolve a symlink |file_name| relative to |root|.""" is obvious once you know ;)
,
Aug 28
,
Sep 4
,
Sep 6
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/chromite/+/ad97af93ecfd2add10c7da60a3484691b9e2e74f commit ad97af93ecfd2add10c7da60a3484691b9e2e74f Author: Lann Martin <lannm@chromium.org> Date: Thu Sep 06 04:04:32 2018 Rename osutils.ResolveSymlink to ResolveSymlinkInRoot This function was confusing to me. Updated to make it more obvious that this is for a special use case and made the `root` argument non-optional. Replaced one usage of ResolveSymlink with os.path.realpath. BUG= chromium:878570 TEST=lib/osutils_unittest; mobmonitor/system/systeminfo_unittest Change-Id: Iafd5fd44b12998a34c59f97918170c40ec46ee19 Reviewed-on: https://chromium-review.googlesource.com/1205631 Commit-Ready: Lann Martin <lannm@chromium.org> Tested-by: Lann Martin <lannm@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org> [modify] https://crrev.com/ad97af93ecfd2add10c7da60a3484691b9e2e74f/mobmonitor/system/systeminfo_unittest.py [modify] https://crrev.com/ad97af93ecfd2add10c7da60a3484691b9e2e74f/cros/test/image_test.py [modify] https://crrev.com/ad97af93ecfd2add10c7da60a3484691b9e2e74f/mobmonitor/system/systeminfo.py [modify] https://crrev.com/ad97af93ecfd2add10c7da60a3484691b9e2e74f/lib/osutils_unittest.py [modify] https://crrev.com/ad97af93ecfd2add10c7da60a3484691b9e2e74f/lib/osutils.py
,
Sep 10
|
||||
►
Sign in to add a comment |
||||
Comment 1 by la...@chromium.org
, Aug 28