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

Adding shell utilities to Blink #281

Closed
wants to merge 181 commits into from
Closed

Conversation

holzschu
Copy link

I suggest an extension to Blink, giving basic shell commands inside the shell (cp, rm, mv...)
This might help user in managing their private keys, or storing files locally.

This is based on GNU coreutils, edited so they compile for iOS and work as functions instead of commands. For each command (e.g. "ls") I rename its main function (e.g. "ls_main"), and call this function from inside MCPSession.m

The full list of commands available:
cd ln ls cp mv rm mkdir rmdir touch pwd
setenv env printenv
id groups realpath uname

For more functionalities, I suggest also compiling VimIOS and creating an App Group, so the two apps can share files. Once this is done, you can edit files in VimIOS and copy/delete them in Blink.

Only a small set of the GNU coreutils are available: some of them don't make sense in a sandboxed environment (kill, nohup, nice, chown), others don't work in a function because of local variables (df), still others send their output to STDOUT_FILENO instead of stdout, which was too complicated to change (cat).

@carloscabanero
Copy link
Member

Just a quick note before reviewing or talking in more detail. The GNU Coreutils are under the GPL3, and unfortunately it is incompatible with Blink - Mosh iOS clause. But maybe we can build the BSD versions?

@holzschu
Copy link
Author

holzschu commented Jun 13, 2017 via email

@holzschu
Copy link
Author

holzschu commented Jun 14, 2017 via email

@carloscabanero
Copy link
Member

This is a tricky one, that is the original 4-clause BSD, which is incompatible with GPLv3. The problem is clause 3. But FreeBSD relicensed all their code to a compatible 3-clause one, including the Core Utils we care about (reason why in their license files they skip the 3). The code is technically the same, do you think we can use that one?

@carloscabanero
Copy link
Member

Thanks! I will prepare a new version for the Raw branch with this included. Don’t know what people will do with it, but interesting to figure out! :)

@goerz
Copy link

goerz commented Jun 15, 2017

What would make this massively useful is to have vim included. I was trying to compile this myself, but I don't think I can, without a paid dev account (because of iCloud etc)

@holzschu
Copy link
Author

I'll look into it, but it's a long shot.
It could work, but I don't think it's going to be easy. Vim needs a terminal that implements libcurses, which is a bit too much for Blink. The alternative would be to persuade the Vim developer to have a joint app group with Blink.

@holzschu
Copy link
Author

Hi @yury,
the latest version of ios_system has wildcard expansion (ls *.txt, rm *.log). It is quite useful. You probably want that.

@yury
Copy link
Collaborator

yury commented Apr 27, 2018

Hi @holzschu,

Sure! that would be great. It will be v2.1 release? Or it is better to fork ios_system? What do you think?

yury added a commit to yury/blink that referenced this pull request Apr 27, 2018
@yury
Copy link
Collaborator

yury commented Apr 27, 2018

@holzschu, I also pass err stream to ios_system. That way curl | grep produce a little better output on the screen

@holzschu
Copy link
Author

Hi @yury,

  • wildcard expansion might actually be already in 2.0. Does ls *.txt work?
  • I would prefer not to have a fork (unless it's useful), but ios_system as a submodule would definitely work.
  • And definitely yes about err stream.

@yury
Copy link
Collaborator

yury commented Apr 27, 2018

Hi @holzschu,

  • Yep, expansion working, so we are very good :)
  • I'm also not want to fork.
  • Ok.

@carloscabanero
Copy link
Member

One more thing @holzschu, the last Blink version works with descriptors instead of streams, so we don't need to do fprintf all the time, etc... This may be useful and easier to port some of the tools and keep them in sync.

@holzschu
Copy link
Author

I don't see the change to descriptors yet in the main branch. The switch from streams to descriptors will require some changes in ios_system and its API. With the current architecture, it should be relatively easy (but not instantaneous).

@carloscabanero
Copy link
Member

Blink will continue to support both,. Just mentioned that in case it helps you with maintenance as usually this apps expect descriptors and not streams.

@yury
Copy link
Collaborator

yury commented Apr 28, 2018

Hi @holzschu,

We plan to migrate to new folders structure:

~/history.txt -> ~/.blink_history # or ~/.blink/history.txt
~/known_hosts -> ~/.ssh/known_hosts
~/defaults -> ~/.blink/defaults
~/hosts -> ~/.blink/hosts
~/keys -> ~/.blink/keys # or ~/.ssh/<keys>

Quick search showed that you do checks if files exists and fallback to known defaults in curl_ios_static/src/tool_operate.c

But maybe I missed something?

@holzschu
Copy link
Author

Hi @yury,
Thanks for the heads up.
For known_hosts, yes, I search for it in ~/Documents/known_hosts and then in ~/Documents/.ssh/known_hosts, so the change should not affect ios_system.

I don't think I'm accessing the other ones.

@yury
Copy link
Collaborator

yury commented Apr 29, 2018

Ok, bad news...

Workaround on setlocale actually masks the problem. It doesn't crash, but mosh session resets its utf-8 locale somehow.

Scenario for replicating is more complex now.

  1. first tab - mosh to server. Run htop or type any non ascii chars. - every thing is ok.
  2. open second tab. Type ls -la.
  3. switch to first tab. Run htop - will get bad screen. all non ascii chars will be replaced with <?> (On latest rn branch. On TF htop will behave strange)

Update:

I think that is our problem:
ls.c
screen shot 2018-04-29 at 6 25 58 pm

Update:
Good news!
Yep, that was our problem.

Update:
ls -la loose color after I commented setlocale in it.

@holzschu
Copy link
Author

Hi @yury,
I remember that setlocale depends on files that are outside the sandbox, and so it often returns NULL. Calling it has an impact, but checking the return value might be a bad idea. What happens if you comment lines 99 to 110 in mosh/src/util/locale_util.cc? (it might still crash in iosclient.cc, but why?)

set locale(LC_ALL,""), which is what everyone is calling, is supposed to get the locale value from the environment. But we didn't set it.

@holzschu
Copy link
Author

Hi @yury,
I've been testing things .
set locale(LC_ALL, "") anywhere except in libmosh returns "C". Which is not the right answer (the terminal has UTF-8 abilities), but at least it doesn't crash.
set locale(LC_ALL, "") inside libmosh returns NULL. The question is: why only inside libmosh? Is there anything different with the terminal?

According to the man page, setlocale tries to access /usr/share/locale/locale/category and /usr/local/share/locale/locale/category, which don't exist in iOS sandboxed environment. One possibility would be to define $PATH_LOCALE and provide locale descriptions in $PATH_LOCALE/locale/category.

@yury
Copy link
Collaborator

yury commented Apr 30, 2018

hi @holzschu,

About commenting setlocale in mosh. We did that at first. It didn't crash, but utf8 sequences are corrupted.

About set PATH_LOCALE - yep, PATH_LOCALE is set in mosh ios client. What I did to prevent crash, I also set PATH_LOCALE in app delegate start. It stops crashing, but ls -la changes it concurent scenario (with tabs).

@holzschu
Copy link
Author

Hi @yury,
I'm curious: what do you set PATH_LOCALE to?

@yury
Copy link
Collaborator

yury commented Apr 30, 2018

Hi @holzschu,

https://github.com/yury/blink/blob/rn/Blink/AppDelegate.m#L59

  NSString *locales_path = [mainBundle pathForResource:@"locales" ofType:@"bundle"];
  setenv("PATH_LOCALE", locales_path.UTF8String, forceOverwrite);

@yury
Copy link
Collaborator

yury commented Apr 30, 2018

yep, it is only LC_CTYPE in the bundle

@holzschu
Copy link
Author

Hi @yury,
OK, thanks.
I got the latest version, plus a version of ios_system where ls.c calls set locale(). It seems to work (I don't see any differences on ls -la, for example). For more consistency, you could setenv LC_CTYPE to UTF-8 in AppDelegate.m, so every command uses UTF-8.

@yury
Copy link
Collaborator

yury commented Apr 30, 2018

Hi @holzschu,

Here the short video what I get.

@holzschu
Copy link
Author

Hi @yury,
I'm going to install htop to reproduce. Right now, I have files named with emojis in the first terminal (running mosh) and ls -la in the second terminal.

@holzschu
Copy link
Author

Quick question: does adding setenv("LC_CTYPE", "UTF-8", 1); on line 61 of AppDelegate.m change anything?

@yury
Copy link
Collaborator

yury commented Apr 30, 2018

actually not.

@yury
Copy link
Collaborator

yury commented Apr 30, 2018

oh, sorry!

It works!

yury added a commit to yury/blink that referenced this pull request Apr 30, 2018
@yury
Copy link
Collaborator

yury commented Apr 30, 2018

@holzschu thanks a lot! Testing...

@carloscabanero
Copy link
Member

Alright, it has been quite a ride but we could finally put all the pieces together!!

Thanks a lot @holzschu for such a great work. We will continue contributing to tools on your repo 👍

@holzschu
Copy link
Author

That is both awesome and overwhelming. It's awesome that you guys continue contributing to the tools!

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

Successfully merging this pull request may close these issues.

None yet

6 participants