Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow partial sync with multiple paths #182

Merged
merged 11 commits into from
Sep 4, 2020
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion plugin.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ configuration:
stream:
type: string
sync:
type: string
type: array
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesnt have any actual functional effect afaik - I think buildkite have some plans to auto-generate docs from this file

parallel:
type: string
client_opts:
Expand Down
18 changes: 17 additions & 1 deletion python/buildkite.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,28 @@ def get_env():
env[p4var] = plugin_value
return env

def list_from_env_array(var):
"""Read list of values from either VAR or VAR_1, VAR_2 etc"""
ca-johnson marked this conversation as resolved.
Show resolved Hide resolved
result = os.environ.get(var, [])
if result:
return [result] # convert single value to list

i = 0
while True:
elem = os.environ.get("%s_%d" % (var, i))
if not elem:
break
result.append(elem)
i += 1

return result

def get_config():
"""Get configuration which will be passed directly to perforce.P4Repo as kwargs"""
conf = {}
conf['view'] = os.environ.get('BUILDKITE_PLUGIN_PERFORCE_VIEW') or '//... ...'
conf['stream'] = os.environ.get('BUILDKITE_PLUGIN_PERFORCE_STREAM')
conf['sync'] = os.environ.get('BUILDKITE_PLUGIN_PERFORCE_SYNC')
conf['sync'] = list_from_env_array('BUILDKITE_PLUGIN_PERFORCE_SYNC')
conf['parallel'] = os.environ.get('BUILDKITE_PLUGIN_PERFORCE_PARALLEL') or 0
conf['client_opts'] = os.environ.get('BUILDKITE_PLUGIN_PERFORCE_CLIENT_OPTIONS')

Expand Down
13 changes: 9 additions & 4 deletions python/perforce.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,15 @@ def __init__(self, root=None, view=None, stream=None,
root: Directory in which to create the client workspace
view: Client workspace mapping
stream: Client workspace stream. Overrides view parameter.
sync: List of paths to sync. Defaults to entire view.
client_opts: Additional options to add to client. (e.g. allwrite)
parallel: How many threads to use for parallel sync.
Comment on lines +24 to +26
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Driveby, document undocumented params

"""
self.root = os.path.abspath(root or '')
self.stream = stream
self.view = self._localize_view(view or [])
self.sync_paths = sync or '//...'
self.sync_paths = sync or ['//...']
assert isinstance(self.sync_paths, list)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will never fail other than when authoring unit tests which pass the wrong type - avoids confusion when making this mistake.

Ideally we would add some types to this class instead

self.client_opts = client_opts or ''
self.parallel = parallel

Expand Down Expand Up @@ -176,9 +180,10 @@ def sync(self, revision=None):
"""Sync the workspace"""
self._setup_client()
self.revert()
sync_files = ['%s%s' % (path, revision or '') for path in self.sync_paths]
result = self.perforce.run_sync(
'--parallel=threads=%s' % self.parallel,
'%s%s' % (self.sync_paths, revision or ''),
*sync_files,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expands list into params, e.g.:

l = [1, 2, 3]
print(l) => [1, 2, 3]
print(*l) => 1 2 3 # i.e. print(1, 2, 3)

handler=SyncOutput(self.perforce.logger),
)
if result:
Expand Down Expand Up @@ -248,14 +253,14 @@ def p4print_unshelve(self, changelist):

# Turn sync spec info a prefix to filter out unwanted files
# e.g. //my-depot/dir/... => //my-depot/dir/
sync_prefix = self.sync_paths.rstrip('.')
sync_prefixes = [prefix.rstrip('.') for prefix in self.sync_paths]

cmds = []
for depotfile, localfile in depot_to_local.items():
if os.path.isfile(localfile):
os.chmod(localfile, stat.S_IWRITE)
os.unlink(localfile)
if depotfile.startswith(sync_prefix):
if any(depotfile.startswith(prefix) for prefix in sync_prefixes):
cmds.append(('print', '-o', localfile, '%s@=%s' % (depotfile, changelist)))

self.run_parallel_cmds(cmds)
Expand Down
20 changes: 19 additions & 1 deletion python/test_perforce.py
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,24 @@ def test_checkout(server, tmpdir):
with open(os.path.join(tmpdir, "p4config")) as content:
assert "P4PORT=%s\n" % repo.perforce.port in content.readlines(), "Unexpected p4config content"

def test_checkout_partial_path(server, tmpdir):
"""Test checking out a subset of view with one path"""
repo = P4Repo(root=tmpdir, sync=['//depot/file.txt'])
repo.sync()
assert 'file.txt' in os.listdir(tmpdir)

def test_checkout_partial_dir(server, tmpdir):
"""Test checking out a subset of view with one directory"""
repo = P4Repo(root=tmpdir, sync=['//depot/...'])
repo.sync()
assert 'file.txt' in os.listdir(tmpdir)

def test_checkout_partial_multiple(server, tmpdir):
"""Test checking out a subset of view with multiple paths"""
repo = P4Repo(root=tmpdir, sync=['//depot/fake-dir/...', '//depot/file.txt'])
repo.sync()
assert 'file.txt' in os.listdir(tmpdir)

def test_checkout_stream(server, tmpdir):
"""Test checking out a stream depot"""
repo = P4Repo(root=tmpdir, stream='//stream-depot/main')
Expand Down Expand Up @@ -321,7 +339,7 @@ def test_p4print_unshelve(server, tmpdir):
assert not os.path.exists(os.path.join(tmpdir, "newfile.txt")), "File unshelved for add was not deleted"

# Shelved changes containing files not selected for sync are skipped
repo = P4Repo(root=tmpdir, sync='//depot/fake-dir/...')
repo = P4Repo(root=tmpdir, sync=['//depot/fake-dir/...'])
repo.sync()
repo.p4print_unshelve('3') # Modify file.txt
assert not os.path.exists(os.path.join(tmpdir, "file.txt"))
Expand Down