Active Topics
| The Following 9 Users Say Thank You to iDont For This Useful Post: | ||
 |
|
2011-07-07
, 08:15
|
|
Moderator |
Posts: 7,109 |
Thanked: 8,820 times |
Joined on Oct 2007
@ Vancouver, BC, Canada
|
#82
|
Hi iDont, is it possible to fix Bug 5317? I thought for sure that your Busybox 1.18 would fix it, but it still doesn't save my history if I press on the X instead of typing "exit". I even rebooted just to be sure!
EDIT: maybe fix Bug 7014 while you're in there?
EDIT AGAIN: Even just applying this workaround would be nice...
But since you're patching the source anyway...
Last edited by qole; 2011-07-07 at 08:24.
EDIT: maybe fix Bug 7014 while you're in there?
EDIT AGAIN: Even just applying this workaround would be nice...
It really should install a signal handler that exits the shell cleanly, but a simpler
workaround without patching the source would be something like:
Code:$ echo "trap exit SIGHUP SIGINT SIGTERM" >> /home/user/.profile
__________________
qole.org --- twitter --- Easy Debian wiki page
Please don't send me a private message, post to the appropriate thread.
Thank you all for your donations!
qole.org --- twitter --- Easy Debian wiki page
Please don't send me a private message, post to the appropriate thread.
Thank you all for your donations!
Last edited by qole; 2011-07-07 at 08:24.
| The Following 4 Users Say Thank You to qole For This Useful Post: | ||
|
|
2011-07-07
, 13:01
|
|
Posts: 268 |
Thanked: 1,053 times |
Joined on May 2010
@ The Netherlands
|
#83
|
Originally Posted by qole
This is one is a little bit complicated. Here's some background information on it:
Hi iDont, is it possible to fix Bug 5317? I thought for sure that your Busybox 1.18 would fix it, but it still doesn't save my history if I press on the X instead of typing "exit". I even rebooted just to be sure!
Upstream BusyBox handles shell history perfectly well. Even Maemo's stock BusyBox handled shell history well, until bug 4174 appeared, that is. As you can read there, it changes BusyBox' shell history saving behaviour from writing out the history directly after executing a command, to saving the history upon shell exit. This, however, introduces new problems (e.g. bug 5317 you've mentioned).
It's a tradeoff we have to make here. Either we keep the proper upstream shell history handling and have a (little?) bit more wear on the eMMC and a resulting (small?) increase of power usage, or we go Nokia's way of saving shell history only upon shell exit, but then we'll have problems with concurrent shells etcetera.
Apparantly, Nokia has left out their shell history patch in Harmatten, thus the bug you've mentioned is 'fixed' for Harmatten. Bug 4174, however, is reintroduced there AFAIK.
I'd like to stick with what Nokia has decided to do for Maemo 5 in the first place, which is including the shell history patch. Some averaged numbers (/guesses) on how much wear upstream shell history handling actually creates on our eMMC's would be great, but I guess those are pretty hard to quantify.
However, to give you the freedom to choose yourself, I've compiled the latest busybox-power without shell-hist.patch; click here to find it. That build should suit your shell history needs
. I'll keep this alternative busybox-power version up-to-date, but I currently don't have any plans to push it to the repositories.Update: Scrap the previous line. Since busybox-power 1.18.5power3, shell-hist.patch has been dropped in favor of a new patch. This new patch fixes all mentioned problems, so there's no need to maintain the special busybox-power build. I can, however, create a new version of it without the new patch if there's any demand for it (drop me a PM).
Originally Posted by qole
Patching the code won't do, unfortunately. It's either saving the history at exit, or saving it after each line; both with their own pros and cons. That, or we need some code to keep the history from multiple terminal instances in memory somehow, and write it out when the last terminal instance closes. However, that's a bit out of my league I'm afraid
EDIT AGAIN: Even just applying this workaround would be nice...
[..] But since you're patching the source anyway...
Anyhow, thanks for the workaround, I seem to have missed it in the bugreport itself earlier. I'll certainly include it in the next busybox-power release.
Originally Posted by qole
Already fixed
EDIT: maybe fix Bug 7014 while you're in there?
:
Originally Posted by debian/config/config.maemo
CONFIG_FEATURE_EDITING_HISTORY=100
Last edited by iDont; 2011-07-31 at 14:43.
| The Following 2 Users Say Thank You to iDont For This Useful Post: | ||
|
|
2011-07-07
, 14:28
|
|
Posts: 2,225 |
Thanked: 3,822 times |
Joined on Jun 2010
@ Florida
|
#84
|
Hmmm... Personally I'm divided. On the one hand I'm used to the shell being slightly 'wrong' in behavior since my main shell experience soure is the N900. On the other hand, if Nokia chose to go with "standard" behavior for Maemo 6 (I'm not calling Harmattan 'MeeGo' in casual conversation), that must mean even the Nokia folk think it's better to keep standard shell behavior. On the other hand it's possible they changed up something else, and as a result stock shell history saving behavior is only better in combination...
Does busybox normally keep track of what *time* every command is entered? The easiest way I can imagine to get the best of both worlds if it doesn't already keep track of time-stamps is to write to history as is 'normal' upsteam, by command, but write to a file in /tmp, thus keeping the actual history for all currently running shells in RAM and not actually written. Then you can write all the lines in the /tmp shell history into the existing shell history file, while clearing the /tmp-located file, when any of the shells are closed/exit-ed.
Honestly though, I think in the long term if we don't patch the shell to work as desired without the extra flash writes, the better choice is the default upstream behavior, not current N900 behavior (although I'm sticking with what's in the repository for now, since I'm used to it enough to not personally mind it). I'm confident if we get ourselves motivated enough, we could figure out how to do the former though (either the way I suggested above, using a /tmp history file as a middle-man, or through more low-level code-voodoo, which would be more efficient, but meh), instead of having to resort to chosing which one gets the repository spot. Which, actually, is already decently nice - thank you iDont for committing to maintaining both.
Does busybox normally keep track of what *time* every command is entered? The easiest way I can imagine to get the best of both worlds if it doesn't already keep track of time-stamps is to write to history as is 'normal' upsteam, by command, but write to a file in /tmp, thus keeping the actual history for all currently running shells in RAM and not actually written. Then you can write all the lines in the /tmp shell history into the existing shell history file, while clearing the /tmp-located file, when any of the shells are closed/exit-ed.
Honestly though, I think in the long term if we don't patch the shell to work as desired without the extra flash writes, the better choice is the default upstream behavior, not current N900 behavior (although I'm sticking with what's in the repository for now, since I'm used to it enough to not personally mind it). I'm confident if we get ourselves motivated enough, we could figure out how to do the former though (either the way I suggested above, using a /tmp history file as a middle-man, or through more low-level code-voodoo, which would be more efficient, but meh), instead of having to resort to chosing which one gets the repository spot. Which, actually, is already decently nice - thank you iDont for committing to maintaining both.
|
|
2011-07-07
, 22:55
|
|
Moderator |
Posts: 7,109 |
Thanked: 8,820 times |
Joined on Oct 2007
@ Vancouver, BC, Canada
|
#85
|
I think I'd be satisfied if iDont just added the workaround to the post-install script. It looks like you only need to check to see if there's a trap exit line in /home/user/.profile and if not, drop in
That will keep the save-on-exit patch and workaround the missing save-on-SIGHUP code.
Code:
trap exit SIGHUP
__________________
qole.org --- twitter --- Easy Debian wiki page
Please don't send me a private message, post to the appropriate thread.
Thank you all for your donations!
qole.org --- twitter --- Easy Debian wiki page
Please don't send me a private message, post to the appropriate thread.
Thank you all for your donations!
|
|
2011-07-08
, 00:32
|
|
Posts: 2,802 |
Thanked: 4,491 times |
Joined on Nov 2007
|
#86
|
Originally Posted by iDont
Feel free to blame me, I should have argued harded on the upstream bugs but I kinda lost interest :-(
Even Maemo's stock BusyBox handled shell history well, until bug 4174 appeared, that is. As you can read there, it changes BusyBox' shell history saving behaviour from writing out the history directly after executing a command, to saving the history upon shell exit. This, however, introduces new problems (e.g. bug 5317 you've mentioned).
It's a tradeoff we have to make here. Either we keep the proper upstream shell history handling and have a (little?) bit more wear on the eMMC and a resulting (small?) increase of power usage
or we go Nokia's way of saving shell history only upon shell exit, but then we'll have problems with concurrent shells etcetera.
Some averaged numbers (/guesses) on how much wear upstream shell history handling actually creates on our eMMC's would be great, but I guess those are pretty hard to quantify.
Originally Posted by Mentalist Traceur
There's no standard sadly (POSIX typically avoids the issue), but busybox's handling of history is rather unique AFAIK. Other POSIXy shells like bash or ksh keep their history in memory and write it out at exit (they pretty much have to since they implement fc and that will be extremely broken with mixed histories).
On the one hand I'm used to the shell being slightly 'wrong' in behavior since my main shell experience soure is the N900. On the other hand, if Nokia chose to go with "standard" behavior for Maemo 6 (I'm not calling Harmattan 'MeeGo' in casual conversation), that must mean even the Nokia folk think it's better to keep standard shell behavior.
Originally Posted by qole
Quick'n'dirty version in the Diablo CSSU:
I think I'd be satisfied if iDont just added the workaround to the post-install script.
Code:
line="trap exit SIGHUP" if ! grep -F -q "$line" /etc/profile; then echo "$line" >> /etc/profile fi
| The Following 4 Users Say Thank You to lma For This Useful Post: | ||
|
|
2011-07-08
, 01:59
|
|
Posts: 2,225 |
Thanked: 3,822 times |
Joined on Jun 2010
@ Florida
|
#87
|
Originally Posted by lma
Fair enough. But I still think my suggestion is realistically feasible... Find whatever place in the source the paths to the history files are kept in the "original" source (where it writes history upon every enter), and edit those paths to place "/tmp" in front of them. And then add the code from the Nokia patch that makes them write the history out at exit (but don't override the former like the patch would), and change that code to pull data from the "/tmp/[whatever]" and put that into the standard [whatever] ash history upon every shell close. That way history accumulates from all the concurrent shells in the order the commands happen across the board, and is saved on every shell close (with the above patch/workaround by you guys, even when you hit the above X.)
There's no standard sadly (POSIX typically avoids the issue), but busybox's handling of history is rather unique AFAIK. Other POSIXy shells like bash or ksh keep their history in memory and write it out at exit (they pretty much have to since they implement fc and that will be extremely broken with mixed histories)
Busybox is coded in C, right? iDont, where do you keep the latest source for this package (my apologies if you mentioned it in this thread already. If you have just say so and I'll go look)? If I get the time I'll take a look at the source see if I can improvise a shitty patch to do the above. (I probably can't, but I can always get my own hopes up before realizing I can't do it.)
- Edit -
NVM - it's right there in the second post.
Last edited by Mentalist Traceur; 2011-07-08 at 03:17.
|
|
2011-07-08
, 18:25
|
|
Posts: 268 |
Thanked: 1,053 times |
Joined on May 2010
@ The Netherlands
|
#88
|
Originally Posted by qole
It's living in GIT already: https://garage.maemo.org/plugins/ggi...=busybox-power.
I think I'd be satisfied if iDont just added the workaround to the post-install script. It looks like you only need to check to see if there's a trap exit line in /home/user/.profile and if not, drop in
Code:
That will keep the save-on-exit patch and workaround the missing save-on-SIGHUP code.Code:trap exit SIGHUP
Why wouldn't you want to trap SIGINT and SIGTERM too, by the way? I'm asking this because I went with the original workaround, which did include those signals as well.
Originally Posted by lma
Yeah, I pretty much thought so. Still, I would love to see something more suitable for our devices than one of the two currently available history handling extremes, which brings me to:
It depends on how you use it obviously. We're talking about one eraseblock cycle every time you press enter which I guess adds up to a not-insignificant amount, but probably not enough to kill your flash by itself within the device's expected lifetime.
Originally Posted by Mentalist Traceur
You're right about this being realistically feasible. Even more, I gave your idea a shot too. Attached to this post is a patch that basically does what you've stated, which is saving the history using upstream's code, but to a temporary location. Busybox will copy the history file back to the correct user's home directory upon shell exit.
Fair enough. But I still think my suggestion is realistically feasible... Find whatever place in the source the paths to the history files are kept in the "original" source (where it writes history upon every enter), and edit those paths to place "/tmp" in front of them. And then add the code from the Nokia patch that makes them write the history out at exit (but don't override the former like the patch would), and change that code to pull data from the "/tmp/[whatever]" and put that into the standard [whatever] ash history upon every shell close. That way history accumulates from all the concurrent shells in the order the commands happen across the board, and is saved on every shell close (with the above patch/workaround by you guys, even when you hit the above X.)
It seems to be working great here, i.e. not different from upstream BusyBox. I've tried out various combinations of multiple shell instances with multiple users (root and user), all without error.
To test out the patch: place it in busybox-power's debian/patches folder, add the filename of the patch to debian/patches/series, remove shell-hist.patch from the same file (important), and finally add:
Code:
CONFIG_ASH_HIST_BUFFER=y CONFIG_ASH_HIST_BUFFER_PATH="/tmp/.shellhistory"
The temporary location for .ash_history can be changed if you want. With the above config entries, the buffer history file will be written to /tmp/.shellhistory/[subdir based on user]/.ash_history
This patch should remove any problems introduced by Nokia's patch (e.g. with concurrent shells), and avoid wear on the eMMC.
People with more advanced C knowledge, please review the patch and notify me of any mistakes/enhancements. I'm sure some things could be done better than how I did them. Also, I'm always a little bit hesitant to push 'invasive' code into production environments, so positive feedback is greatly appreciated too.
Edit: this patch is obsolete. A newer version of it has been merged upstream.
Last edited by iDont; 2011-09-11 at 13:06.
| The Following 2 Users Say Thank You to iDont For This Useful Post: | ||
|
|
2011-07-08
, 23:05
|
|
Posts: 2,802 |
Thanked: 4,491 times |
Joined on Nov 2007
|
#89
|
Originally Posted by iDont
Those problems occur without the patch. Feel free to correct me if I'm missing something, but AFAIK the only problem the patch introduces is not saving history when killed.
This patch should remove any problems introduced by Nokia's patch (e.g. with concurrent shells)
please review the patch
- In histcopy(), check the result of the input fopen and bail out before clobbering the output file.
- I don't agree that if the user doesn't have a ~/.ash_history, or have an empty one, or /tmp is full etc they should be locked out. Return instead of exit?
- The tmppath directory shouldn't be created with mode 0777 (predictable filenames in world-writable directories are a bit exploitable), 0700 will do.
- One could trigger a buffer overrun by feeding the shell a long $HOME (though that doesn't give any additional privileges). Using concat_path_file for tmppath would be safer than malloc/strncat here.
| The Following 4 Users Say Thank You to lma For This Useful Post: | ||
 |
|
2011-07-09
, 00:29
|
|
Posts: 323 |
Thanked: 189 times |
Joined on Oct 2010
@ Brazil
|
#90
|
Originally Posted by qole
May I ask to make it optional? I mean, I edit the .ash_history and put all the things I most use like change frequency or open new windows of filebox & and ping and powertop, and I, for one, prefer that it do not remember what I have done last session unless I hit exit...
I think I'd be satisfied if iDont just added the workaround to the post-install script. It looks like you only need to check to see if there's a trap exit line in /home/user/.profile and if not, drop in
That will keep the save-on-exit patch and workaround the missing save-on-SIGHUP code.Code:trap exit SIGHUP
BUT I may be happy just with editing my .profile to remove the trap sighup as looks like everybody seems to prefer that it remember and I'm the only one that do not like it much. On the PC I think it's essential, but on my phone, not much.
Thanks!
__________________
Love and Goodness are not a property. Are not a franchising. They are present in each one of us, and must be cultivated with KNOWLEDGE.
Love and Goodness are not a property. Are not a franchising. They are present in each one of us, and must be cultivated with KNOWLEDGE.
Last edited by Temporal; 2011-07-09 at 00:33.
It's a shame though that each version of any package has to be voted up separately (i.e. I would love to get 1.18.5power2 in Maemo's extras; it supports installation in the SDK and has a less scary warning during first-time installation). Ah well, let's get 1.18.5power1 in extras first, as it already had 6 thumbs up currently.
About the new problem. I actually do think this is something very specific. If I can trust these stats, then only two installations I'm aware of have failed out of >10000. One of those has been confirmed to be not caused by busybox-power failing, and the other is not confirmed yet. I haven't received a reply from awett yet, so I can't comment any further on the problem he encountered.
Anyway, for what I know, there are currently no pit-falls in the (un)installation scripts. Be assured, a lot of care has been put into them to prevent broken installations
--
On other news: I just received an approval for a garage page for busybox-power, so a proper bugtracker and more are on their way
Also, I want to check if there is any interest in getting busybox-power working on the N8xx/N770. If you are interested in this, leave a message here (I'll move this message to the first/second post later, so it will not go unnoticed)
Last edited by iDont; 2011-07-05 at 18:25. Reason: Fix grammatical error