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

Detect which Runtime? #93

Closed
NathanaelA opened this issue Jun 2, 2015 · 34 comments
Closed

Detect which Runtime? #93

NathanaelA opened this issue Jun 2, 2015 · 34 comments
Labels

Comments

@NathanaelA
Copy link
Contributor

Is their a command to get the current android runtime version, this would be very useful for add on. I've got a add-on that is going to require a patch that hasn't been deployed yet. This would be great if I could say
if (__versionRuntime <= "1.02") then doSomething()

@blagoev
Copy link
Contributor

blagoev commented Jun 4, 2015

Thanks for the suggestion, we will consider how this can be best done and what implications it has.

@NathanaelA

This comment was marked as abuse.

@Plamen5kov
Copy link
Contributor

Hi, @NathanaelA
Can you say what particular problems you are having with v 1.6.0?
And because you mention that having the version easily available would be very helpful to plugin developers, could you share what scenarios and user cases you have in mind. Having a version available in javascript is one possible solution but it might not be the best one, so IMHO we should explore what other options there are before committing to anything.
Having a version of the runtime easily available in javascript seems like a slippery slope to me.

EDIT: If you wan't to use the version of the runtime as a workaround you can do:

var packageJson = require("./package.json");
var runtimeVersion = packageJson.version;

@NathanaelA

This comment was marked as abuse.

@NathanaelA

This comment was marked as abuse.

@Plamen5kov
Copy link
Contributor

The main problem you are facing will be fixed with 1.6.2 release with this commit.
As far as i know, although there is an issue on the version matter, it hasn't been accepted and as far as feature parity goes it won't be a good idea to have this feature present only in the android runtime. I never said your case is an isolated one and please try to understand that I want to find the best solution for the problem.
I can't be convinced by the fact that everyone else is using versioning. By the same logic if everyone else were using feature detection it would be a good idea for us to do it as well.

@slavchev
Copy link

Hi @NathanaelA

I've read carefully your points and while I understand them I don't think versioning will help. Before I go into the details I would like to make it clear that whatever decision we take there will be a trade-off.

About 1) It is indeed a breaking change on our end. We did a change in modules and the runtime and It was hard to assess the impact of it. As a result we break some existing apps. We already committed the fix in the release branch and this morning we discussed to release a new version 1.6.2 tomorrow. I don't want people to use any versioning scheme in their plugins in order to provide a workaround for the breaking change we made. I prefer to fix the bug once. Also, suppose that this bug resurrects in a future version. It is not unheard, nobody's regression tests are perfect. I don't consider versioning a good tool in this scenario.

Points 2) and 3) are similar to 1) I guess this kind of issues will be eventually solved in different manner. I guess the npm builtin versioning comparability feature will be the preferred approach. Also, we can take a look at the guys from WordPress and see how they deal with the versioning/plugins issue. There is no silver bullet and in both cases there is always a testing step involved.

About 4) I think the explicit feature/API availability/discoverability is always proffered to version checks. I would love to discuss how this can be improved in {N} and implement a proper solution. Many of us remember the User-Agent horror and the problems it creates. I definitely don't want to go that way.

@NathanaelA

This comment was marked as abuse.

@slavchev
Copy link

@NathanaelA

I do realize how my concerns about versioning may look hostile for many of the community. I have to say that I value your opinion and the great work you do for {N} :) Please don't take my unwillingness to introduce versioning as something set in stone. Yet, I am not convinced versioning is a good way to go. Once we introduced it will be very hard and painful to remove it if we revisit our decision. Also, please note that I don't want take this decision by myself and I would rather keep the discussion going.

I hope that all can see that for important and critical breaking changes, like #362, we try to provide a fix as fast as possible. In the concrete example our time to reaction is just a few days. I hope this gains the trust of the community. Also, as an active community member you have very quick time to react for every {N} release. Sometimes it may seem that we are holding you back but it is definitely not our intention. It is just the fact that we are trying to be more conservative when providing a new feature. I do realize this can be frustrating and I guess it is a matter of time until both sides learn to deal with such issue. We will try to act more quickly, that's for sure.

Now, about the versioning. We've all seen code full of version checks and it is not pretty. I don't mind if it is pretty or not but more importantly often these checks don't work for many scenarios. Bug regression is just one example. There are other alternatives to versioning. We can provide API contracts similar to the ones in Android or UWP for example. Also, maybe it is a good time to revisit our thinking about semver. As I said earlier I would love to help and dig deeper into the this and provide solutions starting with the most critical issues. Also, I would encourage everybody involved to look closely at solutions provided by npm and WordPress.

Besides, because the runtimes are npm modules we have the version info already. Look at the package.json which is located as a sibling to the app folder. It looks as follows:

{
    "nativescript": {
        "id": "org.nativescript.app",
        "tns-android": {
            "version": "1.6.1"
        }
    },
    "dependencies": {
        "tns-core-modules": "1.6.0"
    }
}

I will check with @teobugslayer and @rosen-vladimirov to see whether it is a good idea to merge its content with app/package.json file so that @Plamen5kov proposal can work.

In any way, I would kindly ask not to rush in providing versioning mechanism without thorough discussion.

@NathanaelA

This comment was marked as abuse.

@slavchev
Copy link

@NathanaelA

I had a short discussion with @rosen-vladimirov and we analyzed the current state and how it can be improved. Part of the current problem is that we allow app upgrade and the user ends with old plugins + new runtime. It is important to state that while we use npm we don't take advantage of its full features and we treat versioning in a slightly different way. We realize that fixing this is part of the problem. Also, the idea of semver has very strong support and we consider to revisit our decision and adopt it. However, this will probably happen for 2.0 release.

We are going to create a proposal at https://github.com/NativeScript/nativescript-cli/issues because it is not Android specific and we consider there it will have much better visibility. I will provide a link once it is available.

EDIT: see NativeScript/nativescript-cli#1530

@NathanaelA

This comment was marked as abuse.

@slavchev
Copy link

If it is a only matter of a simple if statements I would probably publish two versions of my AwesomePlugin. When the logic is more complex I would go for my own dependency, build two versions of it and then consume it from (two different versions of) my AwesomePlugin. I don't see publishing multiple versions a big price to pay as I am already doing it. But it is my way of thinking.

@rosen-vladimirov
Copy link
Contributor

Hi @NathanaelA ,
Intersting example. However it means you want one version in your plugin to be used with two versions of the runtime, which have breaking changes. So this sounds really cool, but I do not feel it is correct in npm world. For example when nodejs released 4.0.0 there were tons of breaking changes. All npm packages that were using old API should have been upgraded no matter of their current release cycle. Of course the update made some packages to stop working with older nodejs versions, but that's fine as you have old versions of the package working with it.
As I've been working on the support for different nodejs versions in nativescript-cli, I can tell you - you do not want to support different major versions in a single version of your product. Currently nativescript-cli can work with nodejs ~0.10.38, ~0.12.2, ^4.2.1, ^5.1.0 but this is really, really hard to achieve and support.
So let me get back to your example - I would vote for releasing 2.0.0 compatible version with all your new features and make it work with newest runtime version. In case someone wants to use them - upgrade the runtime as well. In case you have to write a lot of code, you can just release 2.0.0 which is compatible with 1.5 and later, release 3.0.0 which is compatible with runtime 2.0.0
You'll always have a corner case when some change (in runtimes, modules, cli, plugin, nodejs, v8 or whatever) will prevent some users from upgrading. But I would definitely use the new features in all of my new versions and advise the users to upgrade.

@NathanaelA

This comment was marked as abuse.

@slavchev
Copy link

@NathanaelA

I think there is some misunderstanding. The (main) code base is one. You have two flavors. Does it really matter if you have

if (version_check) {
  goLeft()
} else {
  goRight()
}

or two dependencies

// left flavor
goLeft()

and

// right flavor
goRight()

@NathanaelA

This comment was marked as abuse.

@NathanaelA

This comment was marked as abuse.

@slavchev
Copy link

OK, I let me be more precise

if (version_check) {
  goLeft();
} else {
  goRight()
}

And

// main code base
go()
// left flavor (npm dependency)
function go() {
   goLeft();
}
// right flavor (npm dependency)
function go() {
   goRight()
}

@sitefinitysteve
Copy link

IMO, I don't see why it totally matters, if you don't want to use it yourselves, don't, but if it helps @NathanaelA then let that slippery slope be his to traverse if he wants...

As long as exposing it doesn't cause undue burden to the {N} team... I think it can only help plugin developers.

I personally don't want to have
nativescript-myplugin-v1
nativescript-myplugin-v2
...
nativescript-myplugin-v13
if there's just like a 1-3 line change\patch per version

@slavchev
Copy link

@sitefinitysteve

I think this one of those issues when we know we won't get it right in the first time. That's why we keep this discussion and try to find out a good compromise for most of us, hopefully for everyone.

Also, as I said if it is just a few if statements then I would just create a branch for the old version, build a package, publish it and continue to work in the master branch. You can back-port to the older version branch as long as you think it is feasible. Just my 2 cents.

@sitefinitysteve
Copy link

@slavchev Yeah thats fine, nobody is bent out of shape over it, it's your party :) But like @NathanaelA says, you guys are already internally doing these kinds of checks, what would it fundamentally hurt to expose this to us through the api so we could use it (even if you dont)

@rosen-vladimirov
Copy link
Contributor

@NathanaelA , I'm expressing my own opinion what I would do in case I have to support a plugin.
Btw nativescript-cli detects the nodejs version in order to stop execution in case some really not-working version is used (for example 4.2.0). We've added this check just to stop answering questions like: "Hey why I receive this error when I execute tns ". It's optimization of the support resources 😸
Again, I would like to say that if I'm working on a plugin, I would not include such ifs in my code. But I like your arguments - the whole NativeScript project benefits from such discussions and real-world scenarios.

@NathanaelA

This comment was marked as abuse.

@NathanaelA

This comment was marked as abuse.

@slavchev
Copy link

TL;DR I describe how I use the standard version branches

First thing first. Current tns doesn't use the full npm functionality but all described scenarios are supported in npm.

OK, I am a plugin author and I work on AwesomePlugin (AP). The current version of the runtime (RT) is version A. I work in master branch and I am ready with my AP and there comes RT version B.

Scenario 1

My AP is compatible with both RT version A and B so I tag my current commit, declare my compatatibility (with A and B), build and publish my AP.

Scenario 2

There is a breaking change in RT version B and my AP doesn't work anymore. Say a simple if would suffice to fix the compatibility. I would not go for this option but rather for the next one.

Scenario 3

There is a breaking change in RT version B and my AP doesn't work anymore. I branch from c1 as shown below

------(c1)---------- [master branch]
         \
          \
           \--------(c2) [VESION_A branch]

In commit c2 I set my dependency to RT version A, tag the commit, build and publish VERSION_A of my AP.

Then I continue work in master branch and adapt my code for RT version B in commit c3

------(c1)------------------(c3)------ [master branch]
         \
          \
           \--------(c2) [VESION_A branch]

Once I am ready, I branch from c3 and follow the same steps.

------(c1)------------------(c3)------ [master branch]
         \                   \ 
          \                   \------(c4) [VERSION_B branch]
           \
            \--------(c2) [VESION_A branch]

In commit c4 I set my dependency to RT version B, tag the commit, build and publish VERSION_B of my AP.

I continue to work in master branch and I create a handy feature with commit c5

------(c1)------------------(c3)---------------(c5)--- [master branch]
         \                   \ 
          \                   \------(c4) [VERSION_B branch]
           \
            \--------(c2) [VESION_A branch]

I want to provide this feature to my existing versions of AP (VERSION_A and VERSION_B) so I back-port it with commits c6 and c7 accordingly.

------(c1)------------------(c3)---------------(c5)--- [master branch]
         \                   \ 
          \                   \------(c4)-----(c6)--- [VERSION_B branch]
           \
            \--------(c2)----(c7)--- [VESION_A branch]

I tag c6 and c7 commit, build and publish new versions, VERSION_A1 and VESION_B1, of my AP.

I support a old version branches as long as I find it feasible, For example, I may decide to back-port my new feature only to VERSION_A of my AP.

At that point a bug in RT version A is discovered. It turned out the bug in fixed in RT version B. I switch to VERSION_A branch and fix the bug with commit c8.

------(c1)------------------(c3)---------------(c5)--- [master branch]
         \                   \ 
          \                   \------(c4)-----(c6)--- [VERSION_B branch]
           \
            \--------(c2)----(c7)--------------------------(c8)---- [VESION_A branch]

I tag c8 commit, build and publish new version of my AP.

Scenario 4

There is a breaking change in RT version B and my AP doesn't work anymore. The change in RT version B is a big one and I decide to abstract it in a new dependency. I will keep it short this time but I am going to apply the same approach from scenario 3.

I build my AwesomePlugin component (APC). I branch where I have to and build and publish to versions VERSION_APCA and VERSION_APCB. Then I use APC in two different branches of my AP, build and publish two of it VERSION_A and VERSION_A and I set VERSION_A to have dependency to RT version A and VERSION_APCA. Next, I set VERSION_B to have dependency to RT version B and VERSION_APCB. This is really a variation of the previous scenario.

Again, this is what I would do, YMMV.

EDIT: As far for the V8, they use the described approach https://github.com/v8/v8/network

@blagoev
Copy link
Contributor

blagoev commented Feb 24, 2016

There are example of both worlds. For example Chrome does not allow you to detect the v8 runtime version. At least is not easy and totally hacky. Also note that there are people trying to do just that.

On the other hand Android is good example of promoting version checks for multiple platforms support. You can also detect if it is art or dalvik runtime executing your code.

Also .NET have very easy version checking. https://msdn.microsoft.com/en-us/library/system.environment.version.aspx

I am not opposed to the idea of having version exposed if that will make someone life easy. Having the version information can help someone shoot himself in the foot, but that's fine if that's what he wants/needs to do. Stopping someone from messing something by hiding information will only make them make even more hacks to get what they need. For example including build hooks in plugins to hard code the package.json version to be available at runtime.

So in short my vote is inclined towards what @NathanaelA have requested.

@NathanaelA

This comment was marked as abuse.

@slavchev
Copy link

I tried to analyze the reasons behind this issue. I tried to answer questions like why it was open last June and why it resurfaces now. I think this is mainly because we did a breaking change, namely NativeScript/NativeScript#1577.

Also, there is another important thing. While tns is build around npm we don't take full advantage of npm and even break some of its assumptions. As I said this was discussed with @rosen-vladimirov and this results in NativeScript/nativescript-cli#1530.

I looked deeper in npm and found the following pages

Here is a short excerpt.

In some cases, you want to express the compatibility of your package with a host tool or library, while not necessarily doing a require of this host. This is usually referred to as a plugin.

Currently we have tns-android module but we treat it in a special way. My idea is to treat tns-android as an ordinary plugin (some months ago I created a plugin that containes *.so files, not to be mistaken with native module so I know it is possible). Then it will be possible to use peerDependencies and set dependency to the runtime. In short, the more I think about the problem the more I am convinced that providing proper dependency mechanism is preferred to runtime version checking.

What do you think?

@NathanaelA

This comment was marked as abuse.

@sitefinitysteve
Copy link

+1 for letting us choose and avoid branch complexity... If we want to for plugin X. If it's a TINY change i just want a simple if statement...

@slavchev
Copy link

Guys, the more we discuss the more I get confused 😄

I described what git workflow I use, I never meant that you should do the same. I guess it is my fault that I haven't been more clear about that. Everybody uses some workflow that works for him/her and it is not my business to tell the people what to do 😄

If you don't want to have multiple branches that is fine with me. Nevertheless I would like to understand what do you think about the following suggestion. The same scenario as before. You are about to ship your plugin and we release a new version of the runtime with a breaking change that you can easily workaround with an if statement. Here is my suggestion:

  • publish your plugin as it is (having dependency on the old runtime)
  • apply the changes to make your plugin compatible with the new runtime (no if required)
  • publish again your plugin (having dependency on the new runtime)

Is it a high price to pay? Do you want to avoid having your plugin published twice?

All we want is to make it easier for you guys. We did a mistake by using npm in a wrong way and now we are trying to fix that. And as far as I can see we have your support NativeScript/nativescript-cli#1530 (comment). It is important for us to understand your scenarios.

@NathanaelA

This comment was marked as abuse.

@NathanaelA

This comment was marked as abuse.

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

No branches or pull requests

7 participants