mcfly icon indicating copy to clipboard operation
mcfly copied to clipboard

BUG: McFly exporting `HISTFILE` can cause conflicts between shells

Open dithpri opened this issue 2 years ago • 6 comments

I pretty much exclusively use zsh. However, I sometimes need to run bash (or run it from nix-shell). I've had no problems with this until I installed McFly.

Since McFly exports HISTFILE when loaded in an interactive zsh shell: https://github.com/cantino/mcfly/blob/75808a60c78f65e90767ee53c9468086fcdf0cc5/mcfly.zsh#L13=

If bash is run from such a shell, because HISTFILE is exported, bash's HISTFILE is the same as zsh's. As it happens, the default history file size for bash is 500. So, every time I run bash, my zsh history file gets truncated:

$ wc -l "$HISTFILE"
7475 /home/dith/.local/share/zsh/history
$ bash -i =(echo true)
$ wc -l "$HISTFILE"
501 /home/dith/.local/share/zsh/history

Moreover, bash saves its history to the zsh HISTFILE...

The workaround is to set a different HISTFILE in .bashrc, but this is most definitely a bug introduced by McFly exporting the HISTFILE variable. McFly should honour the variable not being exported and define its own variables for internal use instead of exporting ones that don't belong to it.

Suggestion: export MCFLY_HISTFILE="${HISTFILE:-$HOME/.zsh_history}" (as it seems to be done for fish)

Same goes for the bash script https://github.com/cantino/mcfly/blob/75808a60c78f65e90767ee53c9468086fcdf0cc5/mcfly.bash#L13=

dithpri avatar Apr 22 '22 16:04 dithpri

If you change mcfly.bash to something like the following, does everything start working for you?

# Ensure MCFLY_HISTFILE exists.
default_histfile="${HISTFILE:-$HOME/.bash_history}"
export MCFLY_HISTFILE="${MCFLY_HISTFILE:-$default_histfile}"
if [[ ! -r "${MCFLY_HISTFILE}" ]]; then
  echo "McFly: ${MCFLY_HISTFILE} does not exist or is not readable. Please fix this or set MCFLY_HISTFILE to something else before using McFly."
  return 1
fi

cantino avatar May 08 '22 16:05 cantino

No, that doesn't work, because HISTFILE is exported in mcfly.zsh (the issue I'm having is running bash from a zsh shell).

But changing mcfly.zsh to a similar snippet:

# Ensure MCFLY_HISTFILE exists.
default_histfile="${HISTFILE:-$HOME/.zsh_history}"
export MCFLY_HISTFILE="${MCFLY_HISTFILE:-$default_histfile}"
if [[ ! -r "${MCFLY_HISTFILE}" ]]; then
  echo "McFly: ${MCFLY_HISTFILE} does not exist or is not readable. Please fix this or set MCFLY_HISTFILE to something else before using McFly."
  return 1
fi

does fix this for me.

I think both your suggestion and this one will have to be applied so that it works both ways (so that zsh and bash don't share the same HISTFILE unless set so by the user in both shells' rcs.

This however does not update MCFLY_HISTFILE (but that worked the same way before) when a sub-shell is run, but that's a separate, related, bug.

dithpri avatar May 13 '22 09:05 dithpri

That makes sense. We should just make this change everywhere. Would you be up for making it, or should I?

cantino avatar May 15 '22 18:05 cantino

I'll submit a pull request sometime this week.

dithpri avatar May 19 '22 10:05 dithpri

I tried McFly for about five minutes today and this led to the silent deletion of years of my shell history.

AndrewKvalheim avatar Dec 07 '22 23:12 AndrewKvalheim

I'm sorry to hear that Andrew, we'll prioritize getting this fixed.

cantino avatar Dec 08 '22 00:12 cantino