https://pantsbuild.org/ logo
#general
Title
# general
a

average-vr-56795

12/05/2019, 2:53 PM
Does anyone know why https://github.com/pantsbuild/pants/blob/master/src/python/pants/backend/project_info/tasks/idea_plugin_gen.py uses
self.context.options.positional_args
rather than
[target.address.reference() for target in self.context.target_roots()]
or similar?
w

witty-crayon-22786

12/05/2019, 3:59 PM
iirc, that was to allow it to capture and pass the CLI specs to the IDEA plugin
so that when it loaded/reloaded, it would get the latest/complete set of targets
if that doesn't sound right, can ping yi
a

average-vr-56795

12/05/2019, 4:02 PM
As in, to allow
::
to continue to expand if new targets are added?
(This came up because it means that
--owner-of
doesn’t work for this goal)
w

witty-crayon-22786

12/05/2019, 4:02 PM
yep
...mm.
best laid plans.
could possibly add a conditional there...? bummer though.
really, "the roots" are defined by one of three things. reifying that up into a single enum somewhere to make it more obvious would be good. i think Danny did a bit of that with --query
a

average-vr-56795

12/05/2019, 4:05 PM
We could probably try to do a merging? Merge in extra specs from
target_roots()
which aren’t covered by a spec from the list of positional args?
w

witty-crayon-22786

12/05/2019, 4:06 PM
mm, maybe? an issue might be that if you include concrete targets and they get deleted later the export will start failing.
cc @damp-easter-17791
a

average-vr-56795

12/05/2019, 4:06 PM
Doesn’t seem hugely different to the failure modes of the status quo
w

witty-crayon-22786

12/05/2019, 4:11 PM
hm?
...i suppose that we ask people to reload any time a BUILD file changes anyway.
yea. ok
would still probably rather do an "either/or"
oh, wait. no
the issue is that if your project was configured with a deleted target, reloading will not work
and you'd need to throw it away and re-import.
(if specs, use them, otherwise use roots)
a

average-vr-56795

12/05/2019, 4:13 PM
Sure, but whether that happens because I ran:
./pants --owner-of=foo/bar idea-plugin
or
./pants idea-plugin foo:bar
we have the same problem, right?
It’s only if I ran
./pants idea-plugin foo:
that I wouldn’t error
w

witty-crayon-22786

12/05/2019, 4:14 PM
the issue is that you can't distinguish the first two cases
...or whatever
a

average-vr-56795

12/05/2019, 4:14 PM
Sure, but they should be identical, and right now only one of them works?
w

witty-crayon-22786

12/05/2019, 4:14 PM
...other than by seeing whether there are specs
if there are specs, use them
@average-vr-56795: right, i'm not saying don't fix the issue. i'm saying: the fix is probably a conditional of "use specs if we have them, otherwise target roots"
a

average-vr-56795

12/05/2019, 4:16 PM
I’m saying: Expand the specs If there are any roots which aren’t covered by a spec, add that single-target spec to the list of specs Use the merged list
w

witty-crayon-22786

12/05/2019, 4:17 PM
that isn't possible, because you may only use exactly one of 1) specs, 2) --owner-of, 3) --changed
a

average-vr-56795

12/05/2019, 4:17 PM
I guess we could say “If there are both specs and target roots, and they’re not effectively identical, error”
a

average-vr-56795

12/05/2019, 4:19 PM
Oh! I was not aware! Ok, yes, I’m with you, a conditional sounds… Well, not great, because ideally a task shouldn’t have to know that, but like the right solution here 🙂
w

witty-crayon-22786

12/05/2019, 4:19 PM
yea. this should be converted into an enum that makes the either-or-ness more obvious. see my comment about Danny/--query
a

average-vr-56795

12/05/2019, 4:20 PM
That makes a lot more sense now 🙂 Cool!
w

witty-crayon-22786

12/05/2019, 4:20 PM
other thing is that Yi has suggested that we should definitely port this to v2
which i agree on, but which (for exactly this reason) would probably involved doing the enum thing
6 Views