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

Add locking and traps for gtags_update.sh, and really do locking #361

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Commits on Jun 6, 2024

  1. Add locking to the gtags update script, like ctags has already

    Locking is good to have for all the user's databases, not just ctags.
    smemsh committed Jun 6, 2024
    Configuration menu
    Copy the full SHA
    eb1db0c View commit details
    Browse the repository at this point in the history

Commits on Jun 7, 2024

  1. Add error trap for gtags that removes temporary files on early quit

    We already have this with ctags, should have it for gtags too.
    smemsh committed Jun 7, 2024
    Configuration menu
    Copy the full SHA
    0a6a1a2 View commit details
    Browse the repository at this point in the history
  2. Quote the arg given to shell command, in case it has spaces, etc

    This argument is a path where tags should go (ie g:gutentags_cache_dir,
    or the directory to scan if not set) and would need to be quoted if it
    had spaces or other characters meaningful to the shell.  The variable
    gets assessed by 'eval' before being used to invoke gtags, so we want to
    embed the escapes within the string.
    
    While we do this for correctness, note that unix cscope cannot handle
    directories with spaces in the path to the tags file, because it puts
    the directory name in the database and uses spaces to delimit records in
    the first line (the filename, and also dash-prefixed options cscope had
    when constructing the database, so there's no way to know for sure where
    the filename ends).
    
    Vim cannot handle spaces either.  The limitation does not exist for
    gtags, but vim's ":cs add" (src/if_cscope.c::cs_add()), typically
    invoked from gutentags_plus, does not handle spaces correctly itself.
    It uses spaces in the arguments to break apart into significant
    subwords, to distinguish other arguments to ':cs'.
    
    Lastly the cscope command used by vim in src/if_cscope.c
    cs_create_connection() does not properly quote the filename when it
    passes to /bin/sh -c "exec cscope -dl -f %s" so that wouldn't handle
    spaces either.
    
    For now we can live with the limitation that we can't ever use cscope
    functionality in paths with spaces in them.
    smemsh committed Jun 7, 2024
    Configuration menu
    Copy the full SHA
    668ef5a View commit details
    Browse the repository at this point in the history
  3. Set noclobber in update scripts, otherwise lock file is useless

    The shell default is for clobber to be set, so "echo $$ > $LOCKFILE"
    will succeed no matter how many times it's run.  In other words, this
    wasn't acting as a lockfile at all, unless the user had set 'noclobber'
    in their shell's non-interactive profile.
    
    In each of these scripts, a trap exists that does cleanup, so the lock
    files should get removed at script exit no matter what.  If the computer
    crashed or lost power in between though, the stale lock file would be
    left and have to be manually removed.  This is not unlike any other
    stale lock file issue.
    
    However we could check if the pid exists, since it's written to the
    file.  That sounds invasive and non-portable... maybe later.
    smemsh committed Jun 7, 2024
    Configuration menu
    Copy the full SHA
    f6a5507 View commit details
    Browse the repository at this point in the history

Commits on Jun 9, 2024

  1. Do lock before trap to avoid removing prior run cscope file

    If we try to establish the lock first before EXIT trap is established,
    we can exit without removing the lock file of the previous instance.
    Otherwise we'll correctly exit, but then remove the temp file out from
    under the already-running instance.  Next collision avoidance test will
    pass and we'll clobber previous instance.
    smemsh committed Jun 9, 2024
    Configuration menu
    Copy the full SHA
    5cc72ea View commit details
    Browse the repository at this point in the history

Commits on Sep 16, 2024

  1. Remove even partial files as they can cause gtags segfault

    If we exit early before finish, sometimes partial files are left around.
    Unfortunately these can cause gtags to segfault.  "incremental" in gtags
    should not be thought of as "start where we left off when the last job
    was prematurely killed" but rather "incrementally update the [previously
    completely-generated] database by scanning only files that have
    changed."  If the db was not fully generated, we must remove all the
    partial files from within the trap.  This might cause a lot of work to
    be redone in large source bases, but gtags demonstrably segfaults on
    partials in some cases, and refuses to continue in other cases, claiming
    that the db is corrupt.  It needs to finish, or its results need to be
    discarded, but not anything in-between.
    
    All that is to say, we will not use 'test -s' and leave a partial
    around, but just simply remove any files if the handler is entered at
    all.
    
    We must remove the EXIT from the trap-signals list then, because if we
    leave it, it will always be removed even if fully complete!
    smemsh committed Sep 16, 2024
    Configuration menu
    Copy the full SHA
    0fd7896 View commit details
    Browse the repository at this point in the history