From c6ec2645dce59acdb2a8b659c84e2fc1aee19882 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sun, 16 Jun 2013 12:51:49 -0700 Subject: [PATCH 1/4] Fix for incorrect use of shared ptr references --- exec.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/exec.cpp b/exec.cpp index 82261b18a..0f4a6fa38 100644 --- a/exec.cpp +++ b/exec.cpp @@ -1196,8 +1196,8 @@ void exec(parser_t &parser, job_t *j) performance quite a bit in complex completion code. */ - const shared_ptr &stdout_io = io_chain_get(j->io, STDOUT_FILENO); - const shared_ptr &stderr_io = io_chain_get(j->io, STDERR_FILENO); + const shared_ptr stdout_io = io_chain_get(j->io, STDOUT_FILENO); + const shared_ptr stderr_io = io_chain_get(j->io, STDERR_FILENO); const bool buffer_stdout = stdout_io && stdout_io->io_mode == IO_BUFFER; if ((get_stderr_buffer().empty()) && From 640118e781d9687ba2f2664629fe205ec081479f Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sun, 16 Jun 2013 23:26:43 -0700 Subject: [PATCH 2/4] Cleanup of code that decides whether or not to fork. Fix for issue where stderr may be output twice. --- exec.cpp | 235 +++++++++++++++++++++++++----------------------- tests/test9.in | 11 ++- tests/test9.out | 2 + 3 files changed, 134 insertions(+), 114 deletions(-) diff --git a/exec.cpp b/exec.cpp index 0f4a6fa38..d5c7d4bf8 100644 --- a/exec.cpp +++ b/exec.cpp @@ -149,6 +149,25 @@ int exec_pipe(int fd[2]) return res; } +/* Returns true if the redirection is a file redirection to a file other than /dev/null */ +static bool redirection_is_to_real_file(const io_data_t *io) +{ + bool result = false; + if (io != NULL && io->io_mode == IO_FILE) + { + /* It's a file redirection. Compare the path to /dev/null */ + CAST_INIT(const io_file_t *, io_file, io); + const char *path = io_file->filename_cstr; + if (strcmp(path, "/dev/null") != 0) + { + /* It's not /dev/null */ + result = true; + } + + } + return result; +} + void print_open_fds(void) { for (size_t i=0; i < open_fds.size(); ++i) @@ -518,21 +537,16 @@ static bool can_use_posix_spawn_for_job(const job_t *job, const process_t *proce return false; } } - + + /* Now see if we have a redirection involving a file. The only one we allow is /dev/null, which we assume will not fail. */ bool result = true; for (size_t idx = 0; idx < job->io.size(); idx++) { const shared_ptr &io = job->io.at(idx); - if (io->io_mode == IO_FILE) + if (redirection_is_to_real_file(io.get())) { - CAST_INIT(const io_file_t *, io_file, io.get()); - const char *path = io_file->filename_cstr; - /* This IO action is a file redirection. Only allow /dev/null, which is a common case we assume won't fail. */ - if (strcmp(path, "/dev/null") != 0) - { result = false; break; - } } } return result; @@ -910,7 +924,7 @@ void exec(parser_t &parser, job_t *j) */ if (p == j->first_process) { - const shared_ptr &in = io_chain_get(j->io, 0); + const shared_ptr in = io_chain_get(j->io, 0); if (in) { @@ -1170,82 +1184,75 @@ void exec(parser_t &parser, job_t *j) case INTERNAL_BUILTIN: { - bool skip_fork; - /* Handle output from builtin commands. In the general case, this means forking of a worker process, that will write out the contents of the stdout and stderr buffers to the correct file descriptor. Since - forking is expensive, fish tries to avoid it wehn + forking is expensive, fish tries to avoid it when possible. */ - - /* - If a builtin didn't produce any output, and it is - not inside a pipeline, there is no need to fork - */ - skip_fork = - get_stdout_buffer().empty() && - get_stderr_buffer().empty() && - !p->next; - - /* - If the output of a builtin is to be sent to an internal - buffer, there is no need to fork. This helps out the - performance quite a bit in complex completion code. - */ - + + bool fork_was_skipped = false; + const shared_ptr stdout_io = io_chain_get(j->io, STDOUT_FILENO); const shared_ptr stderr_io = io_chain_get(j->io, STDERR_FILENO); - const bool buffer_stdout = stdout_io && stdout_io->io_mode == IO_BUFFER; - - if ((get_stderr_buffer().empty()) && - (!p->next) && - (! get_stdout_buffer().empty()) && - (buffer_stdout)) - { - CAST_INIT(io_buffer_t *, io_buffer, stdout_io.get()); - const std::string res = wcs2string(get_stdout_buffer()); - io_buffer->out_buffer_append(res.c_str(), res.size()); - skip_fork = true; - } - - /* PCA for some reason, fish forks a lot, even for basic builtins like echo just to write out their buffers. I'm certain a lot of this is unnecessary, but I am not sure exactly when. If j->io is NULL, then it means there's no pipes or anything, so we can certainly just write out our data. Beyond that, we may be able to do the same if io_get returns 0 for STDOUT_FILENO and STDERR_FILENO. - */ - if (! skip_fork && stdout_io.get() == NULL && stderr_io.get() == NULL) - { - if (g_log_forks) + + /* If we are outputting to a file, we have to actually do it, even if we have no output, so that we can truncate the file. Does not apply to /dev/null. */ + bool must_fork = redirection_is_to_real_file(stdout_io.get()) || redirection_is_to_real_file(stderr_io.get()); + if (! must_fork) + { + if (p->next == NULL) { - printf("fork #-: Skipping fork for internal builtin for '%ls'\n", p->argv0()); - } - const wcstring &out = get_stdout_buffer(), &err = get_stderr_buffer(); - const std::string outbuff = wcs2string(out); - const std::string errbuff = wcs2string(err); - bool builtin_io_done = do_builtin_io(outbuff.data(), outbuff.size(), errbuff.data(), errbuff.size()); - if (! builtin_io_done) - { - show_stackframe(); - } - skip_fork = true; - } + const bool stdout_is_to_buffer = stdout_io && stdout_io->io_mode == IO_BUFFER; + const bool no_stdout_output = get_stdout_buffer().empty(); + const bool no_stderr_output = get_stderr_buffer().empty(); - for (io_chain_t::iterator iter = j->io.begin(); iter != j->io.end(); ++iter) - { - const shared_ptr &tmp_io = *iter; - if (tmp_io->io_mode == IO_FILE) - { - const io_file_t *tmp_file_io = static_cast(tmp_io.get()); - if (strcmp(tmp_file_io->filename_cstr, "/dev/null")) + if (no_stdout_output && no_stderr_output) { - skip_fork = false; - break; + /* The builtin produced no output and is not inside of a pipeline. No need to fork or even output anything. */ + if (g_log_forks) + { + // This one is very wordy + //printf("fork #-: Skipping fork due to no output for internal builtin for '%ls'\n", p->argv0()); + } + fork_was_skipped = true; + } + else if (no_stderr_output && stdout_is_to_buffer) + { + /* The builtin produced no stderr, and its stdout is going to an internal buffer. There is no need to fork. This helps out the performance quite a bit in complex completion code. */ + if (g_log_forks) + { + printf("fork #-: Skipping fork due to buffered output for internal builtin for '%ls'\n", p->argv0()); + } + + CAST_INIT(io_buffer_t *, io_buffer, stdout_io.get()); + const std::string res = wcs2string(get_stdout_buffer()); + io_buffer->out_buffer_append(res.data(), res.size()); + fork_was_skipped = true; + } + else if (stdout_io.get() == NULL && stderr_io.get() == NULL) + { + /* We are writing to normal stdout and stderr. Just do it - no need to fork. */ + if (g_log_forks) + { + printf("fork #-: Skipping fork due to ordinary output for internal builtin for '%ls'\n", p->argv0()); + } + const wcstring &out = get_stdout_buffer(), &err = get_stderr_buffer(); + const std::string outbuff = wcs2string(out); + const std::string errbuff = wcs2string(err); + bool builtin_io_done = do_builtin_io(outbuff.data(), outbuff.size(), errbuff.data(), errbuff.size()); + if (! builtin_io_done) + { + show_stackframe(); + } + fork_was_skipped = true; } } } + - - if (skip_fork) + if (fork_was_skipped) { p->completed=1; if (p->next == 0) @@ -1255,54 +1262,56 @@ void exec(parser_t &parser, job_t *j) int status = p->status; proc_set_last_status(job_get_flag(j, JOB_NEGATE)?(!status):status); } - break; - } - - - /* Ok, unfortunatly, we have to do a real fork. Bummer. We work hard to make sure we don't have to wait for all our threads to exit, by arranging things so that we don't have to allocate memory or do anything except system calls in the child. */ - - /* Get the strings we'll write before we fork (since they call malloc) */ - const wcstring &out = get_stdout_buffer(), &err = get_stderr_buffer(); - - /* These strings may contain embedded nulls, so don't treat them as C strings */ - const std::string outbuff_str = wcs2string(out); - const char *outbuff = outbuff_str.data(); - size_t outbuff_len = outbuff_str.size(); - - const std::string errbuff_str = wcs2string(err); - const char *errbuff = errbuff_str.data(); - size_t errbuff_len = errbuff_str.size(); - - fflush(stdout); - fflush(stderr); - if (g_log_forks) - { - printf("fork #%d: Executing fork for internal builtin for '%ls'\n", g_fork_count, p->argv0()); - io_print(j->io); - } - pid = execute_fork(false); - if (pid == 0) - { - /* - This is the child process. Setup redirections, - print correct output to stdout and stderr, and - then exit. - */ - p->pid = getpid(); - setup_child_process(j, p); - do_builtin_io(outbuff, outbuff_len, errbuff, errbuff_len); - exit_without_destructors(p->status); } else { - /* - This is the parent process. Store away - information on the child, and possibly give - it control over the terminal. - */ - p->pid = pid; - set_child_group(j, p, 0); + + /* Ok, unfortunately, we have to do a real fork. Bummer. We work hard to make sure we don't have to wait for all our threads to exit, by arranging things so that we don't have to allocate memory or do anything except system calls in the child. */ + + /* Get the strings we'll write before we fork (since they call malloc) */ + const wcstring &out = get_stdout_buffer(), &err = get_stderr_buffer(); + + /* These strings may contain embedded nulls, so don't treat them as C strings */ + const std::string outbuff_str = wcs2string(out); + const char *outbuff = outbuff_str.data(); + size_t outbuff_len = outbuff_str.size(); + + const std::string errbuff_str = wcs2string(err); + const char *errbuff = errbuff_str.data(); + size_t errbuff_len = errbuff_str.size(); + + fflush(stdout); + fflush(stderr); + if (g_log_forks) + { + printf("fork #%d: Executing fork for internal builtin for '%ls'\n", g_fork_count, p->argv0()); + io_print(j->io); + } + pid = execute_fork(false); + if (pid == 0) + { + /* + This is the child process. Setup redirections, + print correct output to stdout and stderr, and + then exit. + */ + p->pid = getpid(); + setup_child_process(j, p); + do_builtin_io(outbuff, outbuff_len, errbuff, errbuff_len); + exit_without_destructors(p->status); + } + else + { + /* + This is the parent process. Store away + information on the child, and possibly give + it control over the terminal. + */ + p->pid = pid; + + set_child_group(j, p, 0); + } } break; diff --git a/tests/test9.in b/tests/test9.in index b88427278..a38fbc7c1 100644 --- a/tests/test9.in +++ b/tests/test9.in @@ -1,5 +1,14 @@ +# ensure that builtins that produce no output can still truncate files +# (bug PCA almost reintroduced!) +echo "Testing that builtins can truncate files" +echo abc > /tmp/file_truncation_test.txt +cat /tmp/file_truncation_test.txt +echo -n > /tmp/file_truncation_test.txt +cat /tmp/file_truncation_test.txt + # Test events. + # This pattern caused a crash; github issue #449 set -g var before @@ -25,4 +34,4 @@ end emit test3 foo bar # test empty argument -emit \ No newline at end of file +emit diff --git a/tests/test9.out b/tests/test9.out index 779f9a844..8e19365cd 100644 --- a/tests/test9.out +++ b/tests/test9.out @@ -1,2 +1,4 @@ +Testing that builtins can truncate files +abc before:test1 received event test3 with args: foo bar From e027492e11441e9350dd45894d9815bd3ba83cc7 Mon Sep 17 00:00:00 2001 From: Ivan Giuliani Date: Tue, 4 Jun 2013 20:05:16 +0200 Subject: [PATCH 3/4] Added completions for Vagrant --- share/completions/vagrant.fish | 111 +++++++++++++++++++++++++++++++++ 1 file changed, 111 insertions(+) create mode 100644 share/completions/vagrant.fish diff --git a/share/completions/vagrant.fish b/share/completions/vagrant.fish new file mode 100644 index 000000000..d0d888fde --- /dev/null +++ b/share/completions/vagrant.fish @@ -0,0 +1,111 @@ +# vagrant autocompletion + +function __fish_vagrant_no_command --description 'Test if vagrant has yet to be given the main command' + set cmd (commandline -opc) + if [ (count $cmd) -eq 1 ] + return 0 + end + return 1 +end + +function __fish_vagrant_using_command + set cmd (commandline -opc) + if [ (count $cmd) -gt 1 ] + if [ $argv[1] = $cmd[2] ] + return 0 + end + end + return 1 +end + +function __fish_vagrant_using_subcommand + set cmd (commandline -opc) + set cmd_main $argv[1] + set cmd_sub $argv[2] + + if [ (count $cmd) -gt 2 ] + if [ $cmd_main = $cmd[2] ]; and [ $cmd_sub = $cmd[3] ] + return 0 + end + end + return 1 +end + +function __fish_vagrant_boxes --description 'Lists all available Vagrant boxes' + command vagrant box list +end + +# --version and --help are always active +complete -c vagrant -f -s v -l version -d 'Print the version and exit' +complete -c vagrant -f -s h -l help -d 'Print the help and exit' + +# box +complete -c vagrant -f -n '__fish_vagrant_no_command' -a 'box' -d 'Manages boxes' +complete -c vagrant -n '__fish_vagrant_using_command box' -a add -d 'Adds a box' +complete -c vagrant -f -n '__fish_vagrant_using_command box' -a list -d 'Lists all the installed boxes' +complete -c vagrant -f -n '__fish_vagrant_using_command box' -a remove -d 'Removes a box from Vagrant' +complete -c vagrant -f -n '__fish_vagrant_using_command box' -a repackage -d 'Repackages the given box for redistribution' +complete -c vagrant -f -n '__fish_vagrant_using_subcommand box add' -l provider -d 'Verifies the box with the given provider' +complete -c vagrant -f -n '__fish_vagrant_using_subcommand box remove' -a '(__fish_vagrant_boxes)' + +# destroy +complete -c vagrant -f -n '__fish_vagrant_no_command' -a 'destroy' -d 'Destroys the running machine' +complete -c vagrant -f -n '__fish_vagrant_using_command destroy' -s f -l force -d 'Destroy without confirmation' + +# gem +complete -c vagrant -f -n '__fish_vagrant_no_command' -a 'gem' -d 'Install Vagrant plugins through ruby gems' + +# halt +complete -c vagrant -f -n '__fish_vagrant_no_command' -a 'halt' -d 'Shuts down the running machine' +complete -c vagrant -f -n '__fish_vagrant_using_command halt' -s f -l force -d 'Force shut down' + +# init +complete -c vagrant -f -n '__fish_vagrant_no_command' -a 'init' -d 'Initializes the Vagrant environment' +complete -c vagrant -f -n '__fish_vagrant_using_command init' -a '(__fish_vagrant_boxes)' + +# package +complete -c vagrant -n '__fish_vagrant_no_command' -a 'package' -d 'Packages a running machine into a reusable box' +complete -c vagrant -n '__fish_vagrant_using_command package' -l base -d 'Name or UUID of the virtual machine' +complete -c vagrant -n '__fish_vagrant_using_command package' -l output -d 'Output file name' +complete -c vagrant -n '__fish_vagrant_using_command package' -l include -r -d 'Additional files to package with the box' +complete -c vagrant -n '__fish_vagrant_using_command package' -l vagrantfile -r -d 'Include the given Vagrantfile with the box' + +# plugin +complete -c vagrant -n '__fish_vagrant_no_command' -a 'plugin' -d 'Manages plugins' +complete -c vagrant -n '__fish_vagrant_using_command plugin' -a install -r -d 'Installs a plugin' +complete -c vagrant -n '__fish_vagrant_using_command plugin' -a license -r -d 'Installs a license for a proprietary Vagrant plugin' +complete -c vagrant -f -n '__fish_vagrant_using_command plugin' -a list -d 'Lists installed plugins' +complete -c vagrant -n '__fish_vagrant_using_command plugin' -a uninstall -r -d 'Uninstall the given plugin' + +# provision +complete -c vagrant -n '__fish_vagrant_no_command' -a 'provision' -d 'Runs configured provisioners on the running machine' +complete -c vagrant -n '__fish_vagrant_using_command provision' -l provision-with -r -d 'Run only the given provisioners' + +# reload +complete -c vagrant -f -n '__fish_vagrant_no_command' -a 'reload' -d 'Restarts the running machine' +complete -c vagrant -f -n '__fish_vagrant_using_command reload' -l no-provision -r -d 'Provisioners will not run' +complete -c vagrant -f -n '__fish_vagrant_using_command reload' -l provision-with -r -d 'Run only the given provisioners' + +# resume +complete -c vagrant -x -n '__fish_vagrant_no_command' -a 'resume' -d 'Resumes a previously suspended machine' + +# ssh +complete -c vagrant -f -n '__fish_vagrant_no_command' -a 'ssh' -d 'SSH into a running machine' +complete -c vagrant -f -n '__fish_vagrant_using_command ssh' -s c -l command -r -d 'Executes a single SSH command' +complete -c vagrant -f -n '__fish_vagrant_using_command ssh' -s p -l plain -r -d 'Does not authenticate' + +# ssh-config +complete -c vagrant -f -n '__fish_vagrant_no_command' -a 'ssh-config' -d 'Outputs configuration for an SSH config file' +complete -c vagrant -f -n '__fish_vagrant_using_command ssh-config' -l host -r -d 'Name of the host for the outputted configuration' + +# status +complete -c vagrant -x -n '__fish_vagrant_no_command' -a 'status' -d 'Status of the machines Vagrant is managing' + +# suspend +complete -c vagrant -x -n '__fish_vagrant_no_command' -a 'suspend' -d 'Suspends the running machine' + +# up +complete -c vagrant -f -n '__fish_vagrant_no_command' -a 'up' -d 'Creates and configures guest machines' +complete -c vagrant -f -n '__fish_vagrant_using_command up' -l provision -d 'Enable provision' +complete -c vagrant -f -n '__fish_vagrant_using_command up' -l no-provision -d 'Disable provision' +complete -c vagrant -f -n '__fish_vagrant_using_command up' -l provision-with -r -d 'Enable only certain provisioners, by type' From 292908c00a166d2989efc1d1508a067bb49b88ca Mon Sep 17 00:00:00 2001 From: James French Date: Tue, 4 Jun 2013 20:56:37 +0800 Subject: [PATCH 4/4] Portmaster completions * Added FreeBSD's pkg to __fish_print_packages * Portmaster completes on installed packages and ports * Options list as per fish_generate_completions, needs to be tidied up further but will suffice for now --- share/completions/portmaster.fish | 55 ++++++++++++++++++++++ share/functions/__fish_print_packages.fish | 11 +++++ 2 files changed, 66 insertions(+) create mode 100644 share/completions/portmaster.fish diff --git a/share/completions/portmaster.fish b/share/completions/portmaster.fish new file mode 100644 index 000000000..81cbacc91 --- /dev/null +++ b/share/completions/portmaster.fish @@ -0,0 +1,55 @@ +#completion for portmaster + +# These completions are generated from the man page +complete -c portmaster -l force-config --description 'run \'make config\' for all ports (overrides -G).' +complete -c portmaster -s C --description 'prevents \'make clean\' from being run before building.' +complete -c portmaster -s G --description 'prevents \'make config\'.' +complete -c portmaster -s H --description 'hide details of the port build and install in a log file.' +complete -c portmaster -s K --description 'prevents \'make clean\' from being run after building.' +complete -c portmaster -s B --description 'prevents creation of the backup package for the installed port.' +complete -c portmaster -s b --description 'create and keep a backup package of an installed port.' +complete -c portmaster -s g --description 'create a package of the new port.' +complete -c portmaster -s n --description 'run through all steps, but do not make or install any ports.' +complete -c portmaster -s t --description 'recurse dependencies thoroughly, using all-depends-list.' +complete -c portmaster -s v --description 'verbose output.' +complete -c portmaster -s w --description 'save old shared libraries before deinstall [-R]… [See Man Page]' +complete -c portmaster -s i --description 'interactive update mode -- ask whether to rebuild ports.' +complete -c portmaster -s D --description 'no cleaning of distfiles.' +complete -c portmaster -s d --description 'always clean distfiles.' +complete -c portmaster -s m --description 'any arguments to supply to make 1.' +complete -c portmaster -s x --description 'avoid building or updating ports that match this pattern.' +complete -c portmaster -l no-confirm --description 'do not ask the user to confirm the list of port… [See Man Page]' +complete -c portmaster -l no-term-title --description 'do not update the xterm title bar.' +complete -c portmaster -l no-index-fetch --description 'skip fetching the INDEX file.' +complete -c portmaster -l index --description 'use INDEX-[7-9] exclusively to check if a port is up to date.' +complete -c portmaster -l index-first --description 'use the INDEX for status, but double-check with the port.' +complete -c portmaster -l index-only --description 'do not try to use /usr/ports.' +complete -c portmaster -l delete-build-only --description 'delete ports that are build-only dependencies a… [See Man Page]' +complete -c portmaster -l update-if-newer --description '(only for multiple ports listed on the command … [See Man Page]' +complete -c portmaster -s P -l packages --description 'use packages, but build port if not available.' +complete -c portmaster -o PP -l packages-only --description 'fail if no package is available.' +complete -c portmaster -l packages-build --description 'use packages for all build dependencies.' +complete -c portmaster -l packages-if-newer --description 'use package if newer than installed even if the… [See Man Page]' +complete -c portmaster -l always-fetch --description 'fetch package even if it already exists locally.' +complete -c portmaster -l local-packagedir --description 'where local packages can be found, will fall ba… [See Man Page]' +complete -c portmaster -l packages-local --description 'use packages from -local-packagedir only.' +complete -c portmaster -l delete-packages --description 'after installing from a package, delete it El P… [See Man Page]' +complete -c portmaster -s a --description 'check all ports, update as necessary.' +complete -c portmaster -l show-work --description 'show what dependent ports are, and are not inst… [See Man Page]' +complete -c portmaster -s o --description 'replace the installed port with a port from a d… [See Man Page]' +complete -c portmaster -s R --description 'used with the r or f options to skip ports upda… [See Man Page]' +complete -c portmaster -s l --description 'list all installed ports by category.' +complete -c portmaster -s L --description 'list all installed ports by category, and search for updates.' +complete -c portmaster -l list-origins --description 'list directories from /usr/ports for root and leaf ports.' +complete -c portmaster -s y --description 'answer yes to all user prompts for the features… [See Man Page]' +complete -c portmaster -s h -l help --description 'display help message.' +complete -c portmaster -l version --description 'display the version number El ENVIRONMENT The d… [See Man Page]' + +# Grab items from the ports directory, max depth 2 +complete -c portmaster -f --description 'Ports Directory' -a " +( + ls -d /usr/ports/(commandline -ct)*/ \ + | sed -E -e 's#/usr/ports/##' -e 's#/+#/#' -e 's#([^/]+/[^/]+).*#\1#' +)" + +complete -c portmaster -f --description 'Installed Package' -a "(__fish_print_packages)" diff --git a/share/functions/__fish_print_packages.fish b/share/functions/__fish_print_packages.fish index 6ee76c555..95a822c7b 100644 --- a/share/functions/__fish_print_packages.fish +++ b/share/functions/__fish_print_packages.fish @@ -22,6 +22,17 @@ function __fish_print_packages return end + # Pkg is fast on FreeBSD and provides versioning info which we want for + # installed packages + if begin + type -f pkg > /dev/null + and test (uname) = "FreeBSD" + end + pkg query "%n-%v" + return + end + + # yum is slow, just like rpm, so go to the background if type -f /usr/share/yum-cli/completion-helper.py >/dev/null