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

Render a warning rather than failing `list` when no targets are matched #5598

Merged
merged 4 commits into from Mar 15, 2018

Conversation

Projects
None yet
6 participants
@stuhood
Copy link
Member

stuhood commented Mar 14, 2018

Problem

The v2 engine changed the behaviour of ./pants list (with no specs) from "list everything", to "fail with an error". But due to the deprecation/removal of the changed goal, this also has the effect of failing with an error for ./pants --changed=.. list calls that result in no matched targets, which is a behaviour change from v1.

Solution

Rather than raising an error, trigger a warning to stderr which naturally handles both cases by indicating that their query wasn't an error: it just didn't match anything.

@stuhood stuhood requested review from benjyw , kwlzn and dotordogh Mar 14, 2018

@kwlzn

kwlzn approved these changes Mar 14, 2018

Copy link
Member

kwlzn left a comment

lgtm

@benjyw

benjyw approved these changes Mar 14, 2018

@stuhood stuhood added this to the 1.5.x milestone Mar 14, 2018

@baroquebobcat
Copy link
Contributor

baroquebobcat left a comment

lgtm!

Maybe update the title to

Update list goal: when no targets matched, render a warning instead of failing

I think it'll be clearer in the release notes that way.

@dotordogh
Copy link
Contributor

dotordogh left a comment

Thank you!!

@stuhood

This comment has been minimized.

Copy link
Member

stuhood commented Mar 14, 2018

Update list goal: when no targets matched, render a warning instead of failing

Not sure that's clearer... going to leave as is.

"""
if not self.context.target_roots:
raise TaskError('Please specify one or more explicit target '
'specs (e.g. `./pants {0} ::`).'.format(goal_name))
print('WARNING: No targets were matched in goal `{}`.'.format(goal_name), file=sys.stderr)

This comment has been minimized.

@wisechengyi

wisechengyi Mar 14, 2018

Contributor

Any particular reason to use print over self.context.log.warn?

This comment has been minimized.

@stuhood

stuhood Mar 14, 2018

Member

A Quiet task has all log info silenced.

This comment has been minimized.

@wisechengyi

wisechengyi Mar 15, 2018

Contributor

fair

@stuhood stuhood merged commit ff6942d into pantsbuild:master Mar 15, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@stuhood stuhood deleted the twitter:stuhood/warn-for-empty-list branch Mar 15, 2018

wisechengyi added a commit that referenced this pull request Mar 15, 2018

Render a warning rather than failing `list` when no targets are match…
…ed (#5598)

### Problem

The `v2` engine changed the behaviour of `./pants list` (with no specs) from "list everything", to "fail with an error". But due to the deprecation/removal of the `changed` goal, this also has the effect of failing with an error for `./pants --changed=.. list` calls that result in no matched targets, which is a behaviour change from `v1`.

### Solution

Rather than raising an error, trigger a warning to stderr which naturally handles both cases by indicating that their query wasn't an error: it just didn't match anything.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment