Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix long paths #388

Closed
wants to merge 2 commits into from
Closed

Conversation

PeterStaev
Copy link

fixes #369.
Configures each plugin as project and adds it as dependency to the main project (no need to be prebuild in AAR!!!). From what I see this correctly merges resources/manifests, but since i'm not android expert will require some additional testing and verification.

Breaking changes
1. Plugins' manifest file must have the package attribute
2. If the plugin includes some .jar files it would be recommended to be put those side platforms\android\libs directory so that they will be packed up with the project.

Bonus
If plugin has native java code put in the correct java subfolder it will automatically be built. This brings possibility of plugins defining their own native code and activities!

@ns-bot
Copy link

ns-bot commented Mar 16, 2016

Can one of the admins verify this patch?

@Plamen5kov
Copy link
Contributor

Hi @PeterStaev,
First of all let me thank you for the PR and for the effort that you have put in.
It's great you're using the plugin-project-template/build.gradle script to create .aar file. It would be great to integrate this script into a tool that helps plugin developers get used to using and building .aar libraries and will provide full control over the plugin.
If you provide the plugin developer with an alternate way to build the .aar file, besides Android Studio, this will help for a faster build time, because the plugins won't have to be assembled during build time.

@PeterStaev
Copy link
Author

@Plamen5kov are you really serious about this?!?

For the love of god we do not want a tool that builds .aar files. If i want to build an .aar file i would be using Android Studio. Making the build 1 sec slower by compiling the project(s) every time will save developers tons of time using Android studio (or that "tool") to build .aar files. Do you guys really do not understand? We are talking cross platform framework here. If i want to build .aar files or .a files for iOS i would just scrap {N} and use Android Studio/Xcode and will make my app fully native w/o overhead of JS parsing/transpiling.

This just confirms my initial thought that you guys put everything in stone and do not want to hear what other people say about how BAD is to use .aar files. I've already lost enough time on this so I'm leaving this entirely in your hands.

@PeterStaev
Copy link
Author

And I will rephrase your words: I don't think we should add an additional tool as it will be error prone ;)

@Plamen5kov
Copy link
Contributor

Hi @PeterStaev
I looked in the PR and there are a couple of issues:

  • If you add each plugin as a project solution if you add more plugins this will cause significant performance hit +4 seconds for each plugin.
  • on this line if the AndroidManifest.xml file exists and it doesn't have package name there is still a problem. Further more we try not to change the content of the users.
  • you're hardcoding versions of the plugin when you're building them even if you don't know if they are ment to be built or to work with those compile api levels.

@PeterStaev
Copy link
Author

Hi @Plamen5kov ,

If you add each plugin as a project solution if you add more plugins this will cause significant performance hit +4 seconds for each plugin.

I did couple of tests and for 3 plugins the average added time compared to pre-built AARs is ~5.4 secs so this makes ~1.8sec per plugin:
image

on this line if the AndroidManifest.xml file exists and it doesn't have package name there is still a problem. Further more we try not to change the content of the users.

You are not looking at the final changes. I already addressed this with an additional commit

you're hardcoding versions of the plugin when you're building them even if you don't know if they are ment to be built or to work with those compile api levels.

My fix was primarily made to fix the manifest AAR thing, so if i have only a manifest matching the android version to the one of the main project makes sense. Of course using this PR to automatically build .java files will require additional changes if the plugin is to use a different version.

@Plamen5kov
Copy link
Contributor

Hi @PeterStaev,
makes ~1.8sec per plugin
This does not include the time gradle needs to configure the project tree. It's done when gradle project is being configured and it's not included in the build time number.

You are not looking at the final changes. I already addressed this with an additional commit
We are trying not to change the user provided files: 25d572c#diff-2d0cd1a5dcb57f42ae56e5172fa341cbR240

My fix was primarily made to fix the manifest AAR thing, so if i have only a manifest matching the android version to the one of the main project makes sense. Of course using this PR to automatically build .java files will require additional changes if the plugin is to use a different version.
We can't merge a partial solution.

@PeterStaev
Copy link
Author

We are trying not to change the user provided files: 25d572c#diff-2d0cd1a5dcb57f42ae56e5172fa341cbR240

This does NOT change the file in the node_modules so this does not change the actual file. And if you want you can remove it but then the build will fail and plugin authors need to update their manifests.

We can't merge a partial solution.

What is your criteria for this being a partial solution??? The current design of plugins DOES NOT allow to have java files in side the plugins. So it does not matter what version does the plugin compile to.

But whatever, I've already lost enough time implementing this as a fix and explaining the problems that your current intended solution creates. But seems everyone is blind on this matter. So i do not intend losing more time on this hopeless thing.

@Plamen5kov Plamen5kov closed this Mar 24, 2016
@NathanaelA

This comment was marked as abuse.

@Plamen5kov
Copy link
Contributor

Hi @NathanaelA,
... can you think of any specific example where changing the name would cause a issue?
There are problems, when gradle tries to cache the plugin and then someone tries to use the cached version with a different name than the one cached, which can be caused when people upgrade from one version to another. We already had to fix and support a lot of these problems in the previous releases and we strive to avoid more of those scenarios.
The experience we have so far points us in the right direction and we try not to "swim upstream". Last time we had unexpected issues with name length on windows, and we don't want to have similar issues that would slow down {N} users.

@PeterStaev
Copy link
Author

@NathanaelA i'm not discouraged. The problem is specifically with this they try to make whatever possible to continue with the .aar solution and try to prove wrong/incompatible every other solution. First they say that there will be a problem if the Manifest does not have a package name. Well you know what - there will be also a problem if the plugin has a manifest files and does not have an .aar file with your solution. How do you solve that one?
Then when pointed to a fix i applied for this specific case, they come up with another "reason" "we try not to change user files". Yet again they create dummy include.gradle files for plugins that do not have this. How is this corresponding to "not changing user files"?

@PeterStaev PeterStaev deleted the long-path-fix branch March 24, 2016 14:59
@NathanaelA

This comment was marked as abuse.

@NativeScript NativeScript locked and limited conversation to collaborators Mar 24, 2016
@atanasovg
Copy link
Contributor

Locking the conversation to prevent non-constructive opinions.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CLI can easily fail and blow project up on windows when you have multiple plugins.
5 participants