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

billowy-motherboard-58443

09/21/2020, 4:06 PM
or in other words, each imports from the other (config <=> util) ?
a

aloof-angle-91616

09/21/2020, 4:07 PM
so i personally have this ridiculous theory where i think circular dependencies can actually work in some cases but 99% of people find them extremely confusing so we don't support them
that would be a cycle of length 1
is it easily possible to split out the code that e.g.
util
depends on from
config
? or the other way around?
b

billowy-motherboard-58443

09/21/2020, 4:08 PM
ugh... this is what happens when you go from ONE BIG LIB to monorepo of little libs...
hmm
actually
a

aloof-angle-91616

09/21/2020, 4:09 PM
the output is incredibly helpful, thank you. in this case if you can move out
ConfigEntry
from
gh/config
into its own target, that should work
b

billowy-motherboard-58443

09/21/2020, 4:09 PM
config wants util, but util/TEST wants config...
💡 1
a

aloof-angle-91616

09/21/2020, 4:09 PM
i think you've got it
this is part of why it's nice to duplicate dependencies sometimes in parent and child targets -- makes it easier to extend to the rest of the repo
b

billowy-motherboard-58443

09/21/2020, 4:11 PM
ok, now
./pants dependencies --transitive gh/config
does not blow up
a

aloof-angle-91616

09/21/2020, 4:12 PM
that is great to hear
b

billowy-motherboard-58443

09/21/2020, 4:14 PM
but in general.... the deps of a tests subdir should "inherit" from the parent package dir's BUILD:dependencies, yes?
h

hundreds-father-404

09/21/2020, 4:15 PM
Are you still using 1.30 or 2.0? If you use 2.0 and use dependency inference or “explicit file addresses”, we actually do now tolerate cycles
a

aloof-angle-91616

09/21/2020, 4:16 PM
it will always work to assume that a child inherits from the parent's dependencies
omg yes
it's 1.30 @hundreds-father-404
but that's a great tidbit about 2.0
b

billowy-motherboard-58443

09/21/2020, 4:16 PM
so... what's my answer? 🙂
and should I upgrade to 2.0 ?
a

aloof-angle-91616

09/21/2020, 4:17 PM
sorry:
it will always work to assume that a child inherits from the parent's dependencies
was this your question? or were you asking for best practices?
h

hundreds-father-404

09/21/2020, 4:17 PM
it will always work to assume that a child inherits from the parent’s dependencies
I don’t think this is quite the right relationship, as “parent” to me sounds like “parent directory” vs. “subdirectory”. It’s not about the relationship in the file system. Generally, depending on a dependency means that Pants will then pull in all transitive dependencies
a

aloof-angle-91616

09/21/2020, 4:18 PM
ok, but "parent" is very often used to refer to trees in general, which is why filesystems use it
h

hundreds-father-404

09/21/2020, 4:18 PM
so... what’s my answer? 🙂
I’m still trying to catch up. Generally, I would recommend upgrading to 2.0. Dependency inference will do the right thing for you in most cases, so that you don’t have to manually struggle with getting dependencies right. This is a huge reason we added the feature It will also handle this cycle fine for you. If Python is fine with the cycle, Pants will be fine with it
a

aloof-angle-91616

09/21/2020, 4:18 PM
and should I upgrade to 2.0 ?
yes, absolutely imho
b

billowy-motherboard-58443

09/21/2020, 4:19 PM
ok, will swap it around to 2.0
h

hundreds-father-404

09/21/2020, 4:19 PM
Because you’re still setting up Pants, I doubt there will be any big changes you need to make, beyond: https://www.pantsbuild.org/docs/how-to-upgrade-pants-2-0#step-4-upgrade-to-200 The “Key Differences To Be Aware Of” is probably irrelevant
☝️ 1
b

billowy-motherboard-58443

09/21/2020, 4:19 PM
back in a bit
👋 1
a

aloof-angle-91616

09/21/2020, 4:19 PM
let us know if you have difficulties with that!
b

billowy-motherboard-58443

09/21/2020, 4:21 PM
ERROR: Could not find a version that satisfies the requirement pantsbuild.pants==2.0
h

hundreds-father-404

09/21/2020, 4:21 PM
2.0.0b0
a

aloof-angle-91616

09/21/2020, 4:21 PM
we just released a beta^
b

billowy-motherboard-58443

09/21/2020, 4:21 PM
heh
a

aloof-angle-91616

09/21/2020, 4:21 PM
sorry about that
b

billowy-motherboard-58443

09/21/2020, 4:21 PM
"We highly recommend our beta..."
a

aloof-angle-91616

09/21/2020, 4:22 PM
we do!
h

hundreds-father-404

09/21/2020, 4:26 PM
I think that’s more a “huh, that seems strange to recommend a beta rather than stable reason”. Ack. We’re close to graduating it to a release candidate, then stable. There’s some Pants history with why this is 2.0 vs. 1.30, and why we recommend 2.0 over 1.30 even though it’s a beta. For the past ~2 years, we’ve been rewriting Pants to use the the new v2 engine, which is a ground up rewrite based on the past 10 years of lessons. You were already using this v2 engine in 1.30, but Pants 1.30 also supported the v1 engine for JVM/JS/Go support, which was majorly slowing down development. We made a hard break with 2.0 to remove the v1 engine Normally, by now, we would have released 1.34.0 and you would have had these improvements like dependency inference. But 2.0 has been a drawn out release because we’re trying to get as much polish as we can before the stable release.
a

aloof-angle-91616

09/21/2020, 4:26 PM
it's really sick
b

billowy-motherboard-58443

09/21/2020, 4:26 PM
pile of Deprecation warnings
a

aloof-angle-91616

09/21/2020, 4:27 PM
oh, sorry about that. hopefully they should be direct enough to fix
b

billowy-motherboard-58443

09/21/2020, 4:27 PM
I did
rm -rf
on the ~/.cache/pants dir and just did
pants --version
to reinstall
h

hundreds-father-404

09/21/2020, 4:27 PM
Did you see that one section of the upgrade guide? It should knock out most of them
b

billowy-motherboard-58443

09/21/2020, 4:28 PM
missed a few lines there
yes, now better
👍 1
Copy code
$ ./pants --version
Scrubbed PYTHONPATH=/home/mfischer/src/ghpylibs/python from the environment.
16:28:18.65 [INFO] initializing pantsd...
16:28:19.43 [INFO] pantsd initialized.
2.0.0b0
🔥 1
👖 1
h

hundreds-father-404

09/21/2020, 4:28 PM
Excellent
a

aloof-angle-91616

09/21/2020, 4:29 PM
awesome
b

billowy-motherboard-58443

09/21/2020, 4:29 PM
oh fun... tons of pytest
[WARN]
now
and bitching about what is not mentioned in constraints.txt
a

aloof-angle-91616

09/21/2020, 4:30 PM
hmmm. it probably bumps the pytest version. you should be able to set that back to the version you were using
h

hundreds-father-404

09/21/2020, 4:30 PM
So, generally, you can leave off the
dependencies
field. Pants will infer based on your Python imports. You can run
./pants dependencies
to see what it’s inferring You only need to fill it in when Pants cannot figure out the inference, such as when you depend on a
files
or
resources
target (e.g. JSON files)
b

billowy-motherboard-58443

09/21/2020, 4:30 PM
e.g.
Copy code
16:29:20.05 [WARN] The constraints file constraints.txt does not contain entries for the following requirements: colorama, python-dateutil, mock, pytest, pytz, marshmallow
16:29:20.05 [WARN] Ignoring resolve_all_constraints setting in [python_setup] scope because constraints file does not cover all requirements.
a

aloof-angle-91616

09/21/2020, 4:30 PM
ah ok
h

hundreds-father-404

09/21/2020, 4:30 PM
hmmm. it probably bumps the pytest version. you should be able to set that back to the version you were using
Yes, it does do this. See https://www.pantsbuild.org/docs/python-test-goal#pytest-version-and-plugins for how to change the version to be Pytest 5
b

billowy-motherboard-58443

09/21/2020, 4:31 PM
Copy code
[WARN] <unknown>:587: DeprecationWarning: invalid escape sequence \{
I'm guessing that it is failing to tell me which file: <unknown> ?
(FWIW the tests did pass...)
💯 1
h

hundreds-father-404

09/21/2020, 4:31 PM
[WARN] <unknown>587 DeprecationWarning: invalid escape sequence \{
Sorry, I think that’s from dependency inference. We’ll fix this in the next beta release. Pardon the temporary noise
b

billowy-motherboard-58443

09/21/2020, 4:33 PM
that's actually from python / pytest
I see these already under plain pytest (but it tells me the filename)
funny you should mention resource files
h

hundreds-father-404

09/21/2020, 4:33 PM
Ignoring resolve_all_constraints setting in [python_setup] scope because constraints file does not cover all requirements.
So this warning is saying that we can’t use a performance optimization that avoids having to re-resolve requirements multiple times. We use a single global resolve (like a single venv). It makes things much faster. https://www.pantsbuild.org/docs/python-third-party-dependencies#tip-set-up-a-virtual-environment-optional has a script you may find helpful to use Pants to query for your dependencies and then generate a constraints.txt You can alternatively turn off this optimization and silence the message by setting
resolve_all_constraints = never
in
[python-setup]
in
pants.toml
b

billowy-motherboard-58443

09/21/2020, 4:34 PM
yeah, I haven't gotten to getting nit-picky with that file yet
(esp. because it contains both what my repo needs and seemingly what pants itself needs)
h

hundreds-father-404

09/21/2020, 4:35 PM
I see these already under plain pytest (but it tells me the filename)
It sounds like Pytest is duplicating the warning, but yes, with the filename? Imo, Pants shouldn’t be warning in the first place. It’s warning because Pants parses the AST to be able to infer dependencies. We need to catch the DeprecationWarning
a

aloof-angle-91616

09/21/2020, 4:35 PM
@hundreds-father-404 it sounds like we could maybe include that link in the error message?
h

hundreds-father-404

09/21/2020, 4:35 PM
seemingly what pants itself needs)
Are you seeing things that you don’t expect? It should only be what your own requirements are. Note that Pytest is showing up if you have
import pytest
anywhere
b

billowy-motherboard-58443

09/21/2020, 4:35 PM
if I run
py.test -s gh/config/tests
it will tell me
Copy code
gh/config/tests/fig_client/test_adapter.py:56
  /src/gh-pants-test/gh/config/tests/fig_client/test_adapter.py:56: DeprecationWarning: invalid escape sequence \.
    "nested.escaped\.<http://thing.int|thing.int>": "1",

gh/config/tests/fig_client/test_adapter.py:57
  /src/gh-pants-test/gh/config/tests/fig_client/test_adapter.py:57: DeprecationWarning: invalid escape sequence \.
    "nested.escaped\.thing.str": "horse",

gh/config/fig_client/adapter.py:587
  /src/gh-pants-test/gh/config/fig_client/adapter.py:587: DeprecationWarning: invalid escape sequence \{
    delim = "\{0}".format(delim)
so I think something you're doing there is turning the filenames into
<unknown>
👍 1
🤷
a

aloof-angle-91616

09/21/2020, 4:37 PM
that is very helpful feedback
we also saw the
/tmp/process-execution232141
prefix in the import error message from before, we should be able to scrub those error messages
b

billowy-motherboard-58443

09/21/2020, 4:38 PM
ok, so here's something you might enjoy
I removed ALL deps from gh/config/BUILD
gh/config/tests/BUILD has only the resource files...
but it now complains that it can't find
.exceptions
which turns out to be coming from gh/config/__init__.py doing
from .exceptions import....
👍 1
so the tests/BUILD needs dep on "gh/config"
✍️ 1
a

aloof-angle-91616

09/21/2020, 4:40 PM
that's quite interesting
b

billowy-motherboard-58443

09/21/2020, 4:40 PM
things pointing at e.g. "gh/util" resolve without being mentioned, but something in an init parent did not
h

hundreds-father-404

09/21/2020, 4:40 PM
I got a bit lost in the threads from this morning. What did you end up having your BUILD files look like? Did you use one BUILD file per directory, or use one BUILD file for subdirectories too by using
**/*.py
?
but something in an init parent did not
What do you mean? Is the content in the
__init__.py
file?
b

billowy-motherboard-58443

09/21/2020, 4:41 PM
yes
from .exceptions import ConfigException     # noqa (backwards compatibility, as ConfigException used to live here)
I think the ConfigException class used to be defined in config/__init__.py and it was moved to config/exceptions.py - so that import is for back compat callers
hmm
should the test target recursively dig down ?
h

hundreds-father-404

09/21/2020, 4:44 PM
so the tests/BUILD needs dep on “gh/config”
Taking a step back, dependencies in 2.0 are more fine-grained. Generally, dependencies are between files, like file A depends on file B. Here, we want to make sure that the correct files are depending on the correct files, rather than the “entire target”, which ends up depending on every single file, even if you only need a few of them Sorry for the confusion - this changed a bit between 1.30 and 2.0
b

billowy-motherboard-58443

09/21/2020, 4:44 PM
e.g. I have gh/core/tests and gh/core/tests/concurrency/
(np on whatever between 1.3 and 2.0... just exploring them this past week, not deeply invested yet)
seems like the test goal did not go down from tests/ to tests/concurrency/
do I need BUILD files all the way down or is there an incantiation for the top of the tests/BUILD ?
h

hundreds-father-404

09/21/2020, 4:46 PM
I’m not following along super well. Generally, we recommend one BUILD file per directory. If you’re using dependency inference, ti would look like this for most BUILD files (if tests are colocated with source files):
Copy code
python_library()

python_tests(name="tests")
b

billowy-motherboard-58443

09/21/2020, 4:46 PM
ok, so
I have
ns/pkg, ns/other-pkg, etc.
and each pkg has a tests subdir
there may be nestings of dirs under tests
right now I have BUILD at top of repo, under each pkg, and under each pkg/tests/
👍 1
h

hundreds-father-404

09/21/2020, 4:47 PM
You can have fewer BUILD files, but we find that it becomes hard to reason about. It’s often simplest to have one BUILD file per directory, and 1
python_library
per directory, 1
python_tests
per directory, etc. (Unless you have a reason to define things differently, like bump a timeout for just one test)
b

billowy-motherboard-58443

09/21/2020, 4:48 PM
I'd have thought that
python_library
would only be declared at the top dir OF THAT LIBRARY?
or in my example, at ns/pkg and ns/other-pkg
not in each pkg's subdirectories
h

hundreds-father-404

09/21/2020, 4:52 PM
That’s good to know that’s your expectation, I see why you’re thinking that way. And you can make it work that way, indeed. Generally, the mental model for a target is “metadata attached to some code”. When you have only one target for the entire package and subdirectories, you’re saying that every file in it has the same metadata. Often, that’s true. But, why we recommend more granularity is that it will scale better. For example, say that
ns/pkg/subdir/f.py
depends on a JSON file. You must explicitly declare that in the
dependencies
field for the target that refers to
f.py
. If you have only one
python_library
for
ns/pkg
, then you’d end up saying that every single file in
ns/pkg
depends on that JSON file, even though that’s likely not true. This results in worse invalidation So, having one BUILD file per directory usually a) is easier to reason about and b) scales better
b

billowy-motherboard-58443

09/21/2020, 4:53 PM
I think we've got crossed here
I've got e.g. gh/config/tests/BUILD mentioning some json data files (because the tests use them) just fine
but the library gh.config doesn't want those
currrent Q is "why doesn't the test goal recursively go down through the
tests/
dir ?"
i.e. it doesn't run gh/config/tests/foo/test_foo.py
only the test.py or test.py files in gh/config/tests/ are getting run
h

hundreds-father-404

09/21/2020, 4:56 PM
Right. And JSON was probably a bad example for me to use in my explanation because you are actually indeed using JSON. What I meant generally is that you will sometimes need to tweak your metadata from the default, even if the default was fine at first. If you have one BUILD file per directory, that scales much better
why doesn’t the test goal recursively go down through the tests/ dir ?”
Ah, run
./pants test tests::
. This is a glob to say “every relevant target in tests and subdirectories”
b

billowy-motherboard-58443

09/21/2020, 4:58 PM
yep, that seems to help
👍 1
h

hundreds-father-404

09/21/2020, 4:58 PM
(https://www.pantsbuild.org/docs/goals#file-arguments-vs-target-arguments has more about globs, including the
:
glob for all relevant targets in the directory, but not subdirectories. And file globs with
*
and
**
)
So to confirm, are things working properly now?
./pants dependencies path/to/file.py
will show you which dependencies that specific file has. It should map to what your actual imports are, and is useful when debugging things like “why is this import failing”
b

billowy-motherboard-58443

09/21/2020, 5:03 PM
kk
well, just ran into
ns/foo/tests/ <-- has only subdirs with test files
so
pants test ns/foo/tests::
is saying
Copy code
Unmatched globs from gh/garcon/tests's `sources` field: ["gh/garcon/tests/*_test.py", "gh/garcon/tests/conftest.py", "gh/garcon/tests/test_*.py", "gh/garcon/tests/tests.py"]
h

hundreds-father-404

09/21/2020, 5:05 PM
Okay, so if you want one single target located at
ns/foo/tests
and for it to cover all subdirectories, you need to change the
sources
field:
Copy code
python_tests(sources=["**/*_test.py", "**/conftest.py"])
b

billowy-motherboard-58443

09/21/2020, 5:06 PM
hmm, removed stale pycache and pyc files, now it just returns without output
h

hundreds-father-404

09/21/2020, 5:06 PM
Or, have one BUILD file per subdirectory. Either approach works, depends on the granularity you want The default sources field will only look for the current directory. If you use something else, it needs to be changed.
b

billowy-motherboard-58443

09/21/2020, 5:06 PM
kk, I'll tr y that
seems to be working
💯 1
(the explicit sources thing)
h

hundreds-father-404

09/21/2020, 5:10 PM
Cool, glad to hear
b

billowy-motherboard-58443

09/21/2020, 5:10 PM
FWIW, this excercise is on a rather large repo which has lived as long as the company ....
it is opinionated on its layout, as are its maintainers so... 🙂
h

hundreds-father-404

09/21/2020, 5:11 PM
Were y’all using a build tool before?
b

billowy-motherboard-58443

09/21/2020, 5:11 PM
fewer BUILD files is an aim of this experiment
for this lib? Not really
CI =>
tox --
and "Build"
python setup.py.....
but the desire has been expressed for it to be broken up into a monorepo of ns.core, ns.util, ns.foo, etc..
so that folks can have more granular installs of the deps of apps which use this lib
h

hundreds-father-404

09/21/2020, 5:16 PM
It is opinionated on its layout, as are its maintainers
We generally make it a goal for Pants to work with multiple different workflows / layouts. For example, allowing you to set source roots If there are particular features that would help you all out, please let us know. A lot of functionality is driven specifically by user interest. For example, a user wanted to be able to indicate source roots based on the presence of a marker file (
setup.py
). We had never used that workflow, but added it to support their layout. In other words, we don’t want you to have to rewrite your whole project to use Pants 😄 (Although, we hope that it will help encourage good practices at the same time, and bring you benefits like hermiticity, caching, and concurrency)
fewer BUILD files is an aim of this experiment
Cool. This was a major reason for two changes we made in 2.0: a) dependency inference, and b) being able to depend explicitly on files, rather than having to depend on the entire target and pulling in every single file. While I was advocating “one BUILD file per directory”, it is totally possible and valid to have fewer BUILD files like you’re doing now in 2.0. That was not really feasible in 1.x where you had to explicitly declare dependencies, but is now very valid
python setup.py.....
If you haven’t yet, check out https://www.pantsbuild.org/docs/python-setup-py-goal when you’re ready. With
./pants setup-py
, you will only define one
python_distribution
target per package, like you’d expect. Let us know if we can help with getting that going
b

billowy-motherboard-58443

09/21/2020, 5:25 PM
what is the diff between
python_distribution
and
python_library
?
I could swear I tested
bdist_wheel
with this and used
python_library
and not distribution....
h

hundreds-father-404

09/21/2020, 5:28 PM
A python_distribution is metadata about how to package and distribute your code via the
setup-py
goal. You can set things like the
version
and
description
Meanwhile
python_library
is metadata about any Python source files you have.
b

billowy-motherboard-58443

09/21/2020, 5:28 PM
ah. mmm
h

hundreds-father-404

09/21/2020, 5:31 PM
Ah yes, in 1.30, you used to define the
provides
field on
python_library
.
python_distribution
is new to 2.0 to clean up the modeling Again, sorry for the confusion of switching between them. We only changed the docs to default to 2.0 on Friday because we had to rewrite some parts to talk about things like dependency inference. So if you started Pants earlier, that explains why you were using 1.30
b

billowy-motherboard-58443

09/21/2020, 5:32 PM
indeed, got started on this about a week ago
👍 1
ok, so in say... gh/core (a lib we want to publish)
I'd have both
python_distribution
and
python_library
stanzas?
or just distribution?
hmm, sees I tweaked the python_library.sources earlier today to get the */.py thing to chase down nested files
so likely both?
h

hundreds-father-404

09/21/2020, 5:37 PM
I’d have both python_distribution and python_library stanzas?
Yep, you’d have something like this:
Copy code
python_library(
  name="lib", 
  sources=["**/*.py"]
)

python_distribution(
  name="dist"
  dependencies=[":lib"],
  provides=setup_py(
     name="gh-config-dist",
     version="1.0.1",
     ...
   )
)
The
python_library
is attaching metadata to your actual
.py
files. For example, what dependencies each
.py
file has. Or, what
compatibility
it has for Python versions Meanwhile,
python_distribution
is metadata about your distribution you want to build
b

billowy-motherboard-58443

09/21/2020, 5:37 PM
$ ./pants setup-py gh/core
h

hundreds-father-404

09/21/2020, 5:38 PM
Note that
python_distribution
does not work with dependency inference. Why? It doesn’t have any source files, so Pants can’t analyze imports. Instead, you use an explicit dependency Here, I’m assuming that you want to depend on “everything in the
lib
target, and everything that those files in the target depends on too”
b

billowy-motherboard-58443

09/21/2020, 5:38 PM
Copy code
TargetNotExported: Cannot run setup.py on these targets, because they have no `provides=` clause: gh/core/collections.py, gh/core/concurrency/__init__.py:../core, gh/core/log.py, gh/core/logging/formatter.py:../core, gh/core/property.py, gh/core/tests/concurrency/executor_test.py:../../core, gh/core/util.py, gh/core/ioc/service_locator.py:../core, gh/core/retry.py, gh/core/__init__.py, gh/core/ioc/__init__.py:../core, gh/core/ioc/exceptions.py:../core, gh/core/tests/retry_test.py:../core, gh/core/logging/__init__.py:../core, gh/core/tests/collections_test.py:../core, gh/core/tests/ioc/service_locator_test.py:../../core, gh/core/patterns.py, gh/core/concurrency/executor.py:..
h

hundreds-father-404

09/21/2020, 5:38 PM
Run
./pants setup-py gh/core:dist
Or, if you would rather be able to do
./pants setup-py gh/core
, you can leave off the
name
field for the
python_distribution
target. If you leave off
name
, it will default to the name of the directory
(Running
./pants setup-py gh/core
is shorthand for
./pants setup-py gh/core:core
)
b

billowy-motherboard-58443

09/21/2020, 5:39 PM
hmm
Copy code
./pants setup-py gh/core:dist
17:39:37.01 [ERROR] 1 Exception encountered:

  ResolveError: 'dist' was not found in namespace 'gh/core'. Did you mean one of:
  :core
  :gh.core
h

hundreds-father-404

09/21/2020, 5:40 PM
what does your BUILD file look like?
b

billowy-motherboard-58443

09/21/2020, 5:41 PM
Copy code
python_library(
    sources=['**/*.py'],
    dependencies=[],
)

python_distribution(
    name="gh.core",
    dependencies=[ ],
    provides=setup_py(
        name="gh.core",
        version="0.1",
        description="this is gh.core",
        author="mycorp",
        classifiers=[
            "Programming Language :: Python :: 3.6",
        ],
        namespace_packages=['gh.core'],
    )
)
h

hundreds-father-404

09/21/2020, 5:43 PM
Ah, sorry, miscommunication I think. What you have here is that
python_library
has the address of
gh/core:core
, which you can also use
gh/core
as shorthand. This is because you left off the
name
field. You explicitly set
name
to
gh.core
for the
python_distribution
, so its address is
gh/core:gh.core
-- I suspect that instead, you want to leave off
name
for
python_distribution
. This will result in the address
gh/core:core
, which you can use
gh/core
as shorthand. But, if you do this, you need to add something like
name="lib"
to the
python_library
. Each address must be unique, so you can only have one target in each directory that leaves off the
name
field
b

billowy-motherboard-58443

09/21/2020, 5:45 PM
but the setup_py(name="gh.core") should remain, yes?
(FWIW
Copy code
ValueError: namespace_packages cannot be set in the `provides` field for gh/core, but it was set to ['gh.core']. Pants will dynamically set the value for you.
)
h

hundreds-father-404

09/21/2020, 5:47 PM
Yes, that
setup_py(name="gh.core")
is the name of the actual distribution, like what the wheel will be called. It’s different than Pants’s address to the
python_distribution
target. That’s pretty confusing. I think it might make more sense if we called it
dist_name
? Generally, that
setup_py
thing uses the same keyword args you would use in a
setup.py
, which is why we stuck with
name
. But it could be worth diverging here.
b

billowy-motherboard-58443

09/21/2020, 5:48 PM
so,
./pants setup-py gh/core
worked, but it created an empty dist/gh.core-0.1/src/ dir....
h

hundreds-father-404

09/21/2020, 5:49 PM
Because
dependencies
is empty. Add
dependencies=[":lib"]
b

billowy-motherboard-58443

09/21/2020, 5:50 PM
and lib.sources is where I would filter out the tests dirs from the pkg, yes?
h

hundreds-father-404

09/21/2020, 5:51 PM
Yes, exactly. You’ll want
gh/config:lib
to have a sources like
sources=["**/*.py", "!**/*_test.py"]
You can run
./pants dependencies --transitive gh/config
to spot check what is going to end up in the
python_distribution
b

billowy-motherboard-58443

09/21/2020, 5:52 PM
to leave out the whole tests dir, ["**/*.py", "!tests"] ?
seems to have worked
now the killer test, bdist_wheel, install and import it in ipython
ouch
ERROR: gh.core-0.1-py2-none-any.whl is not a supported wheel on this platform.
👀 1
h

hundreds-father-404

09/21/2020, 5:55 PM
What command did you run? You may want to do
./pants setup-py gh/confg -- bdist_wheel --python-tag=py36.py37
for example
b

billowy-motherboard-58443

09/21/2020, 5:56 PM
./pants setup-py gh/core -- bdist_wheel
what does
--python-tag
do here?
(it "worked" at any rate)
wooohoo
🔥 1
Copy code
In [1]: import gh.core                                                                                               

In [2]: from gh.core.log import class_logger
h

hundreds-father-404

09/21/2020, 5:58 PM
That’s a flag to
bdist_wheel
. Anything after the
--
gets passed through It tells the
bdist_wheel
program that it’s compatible with Py36 and Py37, for example I’m starting to get out of my expertise, though. @happy-kitchen-89482 do you know why Michael is having to set
--python-tag
to get the wheel built properly? (Only the last few messages are relevant, you can ignore most this thread)
b

billowy-motherboard-58443

09/21/2020, 5:58 PM
I was thinking it was missing what usually goes in setup.cfg, eg.
Copy code
[bdist_wheel]
universal=1
but I too am out of my depth there
h

hundreds-father-404

09/21/2020, 5:59 PM
Ahh, yes. That’s what’s going on. That is an alternate way to tell
bdist_wheel
what this is compatible with
b

billowy-motherboard-58443

09/21/2020, 6:01 PM
Much progress today - Thank you!
I need coffee, and then to test the
test --changed-since=origin/master
trick with 2.0
(which was honestly the biggest selling point in my org - saves us having to write "which pkgs need to be retested?" code ourselves)
h

hundreds-father-404

09/21/2020, 6:02 PM
You might be able to define a
resources
target for your
setup.cfg
file and add it to the
dependencies
field of your
python_distribution
. I suspect it would fix it We are also considering redesigning/improving some of the
setup-py
stuff so that you can associate certain passthrough arguments permanently to a
python_distribution
target
then to test the test --changed-since=origin/master trick with 2.0
Yay! One of my favorite features. I use
./pants --changed-since=HEAD fmt
all the time For
test
, often you will want to use
--changed-dependees=transitive
. This means that if you change source file a.py, and b.py depends on it, then test_b.py will rerun because it knows a transitive dep changed Also, you may find
./pants --changed-since=origin/master list
helpful when experimenting with the behavior. That will list every file that is picked up, without the cost of actually running tests
b

billowy-motherboard-58443

09/21/2020, 6:05 PM
yep, I found it to work on your example repo before, but need to retest now wth the current code/setup/version
💯 1
oh, nice point about the
list
Q - is it clever enough that if I change core, (assume that every other lib in the repo depends on core) - it will say "oh, I need to test all of those as well"?)
h

hundreds-father-404

09/21/2020, 6:07 PM
Yes, iff you use the
--changed-dependees=direct
or
--changed-dependees=transitive
option though The default is
--changed-dependees=none
, which is only the changed files themselves
b

billowy-motherboard-58443

09/21/2020, 6:09 PM
hmm
./pants  --changed-since=master --changed-dependees=transitive list
and it listed I think, the whole repo tree
(after I made one trivial change to a single file in gh/util/date.py)
yeah, something wrong there, if I do
test
it ist testing things which are not on the graph with that file
h

hundreds-father-404

09/21/2020, 6:14 PM
Do you still have explicit dependencies anywhere other than for the JSON files and Python distribution target? Those dependencies might be much coarser than you want
You may find the dependees goal useful to see what is depending on that file
Use
./pants dependees path/to/f.py
to see everything that is directly depending on it. Does that look right? Then, you can use
dependees --transitive
as the next step
b

billowy-motherboard-58443

09/21/2020, 6:18 PM
ugh, yes,
possibly a bad choice of mine to play with such a used file
🙃 1
why would
dependees
be showing me
Copy code
18:26:11.56 [WARN] <unknown>:587: DeprecationWarning: invalid escape sequence \{
w

witty-crayon-22786

09/21/2020, 6:28 PM
i don’t think that that is related to
dependees
, per-se… but i don’t think we’ve triaged it yet.
in particular, whether that escape sequence is in pants’ code, or end user code. i’ll take a quick look.
b

billowy-motherboard-58443

09/21/2020, 6:32 PM
if I run
pytest
over the same code, those warnings are eemitted, but with the filename rather than
<unknown>
w

witty-crayon-22786

09/21/2020, 6:32 PM
ah, good to know… is the reported filename in your code, or in pants?
h

hundreds-father-404

09/21/2020, 6:33 PM
That’s the dependency inference AST parsing that doesn’t capture depreciation warnings. What I was mentioning much higher up in the thread. We need to fix that @witty-crayon-22786 originally we did catch it, but iirc you were concerned it wasn’t thread safe Alternatively, we could fix Pants to say what the file name is
w

witty-crayon-22786

09/21/2020, 6:34 PM
using a contextmanager to catch it is not threadsafe, correct… there might be other methods.
but yea, i remember now: thank you.
👍 1
b

billowy-motherboard-58443

09/21/2020, 6:34 PM
the files whose code produces the warnings is my code, not yours 🙂
👍 1
h

hundreds-father-404

09/21/2020, 6:36 PM
We definitely need to fix. It’s annoying because there’s no remedy atm; you can’t see the issue file My vote personally would be to catch the deprecation. Pants’s dependency inference imo shouldn’t have the side effect of logging warnings. But if we can’t do that, fixing to say what the file name is would be good
w

witty-crayon-22786

09/21/2020, 6:36 PM
so then in this case, perhaps we can tell the AST module which file we’re evaluating so that the warning is raised “from somewhere”
yea. the thread safety issue just gets a big shrug apparently: https://docs.python.org/3/library/warnings.html#warnings.catch_warnings
😕 1
h

hundreds-father-404

09/21/2020, 6:37 PM
Exactly. That’s what im thinking. We’ll still pass it a string rather than file, but give a hint for what the file name is
w

witty-crayon-22786

09/21/2020, 6:38 PM
i’ll file.
h

hundreds-father-404

09/21/2020, 6:39 PM
Oh, should we just globally ignore this warning no matter what? Set it up in our initial pants runner file. I don’t think Pants should ever warn about this in any context. For our own code, Pytest will use its own options filtering and will still print it. Ditto when users run tests on their own code. Sub processes can still warn; this is only about the main Pants process
w

witty-crayon-22786

09/21/2020, 6:40 PM
presumably it’s not the only warning that is possible in the context of AST parsing
h

hundreds-father-404

09/21/2020, 6:42 PM
True. Like deprecation warnings when you are using collections instead of collections.abc But still, I don’t think we want to warn there either. The subprocess should, but not Pants itself
b

billowy-motherboard-58443

09/21/2020, 6:42 PM
easy way to repro : put
varname = "foo\bar"
in your code somewhere
💯 1
like a regex pattern NOT preceded by
r
w

witty-crayon-22786

09/21/2020, 6:56 PM
h

hundreds-father-404

09/25/2020, 1:13 AM
This was pretty trivial to fix: https://github.com/pantsbuild/pants/issues/10826 Thanks Michael for reporting this
b

billowy-motherboard-58443

09/25/2020, 3:24 PM
👍
4 Views