search

CellularPrivacy / Android-IMSI-Catcher-Detector

AIMSICD • Fight IMSI-Catcher, StingRay and silent SMS!
https://cellularprivacy.github.io/Android-IMSI-Catcher-Detector/
GNU General Public License v3.0
4.7k stars 943 forks source link

Replacing RootTools with RootShell #240

Closed SecUpwN closed 9 years ago

SecUpwN commented 9 years ago

As proposed in here, @E3V3A found some advantages to switch to libsuperuser by @Chainfire.

I like the idea to change to use libsuperuser. Chainfire is always on top of these things and the APIs, and general system bugs, so playing along with that, would be an advantage.

Although I like RootTools, we currently constantly experience messages like this in our logfiles:

:third_party:RootTools:compileDebugJavaNote: /home/travis/build/SecUpwN/Android-IMSI-Catcher-Detector/third_party/RootTools/src/main/java/com/stericson/RootTools/internal/RootToolsInternalMethods.java uses or overrides a deprecated API.

With our App getting even more complex in the future, we rely on stable and crisp root functions.

Hope @Stericson won't be sad about our project making the switch over to libsuperuser. The way I would like this replacement to be, is that we somehow link our App code to the latest binary from the libsuperuser repo to automatically stay to knotch on that (if possible). How to do this? Discuss please.

Stericson commented 9 years ago

No Problem guys. :)

E3V3A commented 9 years ago

@Stericson Thanks. But do you know what's causing that error above? (or how to fix it?)

Stericson commented 9 years ago

Yes, I know what is likely causing it.

RootTools is made to support devices going all the way back to 1.6. There are two methods that RootTools uses which were deprecated in API level 18. The two methods in question is:

StatFs.getBlockSize() AND StatFs.getAvailableBlocks();

That's why you are seeing the warnings.

E3V3A commented 9 years ago

@Stericson Great! So is there a work around, or an available method that is not deprecated? Or should we just ignore it?

Stericson commented 9 years ago

You can fix it by using:

getAvailableBlocksLong() AND getBlockCountLong()

As long as you are at or above api v18 you shouldn't have any problems.

E3V3A commented 9 years ago

Ah, but we're at API 17, so I guess that we'll have to live with that warning a bit longer.

Stericson commented 9 years ago

Looks like before you were targeting API v19, you may have changed it to v17 now. With that change are you sure you are still getting this warning?

E3V3A commented 9 years ago

Ah, good observation. We've been always targeting API 17, but I just found some code targeted for API 19 a few days ago, and hopefully fixed it.

@SecUpwN Can you check that this is still a valid issue?

SecUpwN commented 9 years ago

Can you check that this is still a valid issue?

Analyzing the last 10 logfiles from Travis CI, the warning about the deprecated API are gone.

@Stericson, thank you very much about being so open-minded about the change of the library proposed in the Issue. I'm having a somewhat hard time throwing our RootTools because of that.

RootTools is made to support devices going all the way back to 1.6.

@E3V3A, have you found information on how far back the support of libsuperuser goes? Since there is still one unmerged pull request by @Ueland, maybe he can add this switch as well (if possible)?

E3V3A commented 9 years ago

From here it seem to be API 4. But this is really irrelevant. If build no longer complains with this error then let's close it. I added REMEMBER label, in case we actually do need the change in the future.

Stericson commented 9 years ago

By the way, if you guys are looking for something slimmed down or for something new, take a look at this:

https://github.com/Stericson/RootShell

It's a slimmed down version of RootTools. It has pretty much only the Core of RootTools, the shell.

Not sure if this is up your alley or not but I thought I'd mention it. I just made it public today. Sometime in the future I'll be updating RootTools to utilize this as well.

SecUpwN commented 9 years ago

Huge thanks for chiming in again and recommending this, @Stericson! What are the advantages of RootShell compared to RootTools? Would it make our App smaller, faster or any better? Please tell us how to best integrate it (feel free to craft a pull request for this) and "convince" our developers. :smiley_cat:

Stericson commented 9 years ago

It will make your application smaller.

I'm not certain what methods you use from RootTools, but if you are primarily only using the shell, isRootAvailable, or isAccessGiven from RootTools then RootShell would be a good switch.

RootTools has a lot of other overhead in the form of additional methods such as remounting partitions, finding busybox, etc...

Additionally, this release has a number of bug fixes. These bug fixes will also make it to RootTools, but it will take me a little bit of time to migrate them over to RootTools.

SecUpwN commented 9 years ago

I'm not certain what methods you use from RootTools, but if you are primarily only using the shell, isRootAvailable, or isAccessGiven from RootTools then RootShell would be a good switch.

Great! Not to step on your toes @Stericson, but are you going to update RootShell more often than RootTools? It was one of the reasons why we've even considered making the switch, it's just that we felt that another library may be better suited to stay ahead of general bugs and API challenges. Of course I know that you've got a life (and hopefully a girlfriend). But would it be possible to somehow link our code to always use the very latest RootShell with all updates? If so, feel free to construct a pull request!

Stericson commented 9 years ago

Sorry for the delay in response, the new years took me away from my machine.

It should stay up to date, I use this in my own applications so I'm pretty good at keeping RootTools/RootShell up to date with the latest changes that come about.

I can look at how you are using things and construct a pull request down the road.

SecUpwN commented 9 years ago

I can look at how you are using things and construct a pull request down the road.

That would be awesome, @Stericson! Please wait with forking our repo until #213 is solved for good.

E3V3A commented 9 years ago

Seem that it was just fixed..

Stericson commented 9 years ago

Cool, I'll look into this. I did some basic digging and think you guys could migrate to RootShell which will slim down the code a bit. I'll work on cloning the project and making certain that's the case.

SecUpwN commented 9 years ago

I'll work on cloning the project and making certain that's the case.

You'll likely have to clone again since #213 was not really fixed yet, @Stericson. Now it should be ~35 MB in size and work as expected. I am waiting for your pull request now and will be happy to merge it!

Stericson commented 9 years ago

Sounds good, it might be a week or so before I make the pull request. I need to make sure things are good to go before integrating it into your project.

SecUpwN commented 9 years ago

Sounds good, it might be a week or so before I make the pull request. I need to make sure things are good to go before integrating it into your project.

Fair thing. Please fork our project in a week then to have the most current state. Thanks! :+1:

SecUpwN commented 9 years ago

This Issue has been solved with #265. Awesome work @Stericson, blessings towards your way!