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

flat-zoo-31952

11/18/2020, 11:09 PM
i think i've found a bug, or it may be expected behavior, but i'd like yalls opinion before i file it on gh: whenever i add a dep to
requirements.txt
, that causes nearly all targets in my repo to be counted as an affected transitive dependency in
./pants --changed-since= --changed-dependees=transitive *
commands. I suspect the target list is all targets that are using dependency inference, but i haven't confirmed this yet. Is this behavior expected? It seems like all dependnecy inference targets are depending on
requirements.txt
as a whole, which is bad because it means any change in requirements will trigger tests and rebuilds of everything in the repo
btw still using 2.0.0, plan to upgrade soon
h

hundreds-father-404

11/18/2020, 11:13 PM
Ah, yeah that’s not great. Expected, but not desirable The issue is that the python_requirements macro adds a dependency on the concrete file. So when you change that file, every 3rd party req ends up being transitively changed I think we’ve had an issue open for a long time for how to have macros better handle reading from files and how to cache it correctly. That python_requirements() macro is one of the only things still left over using v1 APIs
f

flat-zoo-31952

11/18/2020, 11:14 PM
is the workaround to add the requirements to my BUILD file? i can script that lol, i'm fine with it, what is it,
python_requirement_library()
?
this is the greater issue tracking it: https://github.com/pantsbuild/pants/issues/7022 Iiuc, you’ll be introducing a new issue where changing requirements.txt will not invalidate anything until you restart pantsd
f

flat-zoo-31952

11/18/2020, 11:29 PM
i feel like just updating my root BUILD file is safer, is there something i'm missing there?
h

hundreds-father-404

11/18/2020, 11:31 PM
Like, adding a
python_requirement_library
to your BUILD file for everything? I think you will still hit a similar issue - when you change a BUILD file, every target defined in that BUILD file is invalidated because we can’t detect which targets you changed (we don’t persist to disk what the target’s metadata was before)
So, until we solve #7022, you have to pick your poison of overly sensitive
—changed-since
, or of updating not changing things until you restart pantsd I’m not sure how hard 7022 is. Stu has more context, and it was an idea for Q4 prioritization
f

flat-zoo-31952

11/18/2020, 11:38 PM
hmm okay i guess i'll take having to restart pantsd then
w

witty-crayon-22786

11/18/2020, 11:39 PM
moving the declarations to BUILD files would avoid needing to restart pantsd.
f

flat-zoo-31952

11/18/2020, 11:40 PM
i'll play around with it
w

witty-crayon-22786

11/18/2020, 11:42 PM
but as Eric said, you’d need to move each declaration into its own BUILD file to avoid the issue with `--changed`… that one goes a bit deeper even than the macro support. i think that i know roughly how to solve it (executing
--changed
logic against our runtime graph rather than the build graph), but have been waiting for the plugin API to settle down a little bit before tackling it.
🙏🏻 1
👍 1
h

hundreds-father-404

11/18/2020, 11:44 PM
That’d be awesome!
—changed
is one of the most used features in pants, so making it more precise is highly leveraged
1
f

flat-zoo-31952

11/19/2020, 12:23 AM
yeah i honestly don't care about macro support, those are just details to me;
--changed-*
working right is everything that matters to me. It's the difference between a working and non-working monorepo solution
👍 1
would putting each requirement its own build file break dependency inference for these?
w

witty-crayon-22786

11/19/2020, 12:26 AM
no, that would work to solve both issues.
h

hundreds-father-404

11/19/2020, 12:27 AM
Nope, it’d still work fine! Only downside is that now you have a bunch of BUILD files and it might be annoying to maintain and possibly confusing whenever you need to add an explicit dependency But you could use a naming scheme like • 3rdparty/python/django • 3rdparty/python/ansicolors Because there’s only one target in each directory, you can leave off the target name and use the default of the directory. So to add an explicit dep, it’s simply the path to the folder, no colon needed
🦜 1
f

flat-zoo-31952

11/19/2020, 12:46 AM
agree it's a bit ugly, but i like ugly solutions that fit my use cases better than pretty ones that don't. Thanks for the tip. I can add my feedback to the linked GH issue if it helps, or should I add another that focuses on the
--change
invalidation granularity?
💯 1
nvm saw stu's post in #engine about https://github.com/pantsbuild/pants/issues/11206
w

witty-crayon-22786

11/19/2020, 12:49 AM
yea, it’s likely medium/long term enough that i didn’t want to derail this thread with it.
5 Views