File name too long when nesting subcomponents #421
Comments
|
Maybe Dagger should chance its strategy for naming the impls, and add a numbering scheme for duplicates? Not sure what I can do in the meantime. |
|
A few things here…
A quick strawman for something that might help would be to use initialisms to try to shorten some of these names. Here's an example if we just did it for any enclosing classes: |
|
Instead of changing the filename generation scheme wholesale, how about providing a class annotation so that folks who encounter this issue could override the Dagger-generated name part? For example, for class DevRegisterAppComponent --> @DaggerName(name = "DDevRegAppCmp"). This would still keep easy-to-read filenames for everyone, but allow teams with complex hierarchies to mitigate the long filename problem locally. Or even better, add "name" parameter to |
|
Or just turn on a "deep component hierarchy" flag and switch to a flat class naming scheme. |
|
It would be pretty lame if you had to know that you needed to pass that flag just in order to have your code compile. @gk5885 what if all subcomponent types were nested directly within the generated component? They'd all still be inaccessible from without the codegen, and we could pass references directly instead of implicitly. Then as long as |
|
Now, please make fun of me, this is my temporary fix for our internal fork: --- a/compiler/src/main/java/dagger/internal/codegen/ComponentWriter.java
+++ b/compiler/src/main/java/dagger/internal/codegen/ComponentWriter.java
@@ -44,12 +44,15 @@ import javax.lang.model.element.Name;
import javax.lang.model.element.TypeElement;
import javax.lang.model.util.Elements;
import javax.lang.model.util.Types;
+import java.util.concurrent.atomic.AtomicInteger;
/**
* Creates the implementation class for a component.
*/
final class ComponentWriter extends AbstractComponentWriter {
+ private static final AtomicInteger count = new AtomicInteger(0);
+
ComponentWriter(
Types types,
Elements elements,
@@ -108,23 +111,10 @@ final class ComponentWriter extends AbstractComponentWriter {
private ImmutableBiMap<ComponentDescriptor, String> disambiguateConflictingSimpleNames(
Collection<ComponentDescriptor> components) {
+ // See https://github.com/google/dagger/issues/421
Map<String, ComponentDescriptor> generatedSimpleNames = new LinkedHashMap<>();
- // The ending condition is when there is a unique simple name generated for every element
- // in components. The sizes should be equivalent (with one generated name per component).
- for (int levels = 0; generatedSimpleNames.size() != components.size(); levels++) {
- generatedSimpleNames.clear();
- for (ComponentDescriptor component : components) {
- List<String> pieces = componentQualifiedNamePieces.get(component);
- String simpleName =
- QUALIFIED_NAME_JOINER.join(
- pieces.subList(Math.max(0, pieces.size() - levels - 1), pieces.size()));
- ComponentDescriptor conflict = generatedSimpleNames.put(simpleName, component);
- if (conflict != null) {
- // if the map previously contained an entry for the same simple name, stop early since
- // 2+ subcomponent descriptors will have the same simple name
- break;
- }
- }
+ for (ComponentDescriptor component : components) {
+ generatedSimpleNames.put("C" + count.incrementAndGet(), component);
}
return ImmutableBiMap.copyOf(generatedSimpleNames).inverse();
} |
|
Which generates things like |
|
FYI, the processor is single threaded so the atomic integer isn't necessary (unless you plan on maintaining this fork for a while ;) |
|
@valeriyo, I'd rather not add an API for tweaking something that is entirely an implementation detail. @loganj, if by "flag naming scheme" you mean using top-level classes rather than nested classes, that would be an option, but means that we'd have to generate components very differently. We would either have to make a lot of fields accessible or pass around a bunch of state (probably more than the max number of cxtor args). @ronshapiro, that's a variant of flattening the component hierarchy that might dodge some accessibility issues, but does complicate the generated code because you can no longer use the implicit OK, time to make fun of @py. Not really, but that is exactly the type of thing we don't want to show up in a stack trace. We might as well call them @ronshapiro, I'd stick with So, here's what I'm thinking we can do…
Sound like a reasonable enough plan? |
|
Yes! You might still want to use a variant of my strategy for the degenerate case where shortening still creates conflicts? |
|
For the record:
The actual fix will be deeply appreciated :) . |
|
Why are you making fun of me? )c: On Wednesday, August 3, 2016, Gregory Kick [email protected] wrote:
|
|
@pyricau, I'm actually at a conference this week and not going to be able to look at this for quite a while. Do you have any interest in taking this on? Otherwise, it might be a while before we get a fix. |
|
Just saw this. I don't have any bandwidth at the moment. I'll pass the word around, maybe someone in my team will get pissed and just do it. |
Fix for google#421. Instead of including more and more levels of container names until we achieve uniqueness, we try two strategies: use simple names everywhere, or else include a uniquing prefix everywhere. The prefix is built from the initials of a containing class for nested subcomponent declarations, or the initials of containing packages for top level. An integer may appended to the prefix if necessary.
Fix for google#421. Instead of including more and more levels of container names until we achieve uniqueness, we try two strategies: use simple names everywhere, or else include a uniquing prefix everywhere. The prefix is built from the initials of a containing class for nested subcomponent declarations, or the initials of containing packages for top level. An integer may appended to the prefix if necessary.
Fix for google#421. Instead of including more and more levels of container names until we achieve uniqueness, we try two strategies: use simple names everywhere, or else include a uniquing prefix everywhere. The prefix is built from the initials of a containing class for nested subcomponent declarations, or the initials of containing packages for top level. An integer may appended to the prefix if necessary.
Fix for google#421. Instead of including more and more levels of container names until we achieve uniqueness, we try two strategies: use simple names everywhere, or else include a uniquing prefix everywhere. The prefix is built from the initials of a containing class for nested subcomponent declarations, or the initials of containing packages for top level. An integer may appended to the prefix if necessary.
Instead of including more and more levels of container names until we achieve uniqueness, we try two strategies: use simple names everywhere, or else include a uniquing prefix everywhere. The prefix is built from the initials of a containing class for nested subcomponent declarations, or the initials of containing packages for top level. An integer may appended to the prefix if necessary. Fixes #421 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=154456167
Instead of including more and more levels of container names until we achieve uniqueness, we try two strategies: use simple names everywhere, or else include a uniquing prefix everywhere. The prefix is built from the initials of a containing class for nested subcomponent declarations, or the initials of containing packages for top level. An integer may appended to the prefix if necessary. Fixes #421 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=154456167
Instead of including more and more levels of container names until we achieve uniqueness, we try two strategies: use simple names everywhere, or else include a uniquing prefix everywhere. The prefix is built from the initials of a containing class for nested subcomponent declarations, or the initials of containing packages for top level. An integer may appended to the prefix if necessary. Fixes #421 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=154456167
|
This was not mentioned properly in the Github release notes - https://github.com/google/dagger/releases/tag/dagger-2.11-rc2 |
|
@pyricau |
|
We added a more robust fix in Dagger 2.42 (basically we no longer nest subcomponent implementations within their parent to avoid this situation). Please try upgrading to at least 2.42. |
|
Hey @bcorso I tired the same but in my project while using dagger 2.13 we have made changes were a module can have a scope(which was later fixed in 2.15 Scopes are no longer allowed on |
I don't really understand this part. You just need to remove the scope annotation from your module (which is currently doing nothing), so it shouldn't change anything about your bindings. This should be pretty simple fix even if you have 250 modules.
You could try to manually shorten the name, but IMO that's a worse solution and even more of a pain than fixing the scopes on modules.
No idea. Maybe something with AGP since it's happening in the transform asm task. |
|
Hey @bcorso I faced one more issue when migrating to 2.51, where in my implementation while providing the dependency I was using the internal modifier, like this which also cause the file name too long issue. And when I removed the internal modifier it was working fine, what could be the reason? |
When using the internal modifier the name of the function gets mangled which causes it to be longer. |
and others
I'm going through the final steps to fully convert our app to Dagger 2, tying the hierarchy of subcomponents together. We have a deep hierarchy of sub components. This means the subcomponent names are too long:
The text was updated successfully, but these errors were encountered: