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

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

Closed
teobugslayer opened this Issue Feb 26, 2016 · 70 comments

Comments

Projects
None yet
@teobugslayer

teobugslayer commented Feb 26, 2016

From @NathanaelA on February 25, 2016 23:55

Windows only supports 255 character length paths... This is VERY IMPORTANT to this issue!

For some reason the CLI is creating a really long path for merging stuff using the names of each installed plugin. So for example when using 8 plugins if each of them are an average of 22 characters (nativescript = 12 by itself, almost every plugin is prefixed with those 12 characters). = 176 characters.

So If I have D:\projects\nativescript\projname = 35
Then you add in \platforms\android\build\intermediates\res\merged = 59
(176 in Plugin name characters) = 172
Then you add the image part of the path \debug\drawable-xxhdpi\someimage.png = 49

= 315 Characters. Guess what happens. No Worky 😀 and now your platform folder is totally screwed up.

THIS IS A BAD ISSUE! It leaves the project effectively dead.
Plugins I was using:
"nativescript-barcodescanner": "^1.2.0",
"nativescript-cardview": "^0.1.1",
"nativescript-dom": "^1.0.0",
"nativescript-drop-down": "^1.2.1",
"nativescript-grid-view": "^1.3.0",
"nativescript-liveedit": "file:..\repos\nativescript-liveedit",
"nativescript-sqlite": "file:..\repos\nativescript-sqlite",
"nativescript-telerik-ui": "file:..\nativescript-ui-debug-dev.tgz",
"tns-core-modules": "^1.6.2"

Then I attempted to add: nativescript-floatingactionbutton

This is a BLOCKING issue for most people when they hit this. I have some ways I think I can work around this.

Copied from original issue: NativeScript/nativescript-cli#1546

@teobugslayer

This comment has been minimized.

Show comment
Hide comment
@teobugslayer

teobugslayer Feb 26, 2016

From @NathanaelA on February 26, 2016 2:28

Ok; for anyone who runs into this -- this is a gradle issue, not a TNS specific issue.
However, the issue is still the same. Gradle / aapt apparently has a couple bugs already in for this:
https://code.google.com/p/android/issues/detail?id=187430
https://code.google.com/p/android/issues/detail?id=160994

Who knows when they will be fixed; but their is a way to mitigate the primary issue I ran into. The include.gradle files internal names can be shortened.

Inside the include.gradle files are a section that (example nativescript-cardview) looks like this

android {
    productFlavors {
        "nativescriptcardview" {
            dimension "nativescriptcardview"
        }
    }
}

Edit and make it look like this:

android {
    productFlavors {
        "nscv" {
            dimension "nscv"
        }
    }
}

Now the plugin only uses 4 characters rather than 20 characters. By doing this to all the existing include.gradle files the size was reduced to considerably under the 255 length.

A couple notes, this could be done via the CLI automatically; We would make the CLI when it copies over the include.gradle files automatically shorten the name... When it generates a new default include.gradle it uses a shortened name.

teobugslayer commented Feb 26, 2016

From @NathanaelA on February 26, 2016 2:28

Ok; for anyone who runs into this -- this is a gradle issue, not a TNS specific issue.
However, the issue is still the same. Gradle / aapt apparently has a couple bugs already in for this:
https://code.google.com/p/android/issues/detail?id=187430
https://code.google.com/p/android/issues/detail?id=160994

Who knows when they will be fixed; but their is a way to mitigate the primary issue I ran into. The include.gradle files internal names can be shortened.

Inside the include.gradle files are a section that (example nativescript-cardview) looks like this

android {
    productFlavors {
        "nativescriptcardview" {
            dimension "nativescriptcardview"
        }
    }
}

Edit and make it look like this:

android {
    productFlavors {
        "nscv" {
            dimension "nscv"
        }
    }
}

Now the plugin only uses 4 characters rather than 20 characters. By doing this to all the existing include.gradle files the size was reduced to considerably under the 255 length.

A couple notes, this could be done via the CLI automatically; We would make the CLI when it copies over the include.gradle files automatically shorten the name... When it generates a new default include.gradle it uses a shortened name.

@teobugslayer

This comment has been minimized.

Show comment
Hide comment
@teobugslayer

teobugslayer Feb 26, 2016

From @Daxito on February 26, 2016 3:30

Is there a way to know when we run into this issue? Any error message or something?

teobugslayer commented Feb 26, 2016

From @Daxito on February 26, 2016 3:30

Is there a way to know when we run into this issue? Any error message or something?

@teobugslayer

This comment has been minimized.

Show comment
Hide comment
@teobugslayer

teobugslayer Feb 26, 2016

From @NathanaelA on February 26, 2016 4:8

In my case it threw lots of error messages about aapt not being able to process some of the png files. At the end it just says gradlew failed.

Here is one of the many errors sets (each error has these 5 lines)

AAPT out(1144172198) : No Delegate set : lost message:Crunching D:\projects\nativescript\test1\platforms\android\build\intermediates\exploded-aar\barcodescanner-release\res\drawable\flip_camera.png
AAPT out(1144172198) : No Delegate set : lost message:Crunching single PNG file: D:\projects\nativescript\test1\platforms\android\build\intermediates\exploded-aar\barcodescanner-release\res\drawable\flip_camera.png
AAPT out(1144172198) : No Delegate set : lost message:  Output file: D:\projects\nativescript\test1\platforms\android\build\intermediates\res\merged\nativescript-barcodescannernativescriptcardviewnativescriptfloatingactionbuttonnativescript-liveeditnativescript-telerik-ui\debug\drawable\flip_camera.png
AAPT out(1144172198) : No Delegate set : lost message:Error
AAPT out(1144172198) : No Delegate set : lost message:Done

And then finally tns reports:

Command D:\projects\nativescript\test1\platforms\android\gradlew.bat failed with exit code 1

teobugslayer commented Feb 26, 2016

From @NathanaelA on February 26, 2016 4:8

In my case it threw lots of error messages about aapt not being able to process some of the png files. At the end it just says gradlew failed.

Here is one of the many errors sets (each error has these 5 lines)

AAPT out(1144172198) : No Delegate set : lost message:Crunching D:\projects\nativescript\test1\platforms\android\build\intermediates\exploded-aar\barcodescanner-release\res\drawable\flip_camera.png
AAPT out(1144172198) : No Delegate set : lost message:Crunching single PNG file: D:\projects\nativescript\test1\platforms\android\build\intermediates\exploded-aar\barcodescanner-release\res\drawable\flip_camera.png
AAPT out(1144172198) : No Delegate set : lost message:  Output file: D:\projects\nativescript\test1\platforms\android\build\intermediates\res\merged\nativescript-barcodescannernativescriptcardviewnativescriptfloatingactionbuttonnativescript-liveeditnativescript-telerik-ui\debug\drawable\flip_camera.png
AAPT out(1144172198) : No Delegate set : lost message:Error
AAPT out(1144172198) : No Delegate set : lost message:Done

And then finally tns reports:

Command D:\projects\nativescript\test1\platforms\android\gradlew.bat failed with exit code 1
@Plamen5kov

This comment has been minimized.

Show comment
Hide comment
@Plamen5kov

Plamen5kov Feb 26, 2016

Contributor

The problem is we currently make every plugin into a flavor (which is not the brightest idea), and gradle includes every flavor name to the name of the package somewhere in the build folder
This problem will automatically go away if there are only include.gradle, .aar files in the plugin's platforms/android folder.
Then gradle won't have to concat these plugin names at all.

Contributor

Plamen5kov commented Feb 26, 2016

The problem is we currently make every plugin into a flavor (which is not the brightest idea), and gradle includes every flavor name to the name of the package somewhere in the build folder
This problem will automatically go away if there are only include.gradle, .aar files in the plugin's platforms/android folder.
Then gradle won't have to concat these plugin names at all.

@NathanaelA

This comment has been minimized.

Show comment
Hide comment
@NathanaelA

NathanaelA Feb 26, 2016

Contributor

@Plamen5kov Well that is good that only occurs in certain cases. But odds are likely when you have several plugins that at least one of them has a .jar.

Question though that you might know:

productFlavors {
        "nscv" {
            dimension "nscv"
        }
}
  1. Do productFlavors Key and the dimension key have to be the same value? And if not which one of those does it use to concatenate?
Contributor

NathanaelA commented Feb 26, 2016

@Plamen5kov Well that is good that only occurs in certain cases. But odds are likely when you have several plugins that at least one of them has a .jar.

Question though that you might know:

productFlavors {
        "nscv" {
            dimension "nscv"
        }
}
  1. Do productFlavors Key and the dimension key have to be the same value? And if not which one of those does it use to concatenate?
@Plamen5kov

This comment has been minimized.

Show comment
Hide comment
@Plamen5kov

Plamen5kov Feb 29, 2016

Contributor

Hi, @NathanaelA
Odds really are that a plugin would have .jar and that's why the .aar format may include java source code that is later .jar-ed. Another option, still available would be to put a normal .jar inside <plugin-name>/platforms/android/ folder but it would have to be only custom source code which does not require any additional resources.
If you have the scenario where you need to use resources, you would need to use the .aar format, either by dragging it along with the plugin or simply by referencing it in the <plugin-name>/platforms/android/include.gradle file.
And as for the question: The productFlavors key needs to hold the same name as plugin's name in the package.json file. You can read more about flavor dimensions here.

Contributor

Plamen5kov commented Feb 29, 2016

Hi, @NathanaelA
Odds really are that a plugin would have .jar and that's why the .aar format may include java source code that is later .jar-ed. Another option, still available would be to put a normal .jar inside <plugin-name>/platforms/android/ folder but it would have to be only custom source code which does not require any additional resources.
If you have the scenario where you need to use resources, you would need to use the .aar format, either by dragging it along with the plugin or simply by referencing it in the <plugin-name>/platforms/android/include.gradle file.
And as for the question: The productFlavors key needs to hold the same name as plugin's name in the package.json file. You can read more about flavor dimensions here.

@NathanaelA

This comment has been minimized.

Show comment
Hide comment
@NathanaelA

NathanaelA Feb 29, 2016

Contributor

@PanayotCankov - Ok.

On my question; your answer doesn't match what I have found out.

productFlavors {
        "nscv" {
            dimension "nscv"
        }
}

This is actually the changed include.gradle file I'm using for the nativescript-cardview. This works perfectly fine and I did NOT change the name in the package.json or any other files of any of the plugins. I also did this same change to 5 or so other plugin's include.gradle files to use the 4-5 character shortened names. Everything builds and is working properly. So the include.gradle doesn't care about the package.json names as far as I can see..

Contributor

NathanaelA commented Feb 29, 2016

@PanayotCankov - Ok.

On my question; your answer doesn't match what I have found out.

productFlavors {
        "nscv" {
            dimension "nscv"
        }
}

This is actually the changed include.gradle file I'm using for the nativescript-cardview. This works perfectly fine and I did NOT change the name in the package.json or any other files of any of the plugins. I also did this same change to 5 or so other plugin's include.gradle files to use the 4-5 character shortened names. Everything builds and is working properly. So the include.gradle doesn't care about the package.json names as far as I can see..

@Plamen5kov

This comment has been minimized.

Show comment
Hide comment
@Plamen5kov

Plamen5kov Mar 1, 2016

Contributor

