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

Merging history does not work #3496

Closed
edouard-lopez opened this issue Oct 26, 2016 · 23 comments
Closed

Merging history does not work #3496

edouard-lopez opened this issue Oct 26, 2016 · 23 comments
Labels
bug Something that's not working as intended
Milestone

Comments

@edouard-lopez
Copy link
Contributor

edouard-lopez commented Oct 26, 2016

related: History synchronization

Env

fish version installed (fish --version):

$ fish --version
fish, version 2.3.1

OS:

$ lsb_release -a
Distributor ID: LinuxMint
Description:    Linux Mint 18 Sarah
Release:        18
Codename:       sarah

Terminal

yakuake -v
Qt: 4.8.7
KDE Development Platform: 4.14.16
Yakuake: 2.9.9

tmux

$ tmux -V
tmux 2.1

config.fish

See my dotfiles repo for my configh.fish

Reproduction steps

  1. open tmux ;
  2. add a new pane (i.e. new fish process) ;
  3. type echo pom in fish-1 instance ;
  4. go to fish-2, look-up in history for echo pom.

Results

No trace of echo pom command even when running history --merge on both fish instance

update:

  1. add config.fish link.
@faho
Copy link
Member

faho commented Oct 26, 2016

This works for me, with and without tmux.

Is tmux actually important here? If you use two yakuake tabs instead, does it work?

Just to confirm, you did execute echo pom, not just type it into the commandline, right?

@faho
Copy link
Member

faho commented Oct 26, 2016

I've looked through your config files and cannot find anything related to this (though I don't know your plugins). I could critique them if you want me to, though.

Can you try without configuration? sh -c 'env HOME=$(mktemp -d) fish', like the issue template suggests.

@spinningarrow
Copy link

Works for me as well, after doing history --merge.

@krader1961
Copy link
Contributor

It's expected that the command you ran in session fish-1 isn't in the history of session fish-2 until you run history --merge. But once you do that it should be visible. This has worked for me as far back as a year ago when I started using fish.

P.S., With the soon to be available 2.4.0 release this becomes history merge (it's no longer a flag although the flag form is still accepted).

@faho
Copy link
Member

faho commented Oct 26, 2016

Oh, also you might not know that if a history element is taken as a suggestion, it won't be visible when searching the history - see #405.

So if you get echo pom as a suggestion, it was correctly merged.

@edouard-lopez
Copy link
Contributor Author

@faho same behavior when

  1. open a new tmux window ;
  2. run sh -c 'env HOME=$(mktemp -d) fish' ;
  3. echo pomme ;
  4. open a new pane ;
  5. run sh -c 'env HOME=$(mktemp -d) fish' ;
  6. run history --merge.

@edouard-lopez
Copy link
Contributor Author

Maybe I got a permissions issue, where is the fish history file ?

@faho
Copy link
Member

faho commented Oct 27, 2016

same behavior when

open a new tmux window ;

Again, is tmux actually relevant here? Can you reproduce this without tmux, just with multiple terminals or tabs in yakuake/konsole?

And can you confirm that you also do not see echo pomme as a suggestion (the greyed-out thing that appears when you type stuff)?

Maybe I got a permissions issue, where is the fish history file ?

~/.local/share/fish/fish_history (or $XDG_DATA_HOME/fish/fish_history, if that variable is defined).

@krader1961
Copy link
Contributor

The problem is because you're defining a unique $HOME in for each shell. That means each shell has a unique idea of what ~/ expands to. Which in turn means each shell will have its own distinct history file not shared with any other fish shell on your system. Why are you doing that?

@faho
Copy link
Member

faho commented Oct 27, 2016

The problem is because you're defining a unique $HOME in for each shell. That means each shell has a unique idea of what ~/ expands to.

Sorry, that was my fault. I don't think @edouard-lopez was doing that before, though, or am I missing something?

@edouard-lopez
Copy link
Contributor Author

@faho

Tmux

Again, is tmux actually relevant here? Can you reproduce this without tmux, just with multiple terminals or tabs in yakuake/konsole?

Tried without tmux and multiple tabs, get same result history --merge doesn't work.

Suggestion

And can you confirm that you also do not see echo pomme as a suggestion (the greyed-out thing that appears when you type stuff)?

No suggestion either

Permissions

Maybe I got a permissions issue, where is the fish history file ?

 -rw------- 1 root root 8,0K Oct 26 18:29 /home/ed8/.local/share/fish/fish_history

But my fish process belong to my user ed8.

$XDG_DATA_HOME is unset/empty.

@edouard-lopez
Copy link
Contributor Author

@krader1961 What do you mean define $HOME for each shell? Is it in my config? how do I fix that?

@faho
Copy link
Member

faho commented Oct 29, 2016

-rw------- 1 root root 8,0K Oct 26 18:29 /home/ed8/.local/share/fish/fish_history

But my fish process belong to my user ed8.

That seems like #2335, which was supposed to be fixed in 2.3.0. When you fix the permissions and do stuff like run fish in sudo -Es, do the permissions stay correct?

What do you mean define $HOME for each shell? Is it in my config? how do I fix that?

The env HOME=$(mktemp -d) will set $HOME to a randomly generated directory, which will of course be different for two sessions launched this way. So our standard directions for starting fish without configuration will also lead to starting fish without history. I can't find anything in your dotfiles about setting $HOME, so those should be okay.

@faho faho changed the title Unified history when using tmux Merging history does not work Oct 29, 2016
@faho
Copy link
Member

faho commented Oct 29, 2016

When you fix the permissions and do stuff like run fish in sudo -Es, do the permissions stay correct?

Just to be clear, you could also remove the file, but then you need to ensure that it is created first with the correct permissions, ie. run some commands and ideally history --save. We haven't gotten the case where someone becomes root as the very first thing, keeping $HOME, right and I'm not sure how we could.

@edouard-lopez
Copy link
Contributor Author

I just realize my restore script is run as root and probably installed fish as root…

Solution

  1. removing the file ;
  2. running history --save ;
  3. changing directory permission ;
  4. restarting terminal

@cjthompson
Copy link

cjthompson commented Nov 18, 2016

I'm having this exactly same issue with the last git version. I opened two fish instances using a temp dir for home, and when I do a history --merge, recent commands disappear, even ones from the local history.

Reproduce:

  1. In instance 1, echo 1
  2. In instance 2, history --merge; and echo 2
  3. In instance 1, history --merge; and echo 3
  4. In instance 2, history --merge; and echo 4

Results:
Instance 1 history:

history
history --merge
history --merge
history --merge
echo 1

Instance 2 history

history
echo 4
history --merge
history --merge
history --merge
echo 1
history

Basically, it looks like the most recently executed command doesn't merge, but sometimes its even several of the last commands.

I'm looking at fish_history between commands and even after history --save there are commands that show up in history but do not exist in fish_history. Thus, when history --merge is run, commands that are in history but not in fish_history file aren't merge and disappear.

What would cause history --save to not save certain commands to the fish_history file?

@krader1961
Copy link
Contributor

What you're seeing, @cjthompson, is a different problem. Based on my testing what is happening is that sometimes a fish instance doesn't notice that fish history file has changed and continues writing to an old, no longer existing, history file. Which also means that when you do history merge it is reading a history file that is not being updated by other fish instances.

You can see this by running lsof -p %self in each of your shells where history merge isn't working. Find the line mentioning the ~/.local/share/fish/fish_history file. The number under the NODE column is the inode number of the file. What you'll see is that the two fish instances have different node/inum values. If you do ls -i ~/.local/share/fish/fish_history you'll see that only one of them matches the inode number of the current history file. That's what I see on my systems when history syncing stops working. I probably should have opened an issue months ago when I noticed this 😄

@cjthompson: Can you confirm what I wrote above explains why history syncing doesn't work for you?

@ridiculousfish
Copy link
Member

ridiculousfish commented Nov 19, 2016

I'll bet this is a funny ordering problem due to history commands manipulating the history and also expecting to be in the history.

Edit: could also be disable_automatic_save_counter shenanigans.

Edit2: d'oh, I think we're just forgetting to update first_unwritten_new_item_index after the call to new_items.clear(). Attempting to update our test to catch this.

@krader1961
Copy link
Contributor

@ridiculousfish: Mind adding that comment to issue #3565. I haven't looked at the code yet in light of your hypothesis to see if that explains what I've observed. Namely, some fish shells never notice that the history file inode has changed; i.e., the history file has been replaced by a new file.

@ridiculousfish
Copy link
Member

Sure, done

ridiculousfish added a commit that referenced this issue Nov 20, 2016
Prevents an issue where we think we've written out history items,
but we haven't, and so they get lost. Fixes #3496
@ridiculousfish
Copy link
Member

ridiculousfish commented Nov 20, 2016

Not zeroing first_unwritten_new_item_index is definitely a bug, and the fix appears to fix @cjthompson 's test case (thanks for the great repro steps by the way!)

Please give it a try and let us know if it doesn't work. The fix is 9b4310b

@ridiculousfish ridiculousfish added this to the fish 2.5.0 milestone Nov 20, 2016
@krader1961 krader1961 added bug Something that's not working as intended and removed question labels Nov 20, 2016
@ElijahLynn
Copy link

I am having this issue with 2.4.0 sometimes but not all the time.

I will try out 9b4310b.

I also wanted to say that the history --merge option appears to be history merge now.

@krader1961
Copy link
Contributor

... the history --merge option appears to be history merge now.

Both will work although the flag form is now deprecated but won't be removed before the next major release. Same for the status command.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something that's not working as intended
Projects
None yet
Development

No branches or pull requests

8 participants