pantsbuild #development

Channels

welcome

github-notifications

development

aloof-angle-91616

06/23/2020, 10:38 PM

@witty-crayon-22786 do you mean the snapshotsubset PR?

aloof-angle-91616

06/23/2020, 10:38 PM

i will see if reverting that makes the hanging stop

👍 1

witty-crayon-22786

06/23/2020, 10:38 PM

ah, yea… i guess i was just assuming based on

test_snapshot_subset_globs

aloof-angle-91616

06/23/2020, 10:38 PM

oh lmao

witty-crayon-22786

06/23/2020, 10:38 PM

thanks

❤️ 1

aloof-angle-91616

06/23/2020, 10:38 PM

ok can i have like 10 minutes to repro this and see if i can fix it quickly

hundreds-father-404

06/23/2020, 10:38 PM

Sure!

aloof-angle-91616

06/23/2020, 10:39 PM

i'll put up a revert if there's no fix made in that time

hundreds-father-404

06/23/2020, 10:39 PM

Thank you! Fingers crossed the fix is easy

hundreds-father-404

06/23/2020, 10:40 PM

Stu, I think this means time outs might not be working locally? I can’t get it to timeout locally. Note that the code path is different than remote timeouts

witty-crayon-22786

06/23/2020, 10:42 PM

i’ve seen them work in the recent past…

aloof-angle-91616

06/23/2020, 10:42 PM

i would def be surprised if this isn't the snapshot subset change. will just push a revert now to get it past CI faster

❤️ 1

witty-crayon-22786

06/23/2020, 10:46 PM

thanks. and thanks again for iterating there.

hundreds-father-404

06/23/2020, 10:46 PM

Thank you Danny, good idea. And if you’re able to find a fix before the CI goes green, we can close the revert PR in favor of the fix

aloof-angle-91616

06/23/2020, 10:47 PM

i'm not too worried about unreverting the change. it explicitly moved things to a separate file so hopefully won't be hard to rebase. and i expected something like this -- will just add a test once i figure out the fix

💯 1

aloof-angle-91616

06/23/2020, 10:48 PM

just making sure the revert fixes it now

❤️ 1

aloof-angle-91616

06/23/2020, 10:50 PM

yes, it does. will see if i can easily find the issue now

aloof-angle-91616

06/23/2020, 10:50 PM

https://github.com/pantsbuild/pants/pull/10143 <= revert

aloof-angle-91616

06/23/2020, 10:56 PM

aha! running passing

-s

to pytest:

Copy code

pants/engine/fs_test.py::FSTest::test_snapshot_subset_globs thread 'tokio-runtime-worker' panicked at 'assertion failed: `(left != right)`

💯 1

aloof-angle-91616

06/23/2020, 10:56 PM

probably not good that a panic will cause a timeout here but easy enough to debug now

hundreds-father-404

06/23/2020, 11:00 PM

great idea!

aloof-angle-91616

06/23/2020, 11:02 PM

the

assert_ne!()

causing the issue was actually just an artifact from the first attempt and isn't correct

aloof-angle-91616

06/23/2020, 11:03 PM

removing it gets to another error, in the same test: we have the correct

.files

in the resulting python

Snapshot

, but not

.directories

. looking into this

aloof-angle-91616

06/23/2020, 11:30 PM

found a real fix!! https://github.com/pantsbuild/pants/pull/10146

❤️ 1

aloof-angle-91616

06/23/2020, 11:35 PM

i think just mental note for the future for myself is that if a test is timing out, always make sure to run it locally, with

-s

on, because that shows panic messages

💯 1

aloof-angle-91616

06/24/2020, 12:53 AM

revert is in https://github.com/pantsbuild/pants/pull/10143

🙏 2

aloof-angle-91616

06/24/2020, 1:15 AM

the fix/unrevert for #9779 works and i have modified the test cases in response to feedback: https://github.com/pantsbuild/pants/pull/10146. kinda funny -- to get the behavior that makes sense (globs without

**

matching directories, but not their files), you have to replace the matching directory node with one that has an empty digest.

aloof-angle-91616

06/24/2020, 4:53 AM

revert revert is in! https://github.com/pantsbuild/pants/pull/10146

❤️ 1

aloof-angle-91616

06/24/2020, 4:54 AM

now we can make

MergeDigests()

faster too: https://github.com/pantsbuild/pants/pull/9982

❤️ 1

aloof-angle-91616

06/24/2020, 4:56 AM

also we can merge with colliding file paths finally

🎉 1