Hi, @NathanaelA

  • So the include.gradle doesn't care about the package.json names as far as I can see.., your's is an isolated case, because you have only an include.gradle file.
  • ...did this same change to 5 or so other plugin's include.gradle files... again if they only had an include.gradle file there shouldn't be any issues. And if there is an AndroidManifest.xml there is a possibility for them to build but not work properly.

The the cases I'm talking about include other scenarios which will be depricated soon. You can read more about them here, having in mind the linked documentation.

@NathanaelA it would be great if you be more careful while trying to @mention somebody.

Contributor

Plamen5kov commented Mar 1, 2016

Hi, @NathanaelA

  • So the include.gradle doesn't care about the package.json names as far as I can see.., your's is an isolated case, because you have only an include.gradle file.
  • ...did this same change to 5 or so other plugin's include.gradle files... again if they only had an include.gradle file there shouldn't be any issues. And if there is an AndroidManifest.xml there is a possibility for them to build but not work properly.

The the cases I'm talking about include other scenarios which will be depricated soon. You can read more about them here, having in mind the linked documentation.

@NathanaelA it would be great if you be more careful while trying to @mention somebody.

@NathanaelA

This comment has been minimized.

Show comment
Hide comment
@NathanaelA

NathanaelA Mar 1, 2016

Contributor

@Plamen5kov - Ok, So if I am following you; those cases you are talking about are going away and re-naming the fields inside the include.gradle will even be safer in the future. As it is currently the only way to get around this issue on windows. From what I gather from the changes in the docs, you will be pre-merging (during installation of the plugin) the android manifest to the master one now in /app/app_resources/android?

On the @mention, oy -- I can't believe I did that. Sorry PanayotCankov.

Contributor

NathanaelA commented Mar 1, 2016

@Plamen5kov - Ok, So if I am following you; those cases you are talking about are going away and re-naming the fields inside the include.gradle will even be safer in the future. As it is currently the only way to get around this issue on windows. From what I gather from the changes in the docs, you will be pre-merging (during installation of the plugin) the android manifest to the master one now in /app/app_resources/android?

On the @mention, oy -- I can't believe I did that. Sorry PanayotCankov.

@Plamen5kov

This comment has been minimized.

Show comment
Hide comment
@Plamen5kov

Plamen5kov Mar 1, 2016

Contributor

@NathanaelA there will be no need for any naming at all.

productFlavors {
        "nscv" {
            dimension "nscv"
        }
}

This whole block, will no longer be necessary.

It will be a breaking change for some plugins, and the scenarios I've described in the docs are what will be asked of developers to migrate to the new version of the plugins.
The unfortunate fact is, it will be a breaking change.
The good news is, for most plugins the migration should be easy.

I will take care of describing in detail how to migrate the current plugins!

Keep in mind the change won't happen overnight and you will have proper time to prepare and migrate the plugins.

Contributor

Plamen5kov commented Mar 1, 2016

@NathanaelA there will be no need for any naming at all.

productFlavors {
        "nscv" {
            dimension "nscv"
        }
}

This whole block, will no longer be necessary.

It will be a breaking change for some plugins, and the scenarios I've described in the docs are what will be asked of developers to migrate to the new version of the plugins.
The unfortunate fact is, it will be a breaking change.
The good news is, for most plugins the migration should be easy.

I will take care of describing in detail how to migrate the current plugins!

Keep in mind the change won't happen overnight and you will have proper time to prepare and migrate the plugins.

@NathanaelA

This comment has been minimized.

Show comment
Hide comment
@NathanaelA

NathanaelA Mar 1, 2016

Contributor

Oh, very SWEET! Is this a 1.7, 1.8 or something scheduled for the whole world breaks 2.0? 😀

Contributor

NathanaelA commented Mar 1, 2016

Oh, very SWEET! Is this a 1.7, 1.8 or something scheduled for the whole world breaks 2.0? 😀

@Plamen5kov

This comment has been minimized.

Show comment
Hide comment
@Plamen5kov

Plamen5kov Mar 1, 2016

Contributor

We haven't talked about it yet but it will be no earlier than 1.8

Contributor

Plamen5kov commented Mar 1, 2016

We haven't talked about it yet but it will be no earlier than 1.8

@Plamen5kov

This comment has been minimized.

Show comment
Hide comment
@Plamen5kov

Plamen5kov Mar 14, 2016

Contributor

Ping @tjvantoll , @EddyVerbruggen , @sitefinitysteve, @bradmartin, @AntonioCuevaUrraco, @hdeshev , @vakrilov , @PeterStaev, @alexziskind1, @emiloberg, @PanayotCankov , @TobiasHennig, @NathanaelA, @ginev

This is concerning plugin migration. With the 1.7 release there will be a warning during build time, saying some plugins have soon to be deprecated files. This warning will be there for one release and this will be the appropriate time to migrate your plugins to the new structure. Because this will be a breaking change there is a blog post describing how exactly to migrate your current plugins to the state they need to be in.

Contributor

Plamen5kov commented Mar 14, 2016

Ping @tjvantoll , @EddyVerbruggen , @sitefinitysteve, @bradmartin, @AntonioCuevaUrraco, @hdeshev , @vakrilov , @PeterStaev, @alexziskind1, @emiloberg, @PanayotCankov , @TobiasHennig, @NathanaelA, @ginev

This is concerning plugin migration. With the 1.7 release there will be a warning during build time, saying some plugins have soon to be deprecated files. This warning will be there for one release and this will be the appropriate time to migrate your plugins to the new structure. Because this will be a breaking change there is a blog post describing how exactly to migrate your current plugins to the state they need to be in.

@PeterStaev

This comment has been minimized.

Show comment
Hide comment
@PeterStaev

PeterStaev Mar 14, 2016

@Plamen5kov So if my plugin needs an AndroidManifest.xml file only because it requires some permissions, I need to create a full AAR file? This does not seem right.

PeterStaev commented Mar 14, 2016

@Plamen5kov So if my plugin needs an AndroidManifest.xml file only because it requires some permissions, I need to create a full AAR file? This does not seem right.

@EddyVerbruggen

This comment has been minimized.

Show comment
Hide comment
@EddyVerbruggen
Member

EddyVerbruggen commented Mar 14, 2016

@Plamen5kov

This comment has been minimized.

Show comment
Hide comment
@Plamen5kov

Plamen5kov Mar 14, 2016

Contributor

@PeterStaev
If your plugin requires only AndroidManifest.xml you'll do better to tell the user to all everything he needs in the app/App_Resources/Android/AndroidManifest.xml, in the other case where you'll need source code besides the AndroidManifest.xml :

Since 1.6 release there is an AndroidManifest.xml in `app/App_Resources/Android/` and you can put all configurations there.

@EddyVerbruggen
In this case you won't need a plugin at all you'll be able to tell the user to add all permissions the plugin needs in the app/App_Resources/Android/AndroidManifest.xml, which is the common way to do thing when developing with android plugins.

Contributor

Plamen5kov commented Mar 14, 2016

@PeterStaev
If your plugin requires only AndroidManifest.xml you'll do better to tell the user to all everything he needs in the app/App_Resources/Android/AndroidManifest.xml, in the other case where you'll need source code besides the AndroidManifest.xml :

Since 1.6 release there is an AndroidManifest.xml in `app/App_Resources/Android/` and you can put all configurations there.

@EddyVerbruggen
In this case you won't need a plugin at all you'll be able to tell the user to add all permissions the plugin needs in the app/App_Resources/Android/AndroidManifest.xml, which is the common way to do thing when developing with android plugins.

@bradmartin

This comment has been minimized.

Show comment
Hide comment
@bradmartin

bradmartin Mar 14, 2016

I see pro/con to having the user do this. The con being devs who have no
clue about permissions since the plugins have so far handled this. So there
should definitely be a doc section dedicated to explaining the permissions
to reduce the repetitive questions against plugins such as 'this didn't
work, it's broken' when it's just the permissions. The pro is the opposite
it gets users more comfortable with permissions and is something I think
they should know when developing mobile apps. Just my thoughts on telling
the user to add the permissions. I have no problem with the migration, as
long as it's not more work for us to ship plugins.

On Mon, Mar 14, 2016, 7:51 AM Plamen Petkov notifications@github.com
wrote:

@PeterStaev https://github.com/PeterStaev
If your plugin requires only AndroidManifest.xml you'll do better to tell
the user to all everything he needs in the
app/App_Resources/Android/AndroidManifest.xml, in the other case where
you'll need source code besides the AndroidManifest.xml :

Since 1.6 release there is an AndroidManifest.xml in app/App_Resources/Android/ and you can put all configurations there.

@EddyVerbruggen https://github.com/EddyVerbruggen
In this case you won't need a plugin at all you'll be able to tell the
user to add all permissions the plugin needs in the
app/App_Resources/Android/AndroidManifest.xml, which is the common way to
do thing when developing with android plugins.


Reply to this email directly or view it on GitHub
#369 (comment)
.

bradmartin commented Mar 14, 2016

I see pro/con to having the user do this. The con being devs who have no
clue about permissions since the plugins have so far handled this. So there
should definitely be a doc section dedicated to explaining the permissions
to reduce the repetitive questions against plugins such as 'this didn't
work, it's broken' when it's just the permissions. The pro is the opposite
it gets users more comfortable with permissions and is something I think
they should know when developing mobile apps. Just my thoughts on telling
the user to add the permissions. I have no problem with the migration, as
long as it's not more work for us to ship plugins.

On Mon, Mar 14, 2016, 7:51 AM Plamen Petkov notifications@github.com
wrote:

@PeterStaev https://github.com/PeterStaev
If your plugin requires only AndroidManifest.xml you'll do better to tell
the user to all everything he needs in the
app/App_Resources/Android/AndroidManifest.xml, in the other case where
you'll need source code besides the AndroidManifest.xml :

Since 1.6 release there is an AndroidManifest.xml in app/App_Resources/Android/ and you can put all configurations there.

@EddyVerbruggen https://github.com/EddyVerbruggen
In this case you won't need a plugin at all you'll be able to tell the
user to add all permissions the plugin needs in the
app/App_Resources/Android/AndroidManifest.xml, which is the common way to
do thing when developing with android plugins.


Reply to this email directly or view it on GitHub
#369 (comment)
.

@EddyVerbruggen

This comment has been minimized.

Show comment
Hide comment
@EddyVerbruggen

EddyVerbruggen Mar 14, 2016

Member

@Plamen5kov I like my plugins to be plug & play so usually I go to great lengths to achieve that. For me it doesn't feel right if a developer needs to manually adjust his project after he added my plugin.

Consider the case where a dev updates a plugin and in this new release a new permission is required. I can't expect devs to check the docs every time, which means I will have even more issues opened on my GitHub issue tracker.

And it's not only permissions I'm concerned about. For instance AdMob requires you to define an activity:
https://github.com/EddyVerbruggen/nativescript-admob/tree/master/platforms/android - devs may paste it in the wrong place and stuff won't work. I'd rather avoid unpleasant stuff like this.

IMO, if we want to attract webdevs and former Cordova users then we should not make it harder for them to use plugins.

Member

EddyVerbruggen commented Mar 14, 2016

@Plamen5kov I like my plugins to be plug & play so usually I go to great lengths to achieve that. For me it doesn't feel right if a developer needs to manually adjust his project after he added my plugin.

Consider the case where a dev updates a plugin and in this new release a new permission is required. I can't expect devs to check the docs every time, which means I will have even more issues opened on my GitHub issue tracker.

And it's not only permissions I'm concerned about. For instance AdMob requires you to define an activity:
https://github.com/EddyVerbruggen/nativescript-admob/tree/master/platforms/android - devs may paste it in the wrong place and stuff won't work. I'd rather avoid unpleasant stuff like this.

IMO, if we want to attract webdevs and former Cordova users then we should not make it harder for them to use plugins.

@PeterStaev

This comment has been minimized.

Show comment
Hide comment
@PeterStaev

PeterStaev Mar 14, 2016

@Plamen5kov I agree with @EddyVerbruggen on this one. Making plugins Plug & Play is there for cordova plugins, why complicate things here for developers asking them to read plugin documentation and manually add permissions? For people using the CLI it might be relatively OK as they most probably visited NPM/GitHub to find the plugin so they will see it in the readme. But imagine this for people using Telerik Platform that add plugins via Visual Studio extension UI for example. They will have absolutely no clue that they will have to add the permissions manually in order for the plugin to work.

PeterStaev commented Mar 14, 2016

@Plamen5kov I agree with @EddyVerbruggen on this one. Making plugins Plug & Play is there for cordova plugins, why complicate things here for developers asking them to read plugin documentation and manually add permissions? For people using the CLI it might be relatively OK as they most probably visited NPM/GitHub to find the plugin so they will see it in the readme. But imagine this for people using Telerik Platform that add plugins via Visual Studio extension UI for example. They will have absolutely no clue that they will have to add the permissions manually in order for the plugin to work.

@sitefinitysteve

This comment has been minimized.

Show comment
Hide comment
@sitefinitysteve

sitefinitysteve Mar 14, 2016

If it has to be this way, what if we had a spot in the package.json to define the required permissions and on build it checks to see if those are in the manifest and cancels the build with the proper message (although it really would just be nice to have the merge happen as is)

sitefinitysteve commented Mar 14, 2016

If it has to be this way, what if we had a spot in the package.json to define the required permissions and on build it checks to see if those are in the manifest and cancels the build with the proper message (although it really would just be nice to have the merge happen as is)

@Plamen5kov

This comment has been minimized.

Show comment
Hide comment
@Plamen5kov

Plamen5kov Mar 14, 2016

Contributor

@EddyVerbruggen, @PeterStaev I'm not disagreeing with you, quite the opposite, I would do exactly the same as you. I would want my plugins to be plug and play, and @EddyVerbruggen as you said I like my plugins to be plug & play so usually I go to great lengths to achieve that.. There is a way for you plugins to be plug & play that the way is creating an .aar file where you can declare whatever you want in your AndroidManifest.xml. You are right that it puts a strain on the plugin developer, but this will pay off in the future, when there isn't unexpected behavior because of the flying AndroidManifest.xml files.

Contributor

Plamen5kov commented Mar 14, 2016

@EddyVerbruggen, @PeterStaev I'm not disagreeing with you, quite the opposite, I would do exactly the same as you. I would want my plugins to be plug and play, and @EddyVerbruggen as you said I like my plugins to be plug & play so usually I go to great lengths to achieve that.. There is a way for you plugins to be plug & play that the way is creating an .aar file where you can declare whatever you want in your AndroidManifest.xml. You are right that it puts a strain on the plugin developer, but this will pay off in the future, when there isn't unexpected behavior because of the flying AndroidManifest.xml files.

@sitefinitysteve

This comment has been minimized.

Show comment
Hide comment
@sitefinitysteve

sitefinitysteve Mar 14, 2016

