Get rid of Missing dependency warnings in java/scala compile. #37
Comments
|
Submitted |
|
What was the commit that fixed this? That missing dep detection is my code, so I'd like to review the change. |
|
I think this is by mistake.. i wanted to close to some other issue. |
|
I pushed a simple branch with a java_library that directly depends on a scala_library. The java file uses a class in the scala file. This currently triggers an incorrect build dependency missing warning. For example it thinks the class is here: When it is in fact here: |
|
Looks like I forgot the link to the branch: https://github.com/pantsbuild/pants/tree/javadep |
|
I notice a very similar issue when using an annotation processor: And Garrett was noticing something on the zip file work he's doing. |
|
Ok - I think the issue is like so: In jvm_compile the key top-level flow is: # Update the products with the latest classes. Must happen before the
# missing dependencies check.
self._register_products(vts.targets, analysis_file)
if self._dep_analyzer:
# Check for missing dependencies.
actual_deps = self._analysis_parser.parse_deps_from_path(analysis_file,
lambda: self._compute_classpath_elements_by_class(cp_entries))
with self.context.new_workunit(name='find-missing-dependencies'):
self._dep_analyzer.check(sources, actual_deps)So, in-short:
Now in 1 the analysis file parser is handed 2 pieces of data - the analysis file for this compile round and a function that can produce an mapping of all known classes to the the jar or local target that provides them (there the local target is a slice of a global classfile output dir). The indexing function looks like so: def _compute_classpath_elements_by_class(self, classpath):
# Don't consider loose classes dirs in our classpath. Those will be considered
# separately, by looking at products.
def non_product(path):
return not (path.startswith(self._pants_workdir) and os.path.isdir(path))
classpath_jars = filter(non_product, classpath)
if self._class_to_jarfile is None:
self._class_to_jarfile = {}
for jarpath in self.find_all_bootstrap_jars() + classpath_jars:
# Per the classloading spec, a 'jar' in this context can also be a .zip file.
if os.path.isfile(jarpath) and ((jarpath.endswith('.jar') or jarpath.endswith('.zip'))):
with open_zip(jarpath, 'r') as jar:
for cls in jar.namelist():
# First jar with a given class wins, just like when classloading.
if cls.endswith(b'.class') and not cls in self._class_to_jarfile:
self._class_to_jarfile[cls] = jarpath
elif os.path.isdir(jarpath):
for dirpath, _, filenames in os.walk(jarpath, followlinks=True):
for f in filter(lambda x: x.endswith('.class'), filenames):
cls = os.path.relpath(os.path.join(dirpath, f), jarpath)
if not cls in self._class_to_jarfile:
self._class_to_jarfile[cls] = jarpThe comment is key and true even though the impl is confusing since the elif branch looks like it indexes loose classes - which it does, but it only does for loose classes outside .pants.d/ which AFAICT never exist. So with the comment in mind and drilling into step 1 ( def parse_deps(self, infile, classpath_indexer):
buildroot = get_buildroot()
classpath_elements_by_class = classpath_indexer()
self._expect_header(infile.readline(), 'pcd entries')
num_pcd_entries = self._parse_num_items(infile.readline())
for _ in xrange(0, num_pcd_entries):
infile.readline() # Skip these lines.
src_to_deps = self._parse_deps_at_position(infile)
ret = defaultdict(set)
for src, deps in src_to_deps.items():
for dep in deps:
rel_classfile = dep + '.class'
classpath_element = classpath_elements_by_class.get(rel_classfile, None)
if classpath_element: # Dep is on an external jar/classes dir.
ret[src].add(classpath_element)
else: # Dep is on an internal class.
classfile = os.path.join(buildroot, self.classes_dir, rel_classfile)
ret[src].add(classfile)
return retUp top the index My prediction then is that any cross-jvm-compiler internal dep will fail dep checks erroneously. |
|
My prediction is wrong but thats because the parse_deps impl in the ZincAnalysisParser is different of course as is the zinc analysis file. The zinc analysis file comes with absolute paths everywhere making things easy on pants; ie: The JMakeAnalysisParser has to work with this: Thus the need to fill in the |
|
Posted a RB here: https://rbcommons.com/s/twitter/r/474/ |
Format implicits are no longer needed.
./pants goal compile on a scala_library which includes java_sources spits this warning.
The below mentioned dependencies are actually provided by java_sources.
The text was updated successfully, but these errors were encountered: