https://pantsbuild.org/ logo
Join Slack
Powered by
In v2.0.0, do target names like `"//:house/cellar"...
# general
j
In v2.0.0, do target names like 
"//:house/cellar",
still work? We have names like this for 
python_library
target_types and they work in v1.25.
h
That means a target defined in 
<build root>/BUILD
with the field 
name="house/cellar"
Is that what you meant? Or do you mean 
//house/cellar
aka 
house/cellar
?
j
The former. In the directory 
REPOROOT/house/cellar
there is a 
BUILD
file with a python_library with the field 
name=house/cellar
. CORRECTION: the build file is in the root, see below
I'm getting an error when running tests (pytest running 
unittest
)
Copy code
raise NoSourceRootError(source_root_request.path)
pants.source.source_root.NoSourceRootError: No source root found for `.`. See <https://www.pantsbuild.org/docs/source-roots> for how to define source roots.
When I don't include 
//:house/cellar
(a dependency in the test) this error goes away.
And the BUILD file is actually not in 
REPOROOT/house/cellar
It is at 
REPOROOT/BUILD.house
. 😓 (trivia: created 5 years ago)
h
Hm, It’s not super kosher to use 
/
in the target name. Normally, if you have 
house/cellar/BUILD
, and a 
python_library()
in it, you would leave off 
name
, or set 
name=lib
. Then, the address combines to be 
house/cellar:lib
> //:house/cellar The 
//
means “start from the build root”, then the 
:house/cellar
means “I have a target whose 
name='house/cellar'
. So, 
//:house/cellar
should only work if you have a top-level BUILD file in your build root, and a target there with 
name="house/cellar"
. I don’t recommend doing that though, as it’s confusing
j
We do have that.
And it is confusing 🙃
This is ancient code that weaves throughout our repo. If I don't have to change it for v2 to drop, I would rather keep it the same.
From what you said, I'll infer that this is not recommended, not tested, and you see no reason it shouldn't work.
h
Okay. So it looks the address is correct - it’s not saying 
ResolveError
that it can’t find The issue is 
SourceRootError
. I’m not sure what’s going on with that. Do you have the full stacktrace?
j
Yes. 1 sec
@hundreds-father-404
stack trace of failed test
FYI: 
./pants filedeps :: | grep house
returns the files in 
house/cellar
and all the rest (as far I can tell).
I think I may have a clue
👍 1
h
what does 
./pants help-advanced source
say?
j
The 
BUILD.house
file does not have 
__init__.py
in the all sources.
house
is listed as a 
root_pattern
OMG. There are python_libraries defined on subsets of the python code in a directory. I have to manually add 
__init__.py
to them
👀 1
gordian knot
Let me get set this right and try again.
Thank you for being a sounding board.
❤️ 1
h
Hm, confusing. Generally, we strongly recommend one BUILD file per directory, and ~one target per directory. See https://www.pantsbuild.org/docs/targets#target-granularity. It gets far less confusing, e.g. thanks to principle of proximity
j
Did you recommend that in 2015?
h
I think so, but I wasn’t around back then. If so, under the name 1-1-1
j
I wasn't here, so I am not sure what happened.
h
It’s why the default 
sources
field is set how it is for 
python_library()
and 
python_tests()
. They don’t use recursive globs. You of course can use recursive globs like 
**/*
, only, we find it often doesn’t scale as well and gets confusing.
j
My guess is this code pre-dates our use of pants and someone said "You don't have to refactor house. Just do this in `BUILD.house`" type type type
🙃 1
OMMG There are python_libraries that span three directories.
❗ 1
h
Fyi you can run 
./pants list path/to/f.py
to see which targets “own” that file
j
finding "unowned file" errors with that
h
interesting, that would mean there is no target whose 
sources
field includes that file
j
let me try with one that is explicitly owned...
👍 1
Cool. It found multiple owners
I think we need to catch those errors so things like 
house/cellar/*.py
doesn't fail on it
If we know they are just targets without anything referencing them.
Oh. found 
--owners-not-found-behavior=ignore
👍 1
But it would be nicer if I could have the output be something like:
Copy code
//house/cellar/washing_machine.py:../../house/cellar
//house/cellar/witch.py: None (no owner found)
//house/cellar/mouse.py:../../house/cellar
h
I don’t recommend using 
=ignore
. If you use 
'*.py'
with the quotes, then Pants will expand the glob for you and won’t care about files without owners. It only errors if you use a file literal, because we assume you are being specific enough that you should know something went wrong
👍🏽 1
Fine for debugging, of course. I more mean for a permanent 
pants.toml
setting
j
not every 
.py
is owned. 😕
Does 
pants list
also work with 
resources
?
h
Yes, it works with any target type. 
./pants list path/to/f.json
for example
🚀 1
j
What is unfortunate about this is that the error message is too vague. I've narrowed it down to this dependency, but I'm not sure what in this dependency is broken (other than 1-1-1). I need to refocus on this. Get it working and then go back.
h
Oh, likely relevant. In v1, I think we did this bad thing of defaulting to creating the source root if it wasn’t already set. We don’t do that anymore. You have the file 
house/cellar/mouse.py
. What is your import statement like? Are you saying 
from house.cellar.mouse import Mouse
, or 
from cellar.mouse
or 
from mouse
?
I suspect that the BUILD file is a red herring. Even if you move the BUILD file, I suspect you’ll still hit this issue. I suspect it’s more an issue with how source roots are configured
j
weird
our v1.26 
pants.ini
does not have it listed in 
source.source_roots
.
h
Bump on this
What is your import statement like?
j
from house.cellar import mouse
h
Okay, so make sure that 
/
is in 
./pants help-advanced source
for 
--root-patterns
, which means the build root. That import means that you have no source root here. You are not stripping anything
And you need 
house/__init__.py
and 
house/cellar/__init__.py
iiuc. I don’t think you need 
<build root>/__init__.py
though, and that would likely break a ton of things
j
If I add 
"."
to our source root, then the error changes
h
Add 
/
, rather than 
.
, I believe
j
.ti did tahT
That did it.
💯 1
I was hoping it would be enough to have 
house
listed in the 
root_patterns
but I guess because they are called with 
house.cellar.mouse
that won't work.
😭
h
Yeah, if you have 
house
, then that means that your imports would be 
from cellar.mouse import Mouse
j
This all makes sense.
We have four projects that all live at the top of the repo.
That is for another time.
💯 1
Thank you!
❤️ 1
Do I need 
pants-plugins
in the 
root_patterns
? @hundreds-father-404
(This is where our plugins live. I have 
BUILD
files in there too because we used to have 
BUILD
fils in our v1 plugins.)
h
That depends if you want to run 
./pants fmt
, 
lint
, etc on your plugin code. I strongly recommend doing that. See https://www.pantsbuild.org/docs/plugins-overview#building-in-repo-plugins-with-pants
j
🚀
So I need to keep 
pants-plugins
in 
source.root_patterns
.
👍 1
h
tl;dr, you don’t have to run 
./pants fmt
etc on your plugins, but we recommend it. Treat your plugins like any other code, that you want them to work robustly
👖 1
j
@hundreds-father-404 Do you know how to troubleshoot
Copy code
"A target may only be defined in a directory containing a file that it owns in "
pants.build_graph.address.InvalidTargetName: A target may only be defined in a directory containing a file that it owns in the filesystem: `../../house/cellar` is not at-or-above the file `house/cellar/__init__.py`.
Looking at 
./pants dependencies "//:house/cellar"
I see things like 
//house/cellar/__init__.py:../../house/cellar
and 
//house/cellar/__init__.py:../../house/cellar/constants
. Remember these target names are 
house/cellar
instead of something less confusing like 
house_cellar
.
That seems to be abnormal compared to our newer projects that have reasonable target names and are more 111.
Could it be that because our 
name
has `/`'s in them that the above code treats the target name as a directory?
I think there might actually be two issues happening. I just added this to the description:
Copy code
This target is defined in a BUILD file at the top of the repo. When I ran debug, I get back changed addresses that look like Address(//action/cellar/mouse.py:../../action/cellar_tests). Shouldn't it look like Address(//action/cellar/<mouse.py://action/cellar_tests>)? I don't think whatever is generating the Address() objects is handling targets defined in top level BUILD files correctly.
w
i think that the root cause is that target names with slashes in them have not been well supported.
and we added new syntax for file dependencies
we might be able to make slash-named targets work, but we might also consider deprecating them
j
Maybe take a poll. Slash names are confusing. I was able to refactor the names last night.
What about top level 
BUILD
files? It seems with file dependencies and 
../../target_name
there is lots of opportunities to be confused.
I don't think you should depreciate them but it seems the 
Address(file/path/file.py:../../target_name)
format is also confusing. Paths to target names are needed so different projects can have the same target name, but targets defined in a top level 
BUILD
files should always start with 
//
IMHO. What is the advantage of having it be relative?
w
@jolly-midnight-72759: i’ll investigate the ticket soon, sorry: had a busy morning
but:
What about top level 
BUILD
 files? It seems with file dependencies and 
../../target_name
 there is lots of opportunities to be confused.
i expect that this was a parsing issue caused by the slash in the name, rather than an issue with targets at the root of the reposiutory
part of the reason that we made the syntax change to include the 
../..
(etc) in the rendered address though is to encourage layouts with targets living closer to the files they own (in which case their relative path is shorter)
but yea, i’ll investigate… sorry that we broke this. glad that you were able to rename those
j
No need to apologize. The naming convention needed to be changed. We also need to move those top level projects, but they are of course both core and entangled with the entire codebase.
6 Views
Open in Slack
PreviousNext
Powered by