#general
- b
bored-art-40741
09/03/2015, 1:48 AMIn particular - b
bored-art-40741
09/03/2015, 1:48 AMCopy code- # Compute jar -> target. - self.context.log.debug('Mapping jars...') - compile_classpath = self.context.products.get_data('compile_classpath') - for jar_lib in self.context.targets(lambda t: isinstance(t, JarLibrary)): - for _, artifact_path in compile_classpath.get_for_target(jar_lib, transitive=False): - targets_by_file[artifact_path].add(jar_lib) - b
bored-art-40741
09/03/2015, 1:48 AMbecame - b
bored-art-40741
09/03/2015, 1:48 AMCopy code+ # Compute jar -> target. + self.context.log.debug('Mapping jars...') + compile_classpath = self.context.products.get_data('compile_classpath') + for jar_lib in self.context.targets(lambda t: isinstance(t, JarLibrary)): + for _, artifact_path in compile_classpath.get_for_target(jar_lib, transitive=False): + if artifact_path.endswith('.jar'): + for cls in self._jar_classfiles(artifact_path): + targets_by_file[cls].add(jar_lib) - b
bored-art-40741
09/03/2015, 1:48 AMAlmost invisibly making a change given the diff - b
bored-art-40741
09/03/2015, 1:49 AMIt's unclear to me why the latter would be correct given how zinc and resolvers would respectively report used deps and provided deps - b
bored-art-40741
09/03/2015, 1:49 AMBut either way that was a super difficult change to track down - w
witty-crayon-22786
09/03/2015, 1:50 AMthat should probably add both the jar and the classfiles to targets_by_file ? - b
bored-art-40741
09/03/2015, 1:51 AMI think so, yeah - a
adventurous-twilight-44110
09/03/2015, 1:51 AMsorry about that — want me to get a rb out? - w
witty-crayon-22786
09/03/2015, 1:51 AM@adventurous-twilight-44110: a test to catch that would be good... - b
bored-art-40741
09/03/2015, 1:51 AMSeems like it's a bug. I'm more concerned that it wasn't caught by a -- - b
bored-art-40741
09/03/2015, 1:52 AMI owe Stu a coke - w
witty-crayon-22786
09/03/2015, 1:52 AMwell, to be fair: there weren't any tests for this code before =( - b
bored-art-40741
09/03/2015, 1:52 AMAnd also let's please avoid large file moves in the same patch as semantic changes - b
bored-art-40741
09/03/2015, 1:52 AMThese types of breaks are ultimately unavoidable, but we can avoid making the diffs hard to track down - w
witty-crayon-22786
09/03/2015, 1:53 AM@bored-art-40741: is this time sensitive, or is tomorrow ok? - b
bored-art-40741
09/03/2015, 1:53 AMNot time sensitive at all. I'm patching some stuff in related code - w
witty-crayon-22786
09/03/2015, 1:53 AMkk - b
bored-art-40741
09/03/2015, 1:54 AMJust wanted to make sure I'd actually found the bug and let everyone know - a
adventurous-twilight-44110
09/03/2015, 1:54 AMcool. tomorrow morning I’ll fix it + create some tests - b
bored-art-40741
09/03/2015, 1:57 AMThanks - b
bored-art-40741
09/03/2015, 2:07 AMPS I think our isort CI checker has stopped working - b
bored-art-40741
09/03/2015, 2:08 AMI've noticed a couple times now that CI doesn't fail when my imports are unsorted - c
cuddly-accountant-94274
09/03/2015, 2:38 AMIs it known that the "stream" used when downloading through BinaryUtil.select_binary() is not really a stream, and we load the file into memory before writing to the local file? - c
cuddly-accountant-94274
09/03/2015, 2:40 AMOr at least, given the implementation of go - c
cuddly-accountant-94274
09/03/2015, 2:40 AM(Investigating why go.tar.gz bootstrap is failing and I realized this was the case.) - b
bored-art-40741
09/03/2015, 2:41 AMbootstrapped_binary.write(stream()) - b
bored-art-40741
09/03/2015, 2:41 AMYeah, there's no attempt there to "stream" - b
bored-art-40741
09/03/2015, 2:42 AMIt's possible though generally I think it's not common to bother unless you're expecting the file to be very large or the read to be long lived