Bug 4174 - (int-107770) shell history file is overwritten after every command
(int-107770)
: shell history file is overwritten after every command
Status: CLOSED FIXED
Product: Core
Busybox
: 5.0-alpha
: All Maemo
: Low normal (vote)
: 5.0-beta
Assigned To: Turo Janka
: busybox-bugs
: https://bugs.busybox.net/show_bug.cgi...
: community-diablo, patch, upstream
:
:
  Show dependency tree
 
Reported: 2009-03-03 04:56 UTC by Lucas Maneos
Modified: 2009-10-22 07:57 UTC (History)
3 users (show)

See Also:


Attachments
Patch for Diablo bb ash to load/save history only at start/exit time (2.79 KB, patch)
2009-03-23 22:03 UTC, Lucas Maneos
Details
same for Fremantle (2.81 KB, patch)
2009-03-23 22:03 UTC, Lucas Maneos
Details
same for current upstream (2.80 KB, patch)
2009-03-23 22:04 UTC, Lucas Maneos
Details
Updated patch (2.74 KB, patch)
2009-04-02 11:57 UTC, Andre Klapper
Details


Note

You need to log in before you can comment on or make changes to this bug.


Description Lucas Maneos (reporter) 2009-03-03 04:56:14 UTC
SOFTWARE VERSION:

5.2008.43-7

STEPS TO REPRODUCE THE PROBLEM:

1. Establish a shell session (osso-xterm or ssh, user or root, doesn't matter).
2. Run "ls -l .ash_history" repeatedly.

EXPECTED OUTCOME:

History file is only updated when shell exits.

ACTUAL OUTCOME:

History file is re-written after every command entered
(libbb/linedit.c:remember_in_history).  This is causing unnecessary wear on the
flash and wastes a bit of power.  There are also side-effects with the way
loading the history is handled (will post another bug shortly).

REPRODUCIBILITY:

Always.

EXTRA SOFTWARE INSTALLED:

Nothing relevant.

OTHER COMMENTS:

There's no way to disable history saving as in most shells by setting an
environment variable, and unsetting HISTFILE doesn't change anything.

5.0-alpha code doesn't look better.  IMHO it's probably worth building without
ENABLE_FEATURE_EDITING_SAVEHISTORY. 

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-GB; rv:1.9.0.6)
Gecko/2009020911 Ubuntu/8.10 (intrepid) Firefox/3.0.6
Comment 1 Andre Klapper maemo.org 2009-03-03 11:51:23 UTC
Comparing with upstream the only relevant patch that *might* have fixed this is
http://sources.busybox.net/index.py/branches/busybox_1_13_stable/libbb/lineedit.c?sortby=file&view=diff&r1=23529&r2=23530
, while Fremantle still ships version 1.10.
If you find some time to take a quick look I'd appreciate it. :)

If that patch does not cover the issue I will forward this upstream.
(As far as I know, Nokia does not change the Busybox code so apart from
configuration issues it's always an upstream problem.)
Comment 2 Lucas Maneos (reporter) 2009-03-03 22:08:13 UTC
(In reply to comment #1)
> Comparing with upstream the only relevant patch that *might* have fixed this is
> http://sources.busybox.net/index.py/branches/busybox_1_13_stable/libbb/lineedit.c?sortby=file&view=diff&r1=23529&r2=23530

This only deals with the in-memory history.  The logic of when to save/load 
still seems the same currently in both trunk (r23938) and the
busybox_1_13_stable branch (r24175).

> (As far as I know, Nokia does not change the Busybox code so apart from
> configuration issues it's always an upstream problem.)

Well, there's https://garage.maemo.org/projects/busybox4maemo now :-)
Comment 3 Andre Klapper maemo.org 2009-03-05 14:49:38 UTC
Thanks for the investigation!

Upstreamed as https://bugs.busybox.net/show_bug.cgi?id=149 .
Comment 4 Andre Klapper maemo.org 2009-03-23 10:50:28 UTC
Patch available in the upstream bug. Testing welcome.
Comment 5 Lucas Maneos (reporter) 2009-03-23 20:24:12 UTC
Both patches attached to https://bugs.busybox.net/show_bug.cgi?id=185 deal with
the load_history issue (bug 4175), but history is still saved after each
interactive command.

POSIX/SUS are vague as usual on the subject, but IMHO ash should just load the
history once at startup and save it once at exit like most (all?) *nix shells
do.   I might have a stab at this myself.
Comment 6 Lucas Maneos (reporter) 2009-03-23 22:03:25 UTC
Created an attachment (id=1145) [details]
Patch for Diablo bb ash to load/save history only at start/exit time

While looking at this I came across another related bug where child shells also
overwrite the history file when they exit.  So for example:

$ echo foo | read bar

would cause the history to be written three times in total.
Comment 7 Lucas Maneos (reporter) 2009-03-23 22:03:50 UTC
Created an attachment (id=1146) [details]
same for Fremantle
Comment 8 Lucas Maneos (reporter) 2009-03-23 22:04:37 UTC
Created an attachment (id=1147) [details]
same for current upstream
Comment 9 Andre Klapper maemo.org 2009-03-24 17:32:30 UTC
Turo, can you handle this directly here, or is an internal ticket required?
Patch available (comment 7).
Comment 10 Andre Klapper maemo.org 2009-03-26 15:30:13 UTC
Upstream feedback:

------- Comment  #6 From Denys Vlasenko  2009-03-26 13:18:56 UTC -------
Those patches are not safe wrt concurrent writes.
Try running two ash shells and give commands in each. Check history file, only
commands from last shell are there.
Comment 11 Lucas Maneos (reporter) 2009-03-26 18:50:59 UTC
Denys is right,  this is an issue when .ash_history is < MAX_HISTORY lines
long.
Comment 12 Andre Klapper maemo.org 2009-04-02 11:57:35 UTC
Created an attachment (id=1162) [details]
Updated patch

The patch that was actually used for fixing this in Fremantle.
Comment 13 Andre Klapper maemo.org 2009-04-02 11:58:11 UTC
This has been fixed for Fremantle.
Comment 14 Lucas Maneos (reporter) 2009-04-02 13:29:57 UTC
Oops, sorry for missing the hi initialisation (that's what you get for starting
at 1.13 and working backwards).

This is good enough for me (and also closes bug 4175).  FYI there is an updated
patch awaiting comments in the upstream bug that takes care of the feedback in
comment 10, but I probably need to redo it for trunk (1.14.0.svn) where bug
4175 is fixed but this one is still present.
Comment 15 Andre Klapper maemo.org 2009-04-28 15:35:18 UTC
Setting Target Milestone to Fremantle SDK beta.
Comment 16 Lucas Maneos (reporter) 2009-10-22 07:57:42 UTC
Marking patches of interest to Diablo (Maemo4) community updates, please excuse
the noise.