pantsbuild #development

Channels

welcome

github-notifications

development

rough-minister-58256

07/12/2016, 1:24 AM

tho you could split that into two rules - one that only consumed PathGlobs and deduped

billowy-beach-68498

07/12/2016, 1:27 AM

are you saying if dup paths belong to 2 distinct paths_list elements, then we can dedup them?

rough-minister-58256

07/12/2016, 1:29 AM

hm, actually that probably won’t work

billowy-beach-68498

07/12/2016, 1:30 AM

what was the case that Stu described for path literals where duplication is necessary?

rough-minister-58256

07/12/2016, 1:31 AM

unsure exactly

billowy-beach-68498

07/12/2016, 1:31 AM

ok. I didn’t follow that part either

rough-minister-58256

07/12/2016, 1:33 AM

based on a

git blame

of the test he linked above, guessing it relates to symlinks: https://github.com/pantsbuild/pants/commit/387bb116c96863aaf2e07fff278a363e15bbfd34

rough-minister-58256

07/12/2016, 1:34 AM

ah, perhaps https://github.com/pantsbuild/pants/issues/3497

billowy-beach-68498

07/12/2016, 1:57 AM

so what is necessary here is to preserve the order, which does not conflict with dedup?

fancy-queen-20734

07/12/2016, 5:50 PM

Ah, I see. So if sources are

=[‘a’, ‘a’]

, we want to preserve that

fancy-queen-20734

07/12/2016, 5:50 PM

the case I was seeing is that if you have

rglobs=[‘a/b/*’]

, you’ll get the contents of

a/b

twice

fancy-queen-20734

07/12/2016, 5:54 PM

I don’t think a user would expect that a single rglob to return duplicates of the matched files

fancy-queen-20734

07/12/2016, 5:55 PM

I think that also implies each glob will add duplicates again, so if you had sources defined with overlapping globs, they would have multiple copies of each. I’m not sure about it though

rough-minister-58256

07/12/2016, 6:12 PM

I’d be curious if this behavior /started/ with your commit @billowy-beach-68498 - if so, might this just be a matter of a bug in the way we emit the various `PathWildcard`/`PathDirWildcard` products etc?

rough-minister-58256

07/12/2016, 6:12 PM

seems like based on Nick’s timeline, he likely had his tests working before - then rebased - then this behavior was introduced.

billowy-beach-68498

07/12/2016, 6:13 PM

yeah, that is very likely. I will do more testing on this today to find out.

witty-crayon-22786

07/12/2016, 6:19 PM

"what was the case that Stu described for path literals where duplication is necessary?" - i linked it above when i mentioned you both

witty-crayon-22786

07/12/2016, 6:20 PM

...but yea, github issue is a bit better

witty-crayon-22786

07/12/2016, 6:21 PM

@billowy-beach-68498: the code that generates

rglobs

from WrappedGlobs duplicates them

witty-crayon-22786

07/12/2016, 6:21 PM

and i think that since your change, that is not necessary.

witty-crayon-22786

07/12/2016, 6:22 PM

in particular, this comment was never true of git: https://github.com/pantsbuild/pants/blob/master/src/python/pants/source/wrapped_globs.py#L264-L270

witty-crayon-22786

07/12/2016, 6:22 PM

and now it is not true of our impl either

billowy-beach-68498

07/12/2016, 6:22 PM

ah ok. I suspect that as well. was gonna ask @fancy-queen-20734 for his particular test case

billowy-beach-68498

07/12/2016, 6:24 PM

but in test_fs.py, if we do assert_walk(Files, [‘*’, ‘**/*’], […]), the result will also have duplicate

billowy-beach-68498

07/12/2016, 6:24 PM

Copy code

assert_walk(Files, [‘*’, ‘**/*’], […])

billowy-beach-68498

07/12/2016, 6:25 PM

because in merge_paths we do not eliminate that

witty-crayon-22786

07/12/2016, 6:25 PM

@billowy-beach-68498: yes. that is primarily in there because of the case described in wrapped_globs

billowy-beach-68498

07/12/2016, 6:25 PM

that has nothing to do with rglobs logic i think

witty-crayon-22786

07/12/2016, 6:25 PM

the reason to test that case (before) was that rglobs was generating those two patterns, and so it was useful to test them together

billowy-beach-68498

07/12/2016, 6:27 PM

ok and the reason rglobs was generating those 2 patterns was previously ** matches 1 or more dirs?