Skip to content

Commit

Permalink
Add option to collapse symlinks in isolate.py
Browse files Browse the repository at this point in the history
  • Loading branch information
kjlubick authored and Commit bot committed Apr 28, 2017
1 parent 46c1328 commit c2e0ff0
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 17 deletions.
23 changes: 15 additions & 8 deletions client/isolate.py
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,7 @@ def load_files(cls, isolated_filepath):

def load_isolate(
self, cwd, isolate_file, path_variables, config_variables,
extra_variables, blacklist, ignore_broken_items):
extra_variables, blacklist, ignore_broken_items, collapse_symlinks):
"""Updates self.isolated and self.saved_state with information loaded from a
.isolate file.
Expand All @@ -489,9 +489,9 @@ def load_isolate(
assert os.path.isabs(isolate_file), isolate_file
isolate_file = file_path.get_native_path_case(isolate_file)
logging.info(
'CompleteState.load_isolate(%s, %s, %s, %s, %s, %s)',
'CompleteState.load_isolate(%s, %s, %s, %s, %s, %s, %s)',
cwd, isolate_file, path_variables, config_variables, extra_variables,
ignore_broken_items)
ignore_broken_items, collapse_symlinks)

# Config variables are not affected by the paths and must be used to
# retrieve the paths, so update them first.
Expand Down Expand Up @@ -550,7 +550,9 @@ def load_isolate(
self.saved_state.root_dir)
for f in infiles
]
follow_symlinks = sys.platform != 'win32'
follow_symlinks = False
if not collapse_symlinks:
follow_symlinks = sys.platform != 'win32'
# Expand the directories by listing each file inside. Up to now, trailing
# os.path.sep must be kept.
infiles = isolated_format.expand_directories_and_symlinks(
Expand All @@ -565,7 +567,7 @@ def load_isolate(
self.saved_state.update_isolated(command, infiles, read_only, relative_cwd)
logging.debug(self)

def files_to_metadata(self, subdir):
def files_to_metadata(self, subdir, collapse_symlinks):
"""Updates self.saved_state.files with the files' mode and hash.
If |subdir| is specified, filters to a subdirectory. The resulting .isolated
Expand All @@ -582,7 +584,8 @@ def files_to_metadata(self, subdir):
filepath,
self.saved_state.files[infile],
self.saved_state.read_only,
self.saved_state.algo)
self.saved_state.algo,
collapse_symlinks)

def save_files(self):
"""Saves self.saved_state and creates a .isolated file."""
Expand Down Expand Up @@ -674,7 +677,8 @@ def load_complete_state(options, cwd, subdir, skip_update):
# Then load the .isolate and expands directories.
complete_state.load_isolate(
cwd, isolate, options.path_variables, options.config_variables,
options.extra_variables, options.blacklist, options.ignore_broken_items)
options.extra_variables, options.blacklist, options.ignore_broken_items,
options.collapse_symlinks)

# Regenerate complete_state.saved_state.files.
if subdir:
Expand All @@ -691,7 +695,7 @@ def load_complete_state(options, cwd, subdir, skip_update):
subdir = subdir.replace('/', os.path.sep)

if not skip_update:
complete_state.files_to_metadata(subdir)
complete_state.files_to_metadata(subdir, options.collapse_symlinks)
return complete_state


Expand Down Expand Up @@ -1122,6 +1126,9 @@ def add_isolate_options(parser):
help='Indicates that invalid entries in the isolated file to be '
'only be logged and not stop processing. Defaults to True if '
'env var ISOLATE_IGNORE_BROKEN_ITEMS is set')
group.add_option(
'-L', '--collapse_symlinks', action='store_true',
help='Treat any symlinks as if they were the normal underlying file')
parser.add_option_group(group)


Expand Down
11 changes: 9 additions & 2 deletions client/isolated_format.py
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ def expand_directories_and_symlinks(


@tools.profile
def file_to_metadata(filepath, prevdict, read_only, algo):
def file_to_metadata(filepath, prevdict, read_only, algo, collapse_symlinks):
"""Processes an input file, a dependency, and return meta data about it.
Behaviors:
Expand All @@ -326,6 +326,8 @@ def file_to_metadata(filepath, prevdict, read_only, algo):
windows, mode is not set since all files are 'executable' by
default.
algo: Hashing algorithm used.
collapse_symlinks: True if symlinked files should be treated like they were
the normal underlying file.
Returns:
The necessary dict to create a entry in the 'files' section of an .isolated
Expand All @@ -340,7 +342,12 @@ def file_to_metadata(filepath, prevdict, read_only, algo):
# There is the risk of the file's timestamp being reset to its last value
# manually while its content changed. We don't protect against that use case.
try:
filestats = os.lstat(filepath)
if collapse_symlinks:
# os.stat follows symbolic links
filestats = os.stat(filepath)
else:
# os.lstat does not follow symbolic links, and thus preserves them.
filestats = os.lstat(filepath)
except OSError:
# The file is not present.
raise MappingError('%s is missing' % filepath)
Expand Down
2 changes: 1 addition & 1 deletion client/isolateserver.py
Original file line number Diff line number Diff line change
Expand Up @@ -1708,7 +1708,7 @@ def directory_to_metadata(root, algo, blacklist):
root, '.' + os.path.sep, blacklist, sys.platform != 'win32')
metadata = {
relpath: isolated_format.file_to_metadata(
os.path.join(root, relpath), {}, 0, algo)
os.path.join(root, relpath), {}, 0, algo, False)
for relpath in paths
}
for v in metadata.itervalues():
Expand Down
8 changes: 5 additions & 3 deletions client/tests/isolate_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,8 @@ def test_read_only(self):
}
complete_state = isolate.CompleteState(None, isolate.SavedState(self.cwd))
complete_state.load_isolate(
unicode(self.cwd), unicode(isolate_file), {}, {}, {}, None, False)
unicode(self.cwd), unicode(isolate_file), {}, {}, {}, None, False,
False)
self.assertEqual(expected, complete_state.saved_state.to_isolated())


Expand Down Expand Up @@ -289,6 +290,7 @@ class Options(object):
}
extra_variables = {'foo': 'bar'}
ignore_broken_items = False
collapse_symlinks = False
return Options()

def _cleanup_isolated(self, expected_isolated):
Expand Down Expand Up @@ -976,7 +978,7 @@ def test_with_os(
'OS': config_os,
}
c.load_isolate(
unicode(self.cwd), root_isolate, {}, config, {}, None, False)
unicode(self.cwd), root_isolate, {}, config, {}, None, False, False)
# Note that load_isolate() doesn't retrieve the meta data about each file.
expected = {
'algo': 'sha-1',
Expand Down Expand Up @@ -1171,7 +1173,7 @@ def test_with_os(config_os, expected_files, command, relative_cwd):
'EXTRA': 'indeed',
}
c.load_isolate(
unicode(cwd), root_isolate, paths, config, extra, None, False)
unicode(cwd), root_isolate, paths, config, extra, None, False, False)
# Note that load_isolate() doesn't retrieve the meta data about each file.
expected = {
'algo': 'sha-1',
Expand Down
31 changes: 28 additions & 3 deletions client/tests/isolated_format_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ def test_file_to_metadata_path_case_simple(self):
linkdir = os.path.join(self.cwd, 'linkdir')
os.symlink('subDir', linkdir)
actual = isolated_format.file_to_metadata(
unicode(linkdir.upper()), {}, True, ALGO)
unicode(linkdir.upper()), {}, True, ALGO, False)
expected = {'l': u'subdir', 't': int(os.stat(linkdir).st_mtime)}
self.assertEqual(expected, actual)

Expand All @@ -109,14 +109,14 @@ def test_file_to_metadata_path_case_complex(self):
os.symlink('linkedDir1', subsymlinkdir)

actual = isolated_format.file_to_metadata(
unicode(subsymlinkdir.upper()), {}, True, ALGO)
unicode(subsymlinkdir.upper()), {}, True, ALGO, False)
expected = {
'l': u'linkeddir1', 't': int(os.stat(subsymlinkdir).st_mtime),
}
self.assertEqual(expected, actual)

actual = isolated_format.file_to_metadata(
unicode(linkeddir1.upper()), {}, True, ALGO)
unicode(linkeddir1.upper()), {}, True, ALGO, False)
expected = {
'l': u'../linkeddir2', 't': int(os.stat(linkeddir1).st_mtime),
}
Expand Down Expand Up @@ -144,6 +144,31 @@ def test_symlink_input_absolute_path(self):
actual = isolated_format.expand_symlinks(src, u'out/foo/bar.txt')
self.assertEqual((u'out/foo/bar.txt', []), actual)

def test_file_to_metadata_path_case_collapse(self):
# Ensure setting the collapse_symlink option doesn't include the symlinks
basedir = os.path.join(self.cwd, 'basedir')
os.mkdir(basedir)
subdir = os.path.join(basedir, 'subdir')
os.mkdir(subdir)
linkdir = os.path.join(basedir, 'linkdir')
os.mkdir(linkdir)

foo_file = os.path.join(subdir, 'Foo.txt')
open(foo_file, 'w').close()
sym_file = os.path.join(basedir, 'linkdir', 'Sym.txt')
os.symlink('../subdir/Foo.txt', sym_file)

actual = isolated_format.file_to_metadata(sym_file, {}, True, ALGO, True)

expected = {
# SHA-1 of empty string
'h': 'da39a3ee5e6b4b0d3255bfef95601890afd80709',
'm': 288,
's': 0,
't': int(round(os.stat(foo_file).st_mtime)),
}
self.assertEqual(expected, actual)


class TestIsolated(auto_stub.TestCase):
def test_load_isolated_empty(self):
Expand Down

0 comments on commit c2e0ff0

Please sign in to comment.