https://pantsbuild.org/ logo
Join Slack
Powered by
Is `TransitiveHydratedTargets.closure` in topologi...
# development
h
Is 
TransitiveHydratedTargets.closure
in topological order? What is the canonical way to get the dep closure of some targets in t-order?
w
it is not, no
i don't think there is one... executing recursively is supposed to be more natural in the engine
although that hasn't been proven in anything other than prototypes
h
Yeah, this is a case where I actually need the graph in order, not directly for execution purposes.
w
closure is ... depth first, i think? can confirm
why topological, if not for execution?
h
Need it to order the ExportedTargets, which I guess eventually may affect execution order, but it's not the same as target closure dep order (although it is induced by it)
I mean we do need it in order to process the ExportedTargets in the right order, but note that those targets may not even be in the closure of the cmd-line specs
It might be possible to do something without an explicit sort in hand though, now that I think of it
w
well, you indicated that execution order is actually relevant there, and i suggested recursion to allow for parallelism
recursion with actual declared dependencies both allows for parallelism, and avoids explicit ordering.
h
Yes, but this is a tricky case because the things we're recursing over might not be in the dep closure (they are the targets that own them)
It might be OK to recurse on "run setup py on the owner of each of my deps".
w
right.
h
It might meet the same owner multiple times, but that should be fine because the engine will have memoized it
So maybe that's fine
w
yep
h
There's a caveat
w
(the engine will give you a useful cycle error if cycles exist)
h
The caveat is if the user asked to run on specific exported targets, do we just do those (in order) or do we say "if you want to enforce order, you have to run on all relevant exported targets in order, which may be more than the ones you asked for"
I think that's fine
w
Well, recursing to do "something" is fine, even if it isn't side-effecting.
Ideally this would not be side-effecting.
h
Oh I just mean, if the only way to enforce order is to recurse then we are committing to "if order matters, we will run not just on the specific exported targets you asked for, but on all of the relevant ones"
But I think that's fine
the reason order might matter also implies that we should do that
i.e., so you don't publish dependees before their dependencies
Oh but then there are issues with writing to the console.
Hmm.
I guess we just don't write to the console then.
Can a console rule recurse on itself?
w
no
the pattern that exists in 
v1
is to have the roots be the things that were actually requested, and thus published to 
dist
for example: 
./pants bundle $roots
only published the roots to 
dist
...even though other bundles might be privately built under 
.pants.d
in order to accomplish that work
the same goes for writing to the console***.
h
Right
but here there is a correctness issue
Possibly
We may have to act on non-roots
and if we do this recursively then we can't, for example, use the Workspace to write the results to dist/
w
sure. but see above. if they weren't roots then you shouldn't.
h
Sure, that is the case today
but it leads to a possible correctness problem
w
...why?
h
If the setup-py command we run publishes to pypi, then we want to publish all dependencies before we publish the roots
w
sure. but that's not related to the dist directory or the console
h
So it's true that technically that publish doesn't require writing to 
dist
I guess
But that is pretty unintuitive
w
the side-effects-in-rules part is the un-intuitive part, i think.
h
Yes, publishing to PyPI is one hell of a side effect...
w
if possible, factoring this as "build all the artifacts, and then have the console_rule do the actual publishing" would be better.
i mean, in pantsbuild's usage of pypi, we do that via twine, right?
ditto ivy/maven publishing: prepare all of the things that you might upload, and then attempt to atomically upload at the end.
h
Well, we don't upload ourselves
the setup.py command does
w
no
twine does.
h
well, may or may not
w
sure. in this case, architecturally, discouraging that would be good.
h
You can upload with 
sdist upload
as your setup.py command
build and upload, that is
We can disallow this I suppose
Twine is definitely a better way to do this
but I don't know that we can or should prevent people from doing this the distutils way
w
we should, probably.
h
We don't provide an alternative
Other than "here's everything in dist/, now you figure out how to upload it"
which is actually fine with me, for now, as I think about it.
w
er... i pointed to one above? you can invoke twine for them
i would call it the first implementation of a 
publish
goal, parallel to a future JVM v2 publish using ivy.
h
you can invoke twine for them outside of pants, but that's not really us providing a solution
For example we're not helping solve the dependency order issue, you'd have to do that by ordering your twine invocations somehow
But yeah we can create a 
publish
goal that invokes twine and sorts that out
And then basically the setup-py goal shouldn't care about order, and we just have to recommend or enforce that it doesn't attempt to publish
❤️ 1
w
you can invoke twine for them as a foreground interactive process.
you're right that ordering would matter in that case.
yea, that sounds good.
Open in Slack
PreviousNext
Powered by