@Plamen5kov Are there (or will there be) detailed docs on how to make the aar (I've never done this before)

sitefinitysteve commented Mar 14, 2016

@Plamen5kov Are there (or will there be) detailed docs on how to make the aar (I've never done this before)

@NathanaelA

This comment has been minimized.

Show comment
Hide comment
@NathanaelA

NathanaelA Mar 15, 2016

Contributor

@slavchev I have a separate question, sorta related if I grab a plugin that has compileSDK & TargeSDK at 23; but my project is limited to 22; will it work with it? At this moment the only way to avoid the dreaded Android 6 perm issues (that won't be fixed until 1.8?) is to use Target of 22. So what happens in this case?

Contributor

NathanaelA commented Mar 15, 2016

@slavchev I have a separate question, sorta related if I grab a plugin that has compileSDK & TargeSDK at 23; but my project is limited to 22; will it work with it? At this moment the only way to avoid the dreaded Android 6 perm issues (that won't be fixed until 1.8?) is to use Target of 22. So what happens in this case?

@slavchev

This comment has been minimized.

Show comment
Hide comment
@slavchev

slavchev Mar 15, 2016

@NathanaelA I guess it will work as long as it uses API level 22 or lower. I have to test it to confirm though.

slavchev commented Mar 15, 2016

@NathanaelA I guess it will work as long as it uses API level 22 or lower. I have to test it to confirm though.

@NathanaelA

This comment has been minimized.

Show comment
Hide comment
@NathanaelA

NathanaelA Mar 15, 2016

Contributor

@slavchev that was my guess; I was hoping you guys had already ran into this and had a ready answer. 😀

Contributor

NathanaelA commented Mar 15, 2016

@slavchev that was my guess; I was hoping you guys had already ran into this and had a ready answer. 😀

@atanasovg atanasovg added this to the 2.0.0 (Under Review) milestone Mar 18, 2016

@Plamen5kov

This comment has been minimized.

Show comment
Hide comment
@Plamen5kov

Plamen5kov Mar 24, 2016

Contributor

The team gave a lot of thought on this discussion and one of the main problems we saw was the case when you would need to create an .aar file for the soul purpose of putting an AndroidManifest.xml in it.

We tried out testing the scenario where we build .aar file, instead of the plugin developers, as the you all suggested. What we tried doing is, introduce a build script that takes care of the specific permission issue. If we try to build every plugin that has only an AndroidManifest.xml file in its platforms/android folder we would add 4s per plugin, but that doesn't seem like a good solution.
The second approach we tried is pushing the AndroidManifest.xml file in a pre-build .aar file. This was considerably faster, but it caused another issues. The newly generated .aar files were invalid for gradle and it failed on the dexing phase, because the BuildConfig in the classes.jar file is the same for all plugins generated this way.

In conclusion, plugins made at build-time take a lot of time. A future solution that we can suggest is using CLI to help build your plugins. Nativescript CLI already has this as a story: NativeScript/nativescript-cli#586

Contributor

Plamen5kov commented Mar 24, 2016

The team gave a lot of thought on this discussion and one of the main problems we saw was the case when you would need to create an .aar file for the soul purpose of putting an AndroidManifest.xml in it.

We tried out testing the scenario where we build .aar file, instead of the plugin developers, as the you all suggested. What we tried doing is, introduce a build script that takes care of the specific permission issue. If we try to build every plugin that has only an AndroidManifest.xml file in its platforms/android folder we would add 4s per plugin, but that doesn't seem like a good solution.
The second approach we tried is pushing the AndroidManifest.xml file in a pre-build .aar file. This was considerably faster, but it caused another issues. The newly generated .aar files were invalid for gradle and it failed on the dexing phase, because the BuildConfig in the classes.jar file is the same for all plugins generated this way.

In conclusion, plugins made at build-time take a lot of time. A future solution that we can suggest is using CLI to help build your plugins. Nativescript CLI already has this as a story: NativeScript/nativescript-cli#586

@rosen-vladimirov

This comment has been minimized.

Show comment
Hide comment
@rosen-vladimirov

rosen-vladimirov Mar 28, 2016

Contributor

Hey guys, @Plamen5kov , @mslavchev ,

I'm wondering how we'll support plugin variables in case this suggestion is implemented. Currently you can define plugin variables in plugin's package.json and use them later in your AndroidManifest.xml (with 1.7.1 of CLI, which is just around the corner, you can use them in strings.xml as well). This is really important when you want to use the value of the variable in your plugin's native code. What will be the expected workflow in case we decide to drop separate files and require .aar file from plugin. How we'll replace the plugin variables values from project to plugin's .aar file?

Contributor

rosen-vladimirov commented Mar 28, 2016

Hey guys, @Plamen5kov , @mslavchev ,

I'm wondering how we'll support plugin variables in case this suggestion is implemented. Currently you can define plugin variables in plugin's package.json and use them later in your AndroidManifest.xml (with 1.7.1 of CLI, which is just around the corner, you can use them in strings.xml as well). This is really important when you want to use the value of the variable in your plugin's native code. What will be the expected workflow in case we decide to drop separate files and require .aar file from plugin. How we'll replace the plugin variables values from project to plugin's .aar file?

@atanasovg

This comment has been minimized.

Show comment
Hide comment
@atanasovg

atanasovg Apr 13, 2016

Member

I'd like to do a summary on this issue and how we are going to proceed in the short term. Let me begin with the engineering reasons behind the proposed breaking change with the .aar files.

Reasoning

The Philosophy

Me and the team believe this is the most sustainable engineering-wise solution in the long run. Our thinking is that we should stick with the underlying native platform tools instead of making our own fork (implementation) on top of it for the sake of hiding some of the native paradigms behind an assumingly easier to digest abstraction. So far our practical experience proved that fighting with Gradle is a battle already lost. Instead, we decided to embrace Gradle and walk our path together. With that said, and because .AAR files are the organic library format in the native Android World, we suggested the aforementioned breaking change.

Problems Solved

Besides the concrete bug with the long paths on Windows, the suggested change would have also fixed several issues related with merging resources through flavors. It would also make the Gradle build script less complicated and more straightforward.

Objections

We heard some valid concerns against the change:

  • For plugins carrying only AndroidManifest.xml in their platforms/android folder it would be an overhead to prepare a full-blown .AAR file to just embed the manifest.
  • Preparing the .AAR file itself requires knowledge of Gradle and Gradle's project structure.
  • Some plugin authors would not bother to migrate to the new format and would let users define the plugin requested permissions manually, in the App_Resources/Android/AndroidManifest.xml file.
  • The Plugin Variables CLI and AppBuilder functionality would be tricky, probably even impossible to adapt.

Workarounds

After we gathered this feedback we tried to extend the solution to mitigate the concerns. What we did was:

  • Treat each plugin as a Gradle project, referenced by the user application (we got a pull request implementing this approach by @PeterStaev). The trade-off with this approach is that each such plugin adds some additional 4 seconds on top of the main build. In other words, if the application utilizes, say 20 plugins (which is a common scenario for larger apps), then the total overhead is 80 seconds. But even this huge overhead wouldn't be such an issue for local CLI builds, mainly because of the graceful incremental build support in Gradle. In other words - the overhead would not be significant once all the plugins are prepared and cached. The major obstacle for us not to take this road are the cloud builds in Telerik Platform. Because these builds are not cached, the incremental support of Gradle is just not present and the above mentioned overhead would be present upon each build, which would simply be a show-stopper for us.
  • Prepare the .AAR file upon tns plugin add. While this seemed a viable solution at first, it ended up to the cloud builds and the lack of cache there.
  • Embed the AndroidManifest.xml in a pre-built temporary .AAR file but this turned not working at all.
  • A research whether something like a Manifest Merge Gradle task exists so that we may manually call it to merge plugin's manifest with the main one. It turned that such stand-alone task does not exist and there is no way to do a separate manifest merge outside the Gradle pipeline.

Summary

None of the workarounds worked for us. While we continue to believe that the originally suggested solution with .AAR files is the only viable engineering-wise solution, sustainable in the long run, we agreed that we will try to provide a temporary fix to the long paths issue on Windows by manually minifying the names (suggested by @NathanaelA). This approach may turn error-prone but for the time being this is the only solution that fixes the issue and seems to be working in all scenarios.

Member

atanasovg commented Apr 13, 2016

I'd like to do a summary on this issue and how we are going to proceed in the short term. Let me begin with the engineering reasons behind the proposed breaking change with the .aar files.

Reasoning

The Philosophy

Me and the team believe this is the most sustainable engineering-wise solution in the long run. Our thinking is that we should stick with the underlying native platform tools instead of making our own fork (implementation) on top of it for the sake of hiding some of the native paradigms behind an assumingly easier to digest abstraction. So far our practical experience proved that fighting with Gradle is a battle already lost. Instead, we decided to embrace Gradle and walk our path together. With that said, and because .AAR files are the organic library format in the native Android World, we suggested the aforementioned breaking change.

Problems Solved

Besides the concrete bug with the long paths on Windows, the suggested change would have also fixed several issues related with merging resources through flavors. It would also make the Gradle build script less complicated and more straightforward.

Objections

We heard some valid concerns against the change:

  • For plugins carrying only AndroidManifest.xml in their platforms/android folder it would be an overhead to prepare a full-blown .AAR file to just embed the manifest.
  • Preparing the .AAR file itself requires knowledge of Gradle and Gradle's project structure.
  • Some plugin authors would not bother to migrate to the new format and would let users define the plugin requested permissions manually, in the App_Resources/Android/AndroidManifest.xml file.
  • The Plugin Variables CLI and AppBuilder functionality would be tricky, probably even impossible to adapt.

Workarounds

After we gathered this feedback we tried to extend the solution to mitigate the concerns. What we did was:

  • Treat each plugin as a Gradle project, referenced by the user application (we got a pull request implementing this approach by @PeterStaev). The trade-off with this approach is that each such plugin adds some additional 4 seconds on top of the main build. In other words, if the application utilizes, say 20 plugins (which is a common scenario for larger apps), then the total overhead is 80 seconds. But even this huge overhead wouldn't be such an issue for local CLI builds, mainly because of the graceful incremental build support in Gradle. In other words - the overhead would not be significant once all the plugins are prepared and cached. The major obstacle for us not to take this road are the cloud builds in Telerik Platform. Because these builds are not cached, the incremental support of Gradle is just not present and the above mentioned overhead would be present upon each build, which would simply be a show-stopper for us.
  • Prepare the .AAR file upon tns plugin add. While this seemed a viable solution at first, it ended up to the cloud builds and the lack of cache there.
  • Embed the AndroidManifest.xml in a pre-built temporary .AAR file but this turned not working at all.
  • A research whether something like a Manifest Merge Gradle task exists so that we may manually call it to merge plugin's manifest with the main one. It turned that such stand-alone task does not exist and there is no way to do a separate manifest merge outside the Gradle pipeline.

Summary

None of the workarounds worked for us. While we continue to believe that the originally suggested solution with .AAR files is the only viable engineering-wise solution, sustainable in the long run, we agreed that we will try to provide a temporary fix to the long paths issue on Windows by manually minifying the names (suggested by @NathanaelA). This approach may turn error-prone but for the time being this is the only solution that fixes the issue and seems to be working in all scenarios.

@NathanaelA

This comment has been minimized.

Show comment
Hide comment
@NathanaelA

NathanaelA Apr 13, 2016

Contributor

@atanasovg - Awesome summary; I was under the impression that my technique that I came up with does not work in the cases the plugin actually has a AndroidManifest.xml -- apparently the gradle script won't be able to find and merge it... Did you find some way to mitigate this issue?

As to your workarounds issues -- here are a couple possible mitigation strategies:
1 & 2. I don't know how beefy your cloud building devices are are; but if you have the android sdk and tns tools on a ssd drive and make the actual builds be ran on ram disks; I would venture a guess that would pretty much eliminate any speed hit you would normally be taking, I could be wrong here as I'm not sure if the hit is caused by disk access speed or something inside gradle (i.e. processing); but it is a theory that could be easily tested and might allow Peter's workaround to be used as otherwise it seems to be a worthwhile solution choice other than your cloud build issues.

2 (only). The other choice would be to add some smarts to your own internal build scripts and start caching the pre-compiled plugins -- then during the installation of the plugins you add a step your build scripts to automatically pull the already pre-built aar files from your cache. At end of build, you add a step that runs through all the built aar files and checks to see if any of them aren't in your cache and then sticks them in the cache. Or as a stop gap measure you can simply take a nightly cron job and do what we do on the NS.Rocks site and pull a updated list of all the plugins and then nightly pre-build every single new plugin version, but I'm not sure how sustainable that will be if the eco system grows massively; but this could get you way way down the road before you have to worry about changing anything again (if ever)

\4. So in effect to solve that issue you would need a custom tool; which could cause more support issues and maintenance; could be done but not the ideal situation.

Contributor

NathanaelA commented Apr 13, 2016

@atanasovg - Awesome summary; I was under the impression that my technique that I came up with does not work in the cases the plugin actually has a AndroidManifest.xml -- apparently the gradle script won't be able to find and merge it... Did you find some way to mitigate this issue?

As to your workarounds issues -- here are a couple possible mitigation strategies:
1 & 2. I don't know how beefy your cloud building devices are are; but if you have the android sdk and tns tools on a ssd drive and make the actual builds be ran on ram disks; I would venture a guess that would pretty much eliminate any speed hit you would normally be taking, I could be wrong here as I'm not sure if the hit is caused by disk access speed or something inside gradle (i.e. processing); but it is a theory that could be easily tested and might allow Peter's workaround to be used as otherwise it seems to be a worthwhile solution choice other than your cloud build issues.

2 (only). The other choice would be to add some smarts to your own internal build scripts and start caching the pre-compiled plugins -- then during the installation of the plugins you add a step your build scripts to automatically pull the already pre-built aar files from your cache. At end of build, you add a step that runs through all the built aar files and checks to see if any of them aren't in your cache and then sticks them in the cache. Or as a stop gap measure you can simply take a nightly cron job and do what we do on the NS.Rocks site and pull a updated list of all the plugins and then nightly pre-build every single new plugin version, but I'm not sure how sustainable that will be if the eco system grows massively; but this could get you way way down the road before you have to worry about changing anything again (if ever)

\4. So in effect to solve that issue you would need a custom tool; which could cause more support issues and maintenance; could be done but not the ideal situation.

@bradmartin

This comment has been minimized.

Show comment
Hide comment
@bradmartin

bradmartin Apr 13, 2016

All of this is getting beyond what I know so when is all of this going into effect? What are the exact steps I need to take for all of my plugins to continue working properly? If this is going to cause version issues, is there a grace period? I have several plugins and if it involves a lot of work that is going to just suck. Sooner I know what I need to actually do I can start preparing/learning about .aar files and everything else. Just wanting to get ahead of whatever is going to be established. I might not fully understand the entire process, just don't want my plugins to stop working when the changes happen 😄

bradmartin commented Apr 13, 2016

All of this is getting beyond what I know so when is all of this going into effect? What are the exact steps I need to take for all of my plugins to continue working properly? If this is going to cause version issues, is there a grace period? I have several plugins and if it involves a lot of work that is going to just suck. Sooner I know what I need to actually do I can start preparing/learning about .aar files and everything else. Just wanting to get ahead of whatever is going to be established. I might not fully understand the entire process, just don't want my plugins to stop working when the changes happen 😄

@atanasovg

This comment has been minimized.

Show comment
Hide comment
@atanasovg

atanasovg Apr 13, 2016

Member

@bradmartin Plugin authors shouldn't change anything (probably that didn't become clear from my comment). We will do some dynamic name minification which should solve the issue with the long paths for now.

@NathanaelA @blagoev has found a solution (https://github.com/NativeScript/android-runtime/pull/419/files). We are still to run it through our CI infrastructure and plugin verification mechanism but it does seem working.

Member

atanasovg commented Apr 13, 2016

@bradmartin Plugin authors shouldn't change anything (probably that didn't become clear from my comment). We will do some dynamic name minification which should solve the issue with the long paths for now.

@NathanaelA @blagoev has found a solution (https://github.com/NativeScript/android-runtime/pull/419/files). We are still to run it through our CI infrastructure and plugin verification mechanism but it does seem working.

@NathanaelA

This comment has been minimized.

Show comment
Hide comment
@NathanaelA

NathanaelA Apr 13, 2016

Contributor

@atanasovg - I think both @bradmartin and I got the impression that leaving things with the shortened name hack was still a temporary solution and that in the near future the official method would be finally decided.

That is why I suggested a couple ways that might help narrow the field down because at this moment we appear to have a couple options but each of them has some downside.

Option 0 (original proposal) alone appears to be a non starter because of the added plugin developer work and appbuilder/plugin cli variables issues. However with option 4 in place (AndroidManifest merging , option 0 because valid as this eliminates the majority of the work for almost all plugins and allows you the Appbuilder/plugin cli variables to work. However, this requires building and mainating a new tool/scripts.

Options 1 & 2 seems like great solutions; but since they don't work for your build farm, I tossed out some suggestions that might allow them to work properly on your build servers in a fast method.

But I echo Brad's message; at the point the final solicitation is decided let us know so that we can get our plugins properly formatted...

Contributor

NathanaelA commented Apr 13, 2016

@atanasovg - I think both @bradmartin and I got the impression that leaving things with the shortened name hack was still a temporary solution and that in the near future the official method would be finally decided.

That is why I suggested a couple ways that might help narrow the field down because at this moment we appear to have a couple options but each of them has some downside.

Option 0 (original proposal) alone appears to be a non starter because of the added plugin developer work and appbuilder/plugin cli variables issues. However with option 4 in place (AndroidManifest merging , option 0 because valid as this eliminates the majority of the work for almost all plugins and allows you the Appbuilder/plugin cli variables to work. However, this requires building and mainating a new tool/scripts.

Options 1 & 2 seems like great solutions; but since they don't work for your build farm, I tossed out some suggestions that might allow them to work properly on your build servers in a fast method.

But I echo Brad's message; at the point the final solicitation is decided let us know so that we can get our plugins properly formatted...

@vbresults

This comment has been minimized.

Show comment
Hide comment
@vbresults

vbresults Nov 9, 2016

@Plamen5kov Please push out that automatic .aar builder, even if we have to add a --auto-build-aar to the prepare/run/build/emulate command to activate it, so there's a simple solution for us.

The extra 4 seconds is not a showstopper for everyone. It would be excellent if it just took the manifest and any .java files in that folder and built the .aar if that flag was provided.

vbresults commented Nov 9, 2016

@Plamen5kov Please push out that automatic .aar builder, even if we have to add a --auto-build-aar to the prepare/run/build/emulate command to activate it, so there's a simple solution for us.

The extra 4 seconds is not a showstopper for everyone. It would be excellent if it just took the manifest and any .java files in that folder and built the .aar if that flag was provided.

@Plamen5kov

This comment has been minimized.

Show comment
Hide comment
@Plamen5kov

Plamen5kov Nov 11, 2016

Contributor

Hi @vbresults,
Currently we don't have the resources to build such a tool, but we accept contributions from the community.

Contributor

Plamen5kov commented Nov 11, 2016

Hi @vbresults,
Currently we don't have the resources to build such a tool, but we accept contributions from the community.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment