diff options
author | Chandan Singh <chandan@chandansingh.net> | 2020-08-04 21:37:28 +0000 |
---|---|---|
committer | bst-marge-bot <marge-bot@buildstream.build> | 2020-08-07 11:39:07 +0000 |
commit | 7dc9be9c0c2c0793f7443b29a802535031e5afbd (patch) | |
tree | 8ef3324dfcea171ddfa3f3786c33e4c121e15f1e | |
parent | 16a5a20e4e832fc4c8bc225b92251efbada5e454 (diff) | |
download | buildstream-chandan/fix-artifact-checkout.tar.gz |
_frontend/cli.py: Fix `artifact checkout` with no argumentschandan/fix-artifact-checkout
When no arguments are passed to `artifact checkout` command, it
currently crashes. We actually have a check of this, but we crash before
that becasue this method tries to read the value of `target` before
we've had time to check for it.
That can only be done correctly after the app has been initialized. So,
refactor that bit of the method to run after we've checked that we are
working with a non-empty target.
Also, add a regression test for it.
Fixes #1367.
-rw-r--r-- | src/buildstream/_frontend/cli.py | 16 | ||||
-rw-r--r-- | tests/frontend/buildcheckout.py | 12 |
2 files changed, 20 insertions, 8 deletions
diff --git a/src/buildstream/_frontend/cli.py b/src/buildstream/_frontend/cli.py index ff68af66d..9cf2aabe7 100644 --- a/src/buildstream/_frontend/cli.py +++ b/src/buildstream/_frontend/cli.py @@ -1234,14 +1234,6 @@ def artifact_checkout(app, force, deps, integrate, hardlinks, tar, compression, if compression: click.echo("ERROR: --compression can only be provided if --tar is provided", err=True) sys.exit(-1) - else: - if directory is None: - location = os.path.abspath(os.path.join(os.getcwd(), target)) - if location[-4:] == ".bst": - location = location[:-4] - else: - location = directory - tar = False else: location = tar try: @@ -1264,6 +1256,14 @@ def artifact_checkout(app, force, deps, integrate, hardlinks, tar, compression, if not target: raise AppError('Missing argument "ELEMENT".') + if not tar: + if directory is None: + location = os.path.abspath(os.path.join(os.getcwd(), target)) + if location[-4:] == ".bst": + location = location[:-4] + else: + location = directory + app.stream.checkout( target, location=location, diff --git a/tests/frontend/buildcheckout.py b/tests/frontend/buildcheckout.py index ca88a1368..e613b4194 100644 --- a/tests/frontend/buildcheckout.py +++ b/tests/frontend/buildcheckout.py @@ -1072,3 +1072,15 @@ def test_partial_checkout_fail(tmpdir, datafiles, cli): res = cli.run(project=project, args=["artifact", "checkout", "--pull", build_elt, "--directory", checkout_dir]) res.assert_main_error(ErrorDomain.STREAM, "uncached-checkout-attempt") assert re.findall(r"Remote \((\S+)\) does not have artifact (\S+) cached", res.stderr) + + +# Regression test for https://gitlab.com/BuildStream/buildstream/-/issues/1367. +# Make sure that `artifact checkout` fails gracefully when no arguments are +# provided. +@pytest.mark.datafiles(DATA_DIR) +def test_fail_no_args(datafiles, cli): + project = str(datafiles) + + result = cli.run(project=project, args=["artifact", "checkout"]) + result.assert_main_error(ErrorDomain.APP, None) + assert "Missing argument" in result.stderr |