Comments (20)
@PerfectSlayer so I've made some changes after your suggestions.
enqueue
is renamed tosubmit
. The original idea is from JobIntentService, but I personally also prefer the Java Executor way- Removed
Job.onResult
and add an overloadJob.submit(ResultCallback)
I added chaining operations, so the final Job
class is like this:
public abstract static class Job {
public abstract Job to(List<String> stdout);
public abstract Job to(List<String> stdout, List<String> stderr);
public abstract Job add(InputStream in);
public abstract Job add(String... cmds);
public abstract Result exec();
public abstract void submit();
public abstract void submit(ResultCallback cb);
}
Along with these changes two non-static APIs in Shell
are replaced with a new one:
// Add
public abstract Job newJob();
// Remove
public abstract Job newJob(InputStream in);
public abstract Job newJob(String... cmds);
To directly interact with a Shell
instance will be like
shell.newJob().add(InputStream).add(cmds....).add(...).exec()
However, I did not touch the high level APIs, because in most cases people will just use Shell.su(cmds...)
or Shell.su(in)
(at least in my case). New operations can also be added to the returned Job
from the high level APIs, so nothing is missing there.
Also, I personally do not like method names to be too verbose (contradict with common Java code naming conventions LOL), so I named my API very simple, like add
and to
. Also I would love to leave the to(stdout)
and to(stdout, stderr)
methods untouched too, because in the case of to(stdout)
, it will check the global flag FLAG_REDIRECT_STDERR
to decide whether STDERR should be collected. To only collect STDERR, just use to(null, List)
.
from libsu.
@victorlapin there are still bugs in the old versions and I don't want to maintain two branches alongside.
All methods and classes in the shim are annotated @Deprecated
, so developers using a decent IDE will be notified to use the newer API.
The shim is fairly simple so it wouldn't be too complicated to maintain, since the previous API is mostly a collection of method overrides and actually only rely on a few key methods. I just need to transform the few key methods and it's good to go!
libsu
is meant to have 0 dependencies itself so I kept it nice and clean. Creating RxJava2 wrapper is definitely very cool! I think it is also possible to wrap it with Support Library's Lifecycle-Aware Components. I'll leave these to other developers though, my main goal should be making this library as robust as possible!
from libsu.
Hello @PerfectSlayer, thanks for all your suggestions! I'm actually quite new to Java development, and I appreciate receiving advice from others 👍
My library is inspired by Chainfire's libsuperuser. The biggest difference I'm aiming for is a single root shell that is shared by all operations across the app. Most of the design decision is based on my own personal needs in Magisk Manager and cues from libsuperuser
.
I agree with your opinion that an Output
class will be a good idea (and the return code). Currently my idea is to have the class be like:
class Output {
List<String> outList;
List<String> errList;
int returnCode;
}
However, it is not possible to merge all sh/su(.....)
to a single Output sh/su(cmds...)
, because in many cases, the outList and errList has to be passed in for special purposes, for example updating the UI along with shell outputs by using CallbackList
(check my example module).
So my idea would be these 2 methods:
Output sh/su(String...)
to let the method construct anOutput
for youvoid sh/su(String..., Output)
to let the shell directly output to the passed inOutput
instance
(This also fixes the parameter positioning you also mentioned)
For Shell.Initializer
, yes we can actually check root access with shell.getStatus() >= ROOT_SHELL
, so onShellInit
and onRootShellInit
is indeed duplicated in a way, I will consider removing one. About throwing an Exception
and returning boolean
: any Exception
means the initialization has an error; it is also possible that something other than exceptions happens, and the developer would like to mark the initialization as failure so I also allowed returning false
. You can argue that developers can catch the Exceptions
themselves, and return false
in the catch block if they want, but I just think it is handy to have both options available. Not sure if that actually created more confusion, I would love your thoughts here.
Shell.Task
, execTask(Task)
is meant to be used for low level implementation, it opens up an API to extend what Shell
can do. In fact, all command/inputstream methods are built on top of executing Shell.Task
s. Many operations in com.topjohnwu.superuser.io
classes are also based on execTask
, so it is meant to be very primitive. I let the Throwable
thrown in executing the task returned by execTask(Task)
instead of thowing it again is because I dislike tons of try {} catch(){}
. IMO it makes code more messy. But this is only my personal opinion, I'm not sure if this is against general Java coding standards, and I would love to know your opinion here.
Lastly, returning empty collections is a good idea, this is something I will definitely change. For varargs, I find it easier for me to use the methods, and if you really need to dynamically construct commands, it is fairly easy to construct it with List
s, and use list.toArray(new String[0])
and pass the String
array to the method, since Java is smart enough to treat arrays as varargs.
Again, thanks for all the suggestions!
from libsu.
You're welcome! I might not be the best Android developer but I can definitely smell when something looks weird using an API 😉
Design decisions
My library is inspired by Chainfire's libsuperuser. The biggest difference I'm aiming for is a single root shell that is shared by all operations across the app.
About design decisions, it could be nice to add them to the README. It helps a lot to understand where the ideas come from and where the development should go. Specially when you are developing a root application, there is a lot of chance you already know the Jorrit's library.
Output class and callback
The output class (or any better name) seems definitely a good idea to explore.
Java callbacks
About the callback, it feels weird to use an input parameter in Java.
There is some similar concepts in Java:
java.util.Observable
(JDK1) which allows callbacks to observe it to get notified (really useful for UI when Java got released),java.util.concurrent.Future
(Java 1.5) which allows to be notified when a result is available,- It evolves with
java.util.concurrent.CompletableFuture
(Java 1.8) which allows composition but the API is harder to use. Observable
was deprecated in Java 9 and replaced byjava.util.concurrent.Flow
which defines standard interface for reactive API. I know Java 9 is not supported in Android but reactive programming has a great adoption within Android ecosystem.- About Android,
android.arch.lifecycle.LiveData
could also be a solution. It could be observed and it tends to be the Google way to do it. It also offers a wrapper to reactive API.
Su/sh callback
Let's say we have an Output
class which instances are returned from Sync shell.
class Output {
List<String> out;
…
}
We could have a subclass or an inherited interface, let's call it LiveOutput
, which instance are returned from the Async shell. It should give the same API than Output
when the command is finished but also provides API to be notified of updates.
So using those concepts leads to a lot of possible API:
- LiveOutput could be a LiveData:
class LiveOutput extends MutableLiveData<Output> {
…
}
// Run command and update the UI on update
LiveOutput liveOutput= su("command")
liveOutput.observe(activity, output -> this.displayOutputs(output.getOut()));
// Just get output lines
List<String> out = liveOutput.get().getOut();
- Output could contains LiveData:
class LiveOutput {
LiveData<List<String> err;
LiveData<List<String> out;
LiveData<Integer> returnCode;
}
// Run command and update the UI on update
LiveOuput output = su("command");
output.out.observer(activity, this::displayOutputs);
// Just get output lines
List<String> out = output.out.get();
- Output could be a producer (reactive streams API) of its content:
class LiveOutput implements Producer<String>, Producer<Integer> {
// We see there is an issue here because Producer<String> is the same for err and out
List<String> out;
…
}
// Run command and update the UI on update
LiveOutput output = su("command");
output.subscribe(new Observer() {
public void onComplete() { }
public void onError(Throwable throwable) { }
public void onNext(String out) {
displayOutput(out);
}
public void onSubscribe(Subscription subscription) { }
});
// Just get output lines
List<String> out = output.getOut();
- Output can offer its own callback mechanism:
class LiveOutput extends Output {
public void onOutput(Consumer<String> consumer) {
…
}
}
// Run command and update the UI on update
LiveOutput output = su("command");
output.onOutput(this::displayOutputs);
// Just get output lines
List<String> out = output.getOut();
My opinion
The solution 1 looks like a bag of everything where any field update will trigger every observer and each observer will need to check if the data they are interested in has changed or not. Quite ugly and inefficient.
The solution 3 is interesting for its reactive approach but fails at some point. There is a lot of boilerplate and there is no interest in onSubscribe
and onError
callbacks.
So I found the solution 2 and 4 quite interesting.
But The solution 2 needs to add the LiveData library as dependency where The solution 4 needs to define custom interface because Consumer
functional interface is not available on old SDK.
(EDIT) Woops
EDIT: I think few minutes later using result to attach callback could lead to miss the first out and err lines.
Let's say the su
command runs it immediately in an other thread, we have a trouble here.
So may be a fluent API or builder could be interesting here:
// Run command and update the UI on update
Output output = su(command).onOuput(this::displayOutputs).exec();
// Just get output lines
List<String> out = output.getOut();
So su/sh will no more return an Output
instance but a builder, like a CommandBuilder
instance which could attach listeners (onOutput(Consummer<String>)
) and start the command execution (exec()
).
What do you think about this idea?
Shell initializer
I looked at the standard Java API and there is some methods with boolean as return parameter and exception thrown at the same time in the signature. Usually IOException
and SecurityException
.
Returning an exception means one thing: you should be able to do something about it.
It could be restore your internal state, closing resources, etc… But adding throws Exception
means be prepared to recover from trouble. So what could you do when Initializer
throws an exception? Actually, nothing else than logging it and do the same thing than when it returns false.
This is why I would not add exception in the signature personally. It's the difference between checked exceptions and runtime exception. If something goes very wrong, the developer will raise a runtime exception (like Integer.parseInt("not_a_number")
does) and you could still catch it and log it.
Moreover, throwing the standard Exception
without creating dedicated checked exception, like a ShellInitializationException
does not help to recover because you can not filter the exception type with different catch blocks. So don't even know you have an initialization exception (may be a NullPointerException
or a IOException
, etc…).
Tasks & varargs
Hum, I will have a look at it later this week because it is already late here. I will update this post or add a new one if you reply before.
Conclusion
Building an API is hard.
Nobody will find the right way from the first time but a good exercise could be to write (just) the interface of the API and update your code using it accordingly.
If it looks great and fulfills your needs, share it with your users and get a lot of feedback!
If everyone enjoy it (or at least most of them 😇), it is time to think about implementing it.
But implementation should be just a detail at this point 😅
from libsu.
@PerfectSlayer take a look at my current plan for the new API on branch 2.0
Basically, some new classes are defined:
public abstract static class Result {
public abstract List<String> getOut();
public abstract List<String> getErr();
public abstract int getCode();
}
public interface ResultCallback {
void onResult(Result out);
}
public abstract static class Job {
public abstract Job to(List<String> stdout);
public abstract Job to(List<String> stdout, List<String> stderr);
public abstract Job onResult(ResultCallback cb);
public abstract Result exec();
public abstract void enqueue();
}
And some new low level and high level APIs are introduced:
// Low level API
public abstract Job newJob(String... cmds);
public abstract Job newJob(InputStream in);
// High level API
public static Job su(String... commands);
public static Job sh(String... commands);
public static Job su(InputStream in);
public static Job sh(InputStream in);
So to execute commands synchronously and get STDOUT:
Shell.su(cmds...).exec().getOut();
Execute commands synchronously and output STDOUT to specific List:
Shell.su(cmds...).to(List).exec()
Execute commands asynchronously and don't care about output:
Shell.su(cmds...).enqueue()
Execute commands asynchronously and get output after operation is done:
Shell.su(cmds...).onResult(result -> { /* do something */ }).enqueue()
Execute commands asynchronously, reactively get output, get notified when operation is done:
class ReactiveList extends CallbackList<String> {
ReactiveList(List<String> base) {
super(base);
}
@Override
public void onAddElement(String s) {
/* Do something reactively */
}
}
Shell.su(cmds...)
.to(new ReactiveList(new ArrayList<>()))
.onResult(result -> {
/* Do something to the result that cannot
* be done reactively, e.g. get return code */
result.getCode();
....
})
.enqueue();
For any developer wanting to do some magic with LiveData
or any other stuffs, it is relatively easy to extend CallbackList
or ResultCallback
to fit their own needs.
Do you have any other suggestions to the new API? I think this API is clean and simple but still very flexible.
P.S. I still create a whole ugly compatibility shim for those still using the old APIs, so the transition will be painless from 1.X.X :)
from libsu.
It really looks nicer to use! 👏
I agree this API is clean and simple. If I can, I would propose some improvements:
-
Rename
Job.enqueue()
toJob.submit()
.
enqueue
is more related to Collections framework wheresubmit
is related to Executors framework -
Rename the two
Job.to
methods with different method names.
ExJob.outputTo(List)
andJob.errorTo(List).
So you could handle error without touching output and you could create inputFrom(List) later for the STDIN. -
Replace the
Shell.sh(InputStream)
andShell.su(InputStream)
byJob.inputFrom(InputStream)
.
So you could remove duplicatesh
andsu
methods and STDIN will be handled like STDOUT and STDERR in theJob
class.
So here is the current Job
class:
public abstract static class Job {
public abstract Job to(List<String> stdout);
public abstract Job to(List<String> stdout, List<String> stderr);
public abstract Job onResult(ResultCallback cb);
public abstract Result exec();
public abstract void enqueue();
}
which could becomes:
public abstract static class Job {
public abstract Job inputFrom(List<String> stdin);
public abstract Job inputFrom(InputStream stdin);
public abstract Job outputTo(List<String> stdout);
public abstract Job errorTo(List<String> stderr);
public abstract Job onResult(ResultCallback cb);
public abstract Result exec();
public abstract void submit();
}
Even Job.onResult
could be removed because it will always be used in combination with Job.summit
. So we could overload it:
public abstract static class Job {
// STDIN, STDOUT, STDERR related
public abstract Job inputFrom(List<String> stdin);
public abstract Job inputFrom(InputStream stdin);
public abstract Job outputTo(List<String> stdout);
public abstract Job errorTo(List<String> stderr);
// Execution related
public abstract Result exec();
public abstract void submit();
public abstract void submit(ResultCallback cb);
}
What do you think about this proposals? 😅
Here is the example from your application:
Shell.sh(getResources().openRawResource(R.raw.count))
.to(consoleList)
.onResult(new Shell.ResultCallback() {
@Override
public void onResult(Shell.Result out) {
Log.d(ExampleApp.TAG, "async_script_result");
}
}).enqueue();
which becomes:
Shell.sh()
.inputFrom(getResources().openRawResource(R.raw.count))
.outputTo(consoleList)
.submit(new Shell.ResultCallback() {
@Override
public void onResult(Shell.Result out) {
Log.d(ExampleApp.TAG, "async_script_result");
}
});
One of the main advantage is consistency for the Job
class and we can run interactive commands!
Shell.sh("sed 'commands'")
.inputFrom(getResources().openRawResource(R.raw.somefile))
.outputTo(editedList)
.exec();
P.S.: Good luck with compatibility shim! Let's finish the API2 and I could help you with the shim 😉
from libsu.
Honestly, I don't think a compatibility shim would be necessary. Want to use the latest and greatest - migrate your code. Don't want to - remain at the current version and call it a day.
BTW, I'm already thinking about writing RxJava2 wrapper for your new API
from libsu.
Comments
Also, I personally do not like method names to be too verbose (contradict with common Java code naming conventions LOL), so I named my API very simple, like add and to. Also I would love to leave the to(stdout) and to(stdout, stderr) methods untouched too, because in the case of to(stdout),…
No problem. It is you API. I'm just here to challenge your choices to improve the overall API, not to make you change everything!
To only collect STDERR, just use to(null, List).
Hum, null
as first parameter is not very elegant. Be sure to add the annotation @Nullable
and comment the behavior 😅
API wrappers
About RxJava2 and Architecture components wrappers, do you agree to host own contributions as new modules near to superuser
and example
? It could offer some visibility and makes them legit (once they are stable enough 😉).
(I guess I can handle the Architecture component part 😄)
Question
Result code
Where you do something like this:
Result result = Shell.newJob().add(InputStream).add(cmds....).add(...).exec();
What contains result.getCode()
?
The result of the latest command? Of the first command? What about all intermediate commands, can we get their result codes? Should we have List<Integer> getCodes()
and int getLastCode()
method instead?
Separate APIs
Could you separate the different APIs? The old one (V1) from the new one (V2) and the high-level API (Shell.su()
, Shell.sh
) from the low level API (Job
, etc…).
The IDE can't tell you which one to use if they all are in the same class.
But if you separate them, high level API will be easy to use (specially with static import) and learn (very few methods).
Moreover, having distinct classes for V1 and V2 wont clutter the IDE outline with a lot of deprecated methods. It will be easier to read and understand 👍
from libsu.
API Wrappers
Sure, I would love to host them in a separate module if anyone would love to contribute!
Return Code
The current implementation will only get the return code after everything is done (so technically only the last operation in commands or scripts). One job will only have one single return code. I think this is reasonable, because this is how shell scripts works. If you want to get return code for many operations, split them into different Jobs. This extensively in internal.ShellFile
, you can check out how I do it myself (it is mostly using ShellUtils.fastCmdResult(...)
)
In Magisk Manager, I run multiple commands synchronously in the background thread and check the state for each command if needed.
Also a nice trick I used is storing tons of functions in a shell script, and load them into the shell in the Initializer. Then I can call these functions easily afterwards.
APIs
Currently the only "high level" API is Shell.su(...)
and Shell.sh(...)
. I'm not sure if separating these methods is necessary though. The old API is accessed via Shell.Sync
and Shell.Async
, they are already separated.
from libsu.
@PerfectSlayer, some small adjustments to the API is pushed to 2.0, and all documentation is updated in Shell.java
, please check it out.
This should be really close to final. I would have to push this out so I can refactor Magisk Manager, then finally a stable Magisk v17.0 will be release. This library is on my tight development schedule LOL
from libsu.
2.0.0 just released, I'll leave this issue opened for all API discussions in the future 👍
from libsu.
@topjohnwu I'm using Kotlin instead of Java, so it probably won't be looking good in the single project
from libsu.
@victorlapin What is the issue? If you are making a separate module, it should be fine.
@topjohnwu I planed to make the review yesterday night but I had to drive a friend to the hospital… I already start and some some minor bug in your latest commits. So be ready to merge a PR 😅
Whatever, it is nice to see how the API evolve. Good job! 👏
from libsu.
@topjohnwu Do you have any idea about who is using your library?
It could be interesting to gather their feedback and learn which wrapper should be developed first 😉
from libsu.
I am, for example =)
Also, I've seen feedback from the guy who's developing Swift Backup
from libsu.
My library is inspired by Chainfire's libsuperuser. The biggest difference I'm aiming for is a single root shell that is shared by all operations across the app. Most of the design decision is based on my own personal needs in Magisk Manager and cues from
libsuperuser
.
Both that and to what extent they are compatible might be worth documenting... :)
from libsu.
@Chainfire the "inspiration" part is mostly the su
invocation code. A long time ago I have nearly no idea how to code in Java, so I reached out to libsuperuser
for creating the first few crappy Magisk Managers. However I need excessive su calls in a short time, and at that time I used Shell.SU
in libsuperuser
which is far from ideal for that use case. In the marvelous "How-To-SU" documentation, you've mentioned maintaining a long lived root session would be more suitable. So I spent a bunch of time learning Java and implemented my own solution, and eventually I feel comfortable to release it to the public, which was the initial version of libsu
.
This library has evolved a lot since the initial release, and I can say that most of the code (other than the part to test root shell) is completely written from scratch. I've not dug deep enough into libsuperuser
so I can't say definitely but IMO the 2 libraries are using completely different approaches.
So I would say that the two libraries are completely incompatible LOL, Franco (the developer of Franco Kernel) took quite some effort migrating from libsuperuser
to libsu
as it is far from a "drop-in" replacement.
from libsu.
Right - from some of the wording above it sounded like it might be an easy migrate, which would be good for people to know (as obviously libsuperuser is not in active development). But it seems that is not actually the case :)
I need excessive su calls in a short time, and at that time I used Shell.SU in libsuperuser which is far from ideal for that use case.
That is a curious statement though, the Shell.Interactive class exists exactly for that purpose, with the ability to add commands and read the results both synchronous and asynchronously, either handling the per-line/total input/output for you, or allowing you to Stream it yourself; even lets you decide what parts to run on which threads.
When used correctly it is very powerful, and certainly fit to run hundreds of commands per second through a single session (or multiple, if you'd like). Then again the documentation and examples are pretty lacking, and it seems many new users have had trouble getting it to do exactly what they want it to, especially in multi-threaded setups.
That's all history though. Again I just had the impression it might be a reasonable drop-in replacement, in which case that should definitely be stated somewhere. As it isn't, it doesn't matter :)
from libsu.
So what about implementing a wrapper to libsu
?
I don't know if there is a lot of libsuperuser
users who feel to the need to move away from it as it does not seem maintained anymore but providing libsuperuser
API using a libsu
implementation could be a way to attract some users here. And more users here could lead to a better or longer support of this lib.
I don't know if it worth it?
from libsu.
Actually, I found out Shell.Interactive
much later after I developed libsu
:), maybe if I found it much earlier I won't have to create this library from scratch LOL
from libsu.
Related Issues (20)
- How do I transfer to Android tablet
- Crashing in onInit() HOT 4
- Remove ROOT_MOUNT_MASTER status HOT 1
- How do I simulate Control + C for a process HOT 3
- 使用360加固后无法绑定RootService HOT 7
- Doesn't support targetSdk>31? HOT 6
- LIBSU: java.io.InterruptedIOException HOT 2
- Access MainActivity HOT 5
- Refuses to work even though it has root. HOT 1
- LIBRARY REFUSES to access files. HOT 2
- 建议 HOT 2
- !
- [Security] Repository is vulnerable to MavenGate
- StrictMode violation: reading files from the disk in the main thread
- 希望能动态设置超时 HOT 1
- Apps using libsu do not work on root solutions like APatch. HOT 5
- NoSuchMethodException on Android 5.1.1 HOT 2
- . HOT 1
- Worst Documentation HOT 1
- System resources broken on Samsung too HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from libsu.