Skip to content

Conversation

petekanev
Copy link
Contributor

Addresses issue #832 by making the following small changes:

  • when collecting classes from input jars, do not crash on clashing classes (classes that are already brought in from another library). The assumption here is that the build will crash at earlier point during the java compilation step, and it should not concern the metadata generator.
    (e.g. An android library depends on appache http library, but if imported in gradle as a dependency, the class collection logic in the metadata generation would fail, complaining that 3 classes coincide in name - some of the appache http classes are left inside the android sdk packages for whatever legacy reasons they have)

  • when traversing a class's methods/fields, do not skip class if the parameter of a single method is not found in the ClassLoader. Instead continue, and try to generate as much metadata as possible for the class being analyzed.

@petekanev petekanev requested a review from Plamen5kov September 5, 2017 11:34
@ns-bot
Copy link

ns-bot commented Sep 5, 2017

💚

@petekanev
Copy link
Contributor Author

run uitests

@dtopuzov
Copy link
Contributor

dtopuzov commented Sep 7, 2017

run ci

@ns-bot
Copy link

ns-bot commented Sep 7, 2017

💔

ClassDescriptor clazz = ClassRepo.findClass(className);
generate(clazz, root);
if (clazz == null) {
throw new ClassNotFoundException("Class " + className + " not found in the input jars.");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change jars to libraries, since from the user perspective .aar files are not jars.

Copy link
Contributor

@Plamen5kov Plamen5kov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After green build + unit tests
and minor changes from the comments are met

// Should we worry about this? If classes would crash, that would happen during the compilation step
// which occurs before the metadata generation

// throw new IllegalArgumentException(errMsg);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We needed this check when metadata was generated on the basis of .dex files. It's now obsolete because, as you said, the compilation task will take care of providing an understandable error for the user, so we don't need to.
You can remove the comment and leave the code as it is, without the throw statement.

@petekanev petekanev force-pushed the pete/improve-metadata branch from 6792a21 to 7438f48 Compare September 11, 2017 08:18
@ns-bot
Copy link

ns-bot commented Sep 11, 2017

💚

@petekanev petekanev force-pushed the pete/improve-metadata branch from 7438f48 to d9e81d7 Compare September 11, 2017 10:11
@ns-bot
Copy link

ns-bot commented Sep 11, 2017

💔

@petekanev
Copy link
Contributor Author

run ci

@ns-bot
Copy link

ns-bot commented Sep 11, 2017

💚

@petekanev petekanev merged commit 6bc64ce into master Sep 12, 2017
@petekanev petekanev deleted the pete/improve-metadata branch September 12, 2017 07:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants