Mirrors/fish-shell
2
0
Fork 0
mirror of https://github.com/fish-shell/fish-shell synced 2024-12-26 04:43:10 +00:00
Code Issues Projects Releases Packages Wiki Activity

fix uvar tests

Browse source 
I noticed that universal variable tests were failing on Cygwin and
Dragonfly BSD. The failures were because we are attempting to verify the
correct behavior of mechanisms that are known to be broken on those
platforms. There are still uvar test failures on those platforms with
this change but they are due to actual problems rather than bugs in the
tests.

Fixes #3587
This commit is contained in:
Kurtis Rader 2016-12-25 20:53:59 -08:00
parent aedee4e1a4
commit 574424dc5a
3 changed files with 57 additions and 123 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

119
src/env_universal_common.cpp
View file

@ -1,28 +1,30 @@
// The utility library for universal variables. Used both by the client library and by the daemon. // The utility library for universal variables. Used both by the client library and by the daemon.
#include "config.h" #include "config.h" // IWYU pragma: keep
#include <arpa/inet.h> // IWYU pragma: keep #include <arpa/inet.h> // IWYU pragma: keep
#include <assert.h> #include <assert.h>
#include <errno.h> #include <errno.h>
#include <fcntl.h> #include <fcntl.h>
#include <limits.h> #include <limits.h>
#include <netinet/in.h> #include <netinet/in.h> // IWYU pragma: keep
#include <pwd.h> #include <pwd.h>
#include <stdlib.h> #include <stdlib.h>
#include <string.h> #include <string.h>
#ifdef __CYGWIN__
#include <sys/mman.h> #include <sys/mman.h>
#endif
#ifdef HAVE_SYS_SELECT_H
#include <sys/select.h> // IWYU pragma: keep
#endif
#include <sys/socket.h> #include <sys/socket.h>
#include <sys/stat.h> #include <sys/stat.h>
#include <sys/time.h> #include <sys/time.h> // IWYU pragma: keep
#include <sys/types.h> #include <sys/types.h>
// We need the sys/file.h for the flock() declaration on Linux but not OS X. // We need the sys/file.h for the flock() declaration on Linux but not OS X.
#include <sys/file.h> // IWYU pragma: keep #include <sys/file.h> // IWYU pragma: keep
// We need the ioctl.h header so we can check if SIOCGIFHWADDR is defined by it so we know if we're // We need the ioctl.h header so we can check if SIOCGIFHWADDR is defined by it so we know if we're
// on a Linux system. // on a Linux system.
#include <sys/ioctl.h> // IWYU pragma: keep #include <sys/ioctl.h> // IWYU pragma: keep
#ifdef HAVE_SYS_SELECT_H
#include <sys/select.h>
#endif
#include <unistd.h> #include <unistd.h>
#include <wchar.h> #include <wchar.h>
#include <map> #include <map>
@ -35,7 +37,7 @@
#include "fallback.h" // IWYU pragma: keep #include "fallback.h" // IWYU pragma: keep
#include "path.h" #include "path.h"
#include "utf8.h" #include "utf8.h"
#include "util.h" #include "util.h" // IWYU pragma: keep
#include "wutil.h" #include "wutil.h"
#if __APPLE__ #if __APPLE__
@ -950,8 +952,8 @@ wcstring get_machine_identifier() {
return result; return result;
} }
#ifdef HAVE_SHM_OPEN
class universal_notifier_shmem_poller_t : public universal_notifier_t { class universal_notifier_shmem_poller_t : public universal_notifier_t {
#ifdef __CYGWIN__
// This is what our shared memory looks like. Everything here is stored in network byte order // This is what our shared memory looks like. Everything here is stored in network byte order
// (big-endian). // (big-endian).
struct universal_notifier_shmem_t { struct universal_notifier_shmem_t {
@ -1089,17 +1091,22 @@ class universal_notifier_shmem_poller_t : public universal_notifier_t {
} }
return usec_per_sec / 3; // 3 times a second return usec_per_sec / 3; // 3 times a second
} }
}; #else // this class isn't valid on this system
public:
universal_notifier_shmem_poller_t() {
DIE("universal_notifier_shmem_poller_t cannot be used on this system");
}
#endif #endif
};
/// A notifyd-based notifier. Very straightforward. /// A notifyd-based notifier. Very straightforward.
class universal_notifier_notifyd_t : public universal_notifier_t { class universal_notifier_notifyd_t : public universal_notifier_t {
#if FISH_NOTIFYD_AVAILABLE
int notify_fd; int notify_fd;
int token; int token;
std::string name; std::string name;
void setup_notifyd() { void setup_notifyd() {
#if FISH_NOTIFYD_AVAILABLE
// Per notify(3), the user.uid.%d style is only accessible to processes with that uid. // Per notify(3), the user.uid.%d style is only accessible to processes with that uid.
char local_name[256]; char local_name[256];
snprintf(local_name, sizeof local_name, "user.uid.%d.%ls.uvars", getuid(), snprintf(local_name, sizeof local_name, "user.uid.%d.%ls.uvars", getuid(),
@ -1128,7 +1135,6 @@ class universal_notifier_notifyd_t : public universal_notifier_t {
// marked as CLO_EXEC, that's probably a good thing. // marked as CLO_EXEC, that's probably a good thing.
set_cloexec(this->notify_fd + 1); set_cloexec(this->notify_fd + 1);
} }
#endif
} }
public: public:
@ -1138,9 +1144,7 @@ class universal_notifier_notifyd_t : public universal_notifier_t {
~universal_notifier_notifyd_t() { ~universal_notifier_notifyd_t() {
if (token != -1 /* NOTIFY_TOKEN_INVALID */) { if (token != -1 /* NOTIFY_TOKEN_INVALID */) {
#if FISH_NOTIFYD_AVAILABLE
notify_cancel(token); notify_cancel(token);
#endif
} }
} }
@ -1161,16 +1165,18 @@ class universal_notifier_notifyd_t : public universal_notifier_t {
} }
void post_notification() { void post_notification() {
#if FISH_NOTIFYD_AVAILABLE
uint32_t status = notify_post(name.c_str()); uint32_t status = notify_post(name.c_str());
if (status != NOTIFY_STATUS_OK) { if (status != NOTIFY_STATUS_OK) {
fprintf(stderr, debug(1, "notify_post() failed with status %u. Uvar notifications may not be sent.",
"Warning: notify_post() failed with status %u. Universal variable " status);
"notifications may not be sent.",
status);
} }
#endif
} }
#else // this class isn't valid on this system
public:
universal_notifier_notifyd_t() {
DIE("universal_notifier_notifyd_t cannot be used on this system");
}
#endif
}; };
#define NAMED_PIPE_FLASH_DURATION_USEC (1000000 / 10) #define NAMED_PIPE_FLASH_DURATION_USEC (1000000 / 10)
@ -1186,6 +1192,7 @@ class universal_notifier_notifyd_t : public universal_notifier_t {
// when there is data remaining in the pipe, if the pipe is kept readable too long, clients will // when there is data remaining in the pipe, if the pipe is kept readable too long, clients will
// attempt to read data out of it (to render it no longer readable). // attempt to read data out of it (to render it no longer readable).
class universal_notifier_named_pipe_t : public universal_notifier_t { class universal_notifier_named_pipe_t : public universal_notifier_t {
#if !defined(__APPLE__) && !defined(__CYGWIN__)
int pipe_fd; int pipe_fd;
long long readback_time_usec; long long readback_time_usec;
size_t readback_amount; size_t readback_amount;
@ -1206,7 +1213,7 @@ class universal_notifier_named_pipe_t : public universal_notifier_t {
} }
public: public:
explicit universal_notifier_named_pipe_t(const wchar_t *test_path) universal_notifier_named_pipe_t(const wchar_t *test_path)
: pipe_fd(-1), : pipe_fd(-1),
readback_time_usec(0), readback_time_usec(0),
readback_amount(0), readback_amount(0),
@ -1332,54 +1339,15 @@ class universal_notifier_named_pipe_t : public universal_notifier_t {
} }
return true; return true;
} }
#else // this class isn't valid on this system
public:
universal_notifier_named_pipe_t(const wchar_t *test_path) {
DIE("universal_notifier_named_pipe_t cannot be used on this system");
}
#endif
}; };
class universal_notifier_null_t : public universal_notifier_t {}; // does nothing
static universal_notifier_t::notifier_strategy_t fetch_default_strategy_from_environment() {
universal_notifier_t::notifier_strategy_t result = universal_notifier_t::strategy_default;
const struct {
const char *name;
universal_notifier_t::notifier_strategy_t strat;
} options[] = {{"default", universal_notifier_t::strategy_default},
#ifdef HAVE_SHM_OPEN
{"shmem", universal_notifier_t::strategy_shmem_polling},
#endif
{"pipe", universal_notifier_t::strategy_named_pipe},
{"notifyd", universal_notifier_t::strategy_notifyd}};
const size_t opt_count = sizeof options / sizeof *options;
const char *var = getenv(UNIVERSAL_NOTIFIER_ENV_NAME);
if (var == NULL || var[0] == '\0') {
return result;
}
size_t i;
for (i = 0; i < opt_count; i++) {
if (!strcmp(var, options[i].name)) {
result = options[i].strat;
break;
}
}
if (i >= opt_count) {
fprintf(stderr, "Warning: unrecognized value for %s: '%s'\n", UNIVERSAL_NOTIFIER_ENV_NAME,
var);
fprintf(stderr, "Warning: valid values are ");
for (size_t j = 0; j < opt_count; j++) {
fprintf(stderr, "%s%s", j > 0 ? ", " : "", options[j].name);
}
fputc('\n', stderr);
}
return result;
}
universal_notifier_t::notifier_strategy_t universal_notifier_t::resolve_default_strategy() { universal_notifier_t::notifier_strategy_t universal_notifier_t::resolve_default_strategy() {
static universal_notifier_t::notifier_strategy_t s_explicit_strategy =
fetch_default_strategy_from_environment();
if (s_explicit_strategy != strategy_default) {
return s_explicit_strategy;
}
#if FISH_NOTIFYD_AVAILABLE #if FISH_NOTIFYD_AVAILABLE
return strategy_notifyd; return strategy_notifyd;
#elif defined(__CYGWIN__) #elif defined(__CYGWIN__)
@ -1390,32 +1358,25 @@ universal_notifier_t::notifier_strategy_t universal_notifier_t::resolve_default_
} }
universal_notifier_t &universal_notifier_t::default_notifier() { universal_notifier_t &universal_notifier_t::default_notifier() {
static universal_notifier_t *result = new_notifier_for_strategy(strategy_default); static universal_notifier_t *result =
new_notifier_for_strategy(universal_notifier_t::resolve_default_strategy());
return *result; return *result;
} }
universal_notifier_t *universal_notifier_t::new_notifier_for_strategy( universal_notifier_t *universal_notifier_t::new_notifier_for_strategy(
universal_notifier_t::notifier_strategy_t strat, const wchar_t *test_path) { universal_notifier_t::notifier_strategy_t strat, const wchar_t *test_path) {
if (strat == strategy_default) {
strat = resolve_default_strategy(); //!OCLINT(parameter reassignment)
}
switch (strat) { switch (strat) {
#ifdef HAVE_SHM_OPEN
case strategy_shmem_polling: {
return new universal_notifier_shmem_poller_t();
}
#endif
case strategy_notifyd: { case strategy_notifyd: {
return new universal_notifier_notifyd_t(); return new universal_notifier_notifyd_t();
} }
case strategy_shmem_polling: {
return new universal_notifier_shmem_poller_t();
}
case strategy_named_pipe: { case strategy_named_pipe: {
return new universal_notifier_named_pipe_t(test_path); return new universal_notifier_named_pipe_t(test_path);
} }
case strategy_null: {
return new universal_notifier_null_t();
}
default: { default: {
fprintf(stderr, "Unsupported strategy %d\n", strat); debug(0, "Unsupported universal notifier strategy %d\n", strat);
return NULL; return NULL;
} }
} }
@ -1439,6 +1400,7 @@ bool universal_notifier_t::notification_fd_became_readable(int fd) {
return false; return false;
} }
#if !defined(__APPLE__) && !defined(__CYGWIN__)
void universal_notifier_named_pipe_t::make_pipe(const wchar_t *test_path) { void universal_notifier_named_pipe_t::make_pipe(const wchar_t *test_path) {
wcstring vars_path = test_path ? wcstring(test_path) : default_named_pipe_path(); wcstring vars_path = test_path ? wcstring(test_path) : default_named_pipe_path();
vars_path.append(L".notifier"); vars_path.append(L".notifier");
@ -1464,3 +1426,4 @@ void universal_notifier_named_pipe_t::make_pipe(const wchar_t *test_path) {
pipe_fd = fd; pipe_fd = fd;
} }
#endif

18
src/env_universal_common.h
View file

@ -1,6 +1,6 @@
#ifndef FISH_ENV_UNIVERSAL_COMMON_H #ifndef FISH_ENV_UNIVERSAL_COMMON_H
#define FISH_ENV_UNIVERSAL_COMMON_H #define FISH_ENV_UNIVERSAL_COMMON_H
#include "config.h" #include "config.h" // IWYU pragma: keep
#include <pthread.h> #include <pthread.h>
#include <stdio.h> #include <stdio.h>
@ -113,24 +113,14 @@ class env_universal_t {
class universal_notifier_t { class universal_notifier_t {
public: public:
enum notifier_strategy_t { enum notifier_strategy_t {
// Default meta-strategy to use the 'best' notifier for the system.
strategy_default,
#ifdef HAVE_SHM_OPEN
// Use a value in shared memory. Simple, but requires polling and therefore semi-frequent // Use a value in shared memory. Simple, but requires polling and therefore semi-frequent
// wakeups. // wakeups.
strategy_shmem_polling, strategy_shmem_polling,
#endif // Strategy that uses notify(3). Simple and efficient, but OS X/macOS only.
strategy_notifyd,
// Strategy that uses a named pipe. Somewhat complex, but portable and doesn't require // Strategy that uses a named pipe. Somewhat complex, but portable and doesn't require
// polling most of the time. // polling most of the time.
strategy_named_pipe, strategy_named_pipe,
// Strategy that uses notify(3). Simple and efficient, but OS X only.
strategy_notifyd,
// Null notifier, does nothing.
strategy_null
}; };
protected: protected:
@ -140,9 +130,9 @@ class universal_notifier_t {
// No copying. // No copying.
universal_notifier_t &operator=(const universal_notifier_t &); universal_notifier_t &operator=(const universal_notifier_t &);
universal_notifier_t(const universal_notifier_t &x); universal_notifier_t(const universal_notifier_t &x);
static notifier_strategy_t resolve_default_strategy();
public: public:
static notifier_strategy_t resolve_default_strategy();
virtual ~universal_notifier_t(); virtual ~universal_notifier_t();
// Factory constructor. Free with delete. // Factory constructor. Free with delete.

43
src/fish_tests.cpp
View file

@ -25,11 +25,8 @@
#include <wchar.h> #include <wchar.h>
#include <wctype.h> #include <wctype.h>
#include <algorithm> #include <algorithm>
#include <iostream>
#include <iterator>
#include <memory> #include <memory>
#include <set> #include <set>
#include <sstream>
#include <string> #include <string>
#include <vector> #include <vector>
@ -1648,7 +1645,7 @@ struct pager_layout_testcase_t {
wcstring text = sd.line(0).to_string(); wcstring text = sd.line(0).to_string();
if (text != expected) { if (text != expected) {
fprintf(stderr, "width %lu got <%ls>, expected <%ls>\n", this->width, text.c_str(), fprintf(stderr, "width %zu got <%ls>, expected <%ls>\n", this->width, text.c_str(),
expected.c_str()); expected.c_str());
} }
do_test(text == expected); do_test(text == expected);
@ -2583,37 +2580,24 @@ bool poll_notifier(universal_notifier_t *note) {
return result; return result;
} }
static void trigger_or_wait_for_notification(universal_notifier_t *notifier, static void trigger_or_wait_for_notification(universal_notifier_t::notifier_strategy_t strategy) {
universal_notifier_t::notifier_strategy_t strategy) {
// This argument should be removed if it isn't actually needed by these unit tests. I'm going to
// silence the warning. See https://github.com/fish-shell/fish-shell/issues/3439.
UNUSED(notifier);
switch (strategy) { switch (strategy) {
case universal_notifier_t::strategy_default: {
DIE("strategy_default should be passed");
break;
}
#ifdef HAVE_SHM_OPEN
case universal_notifier_t::strategy_shmem_polling: { case universal_notifier_t::strategy_shmem_polling: {
break; // nothing required break; // nothing required
} }
#endif
case universal_notifier_t::strategy_notifyd: { case universal_notifier_t::strategy_notifyd: {
// notifyd requires a round trip to the notifyd server, which means we have to wait a // notifyd requires a round trip to the notifyd server, which means we have to wait a
// little bit to receive it. In practice, this seems to be enough. // little bit to receive it. In practice 40 ms seems to be enough.
usleep(1000000 / 25); usleep(40000);
break; break;
} }
case universal_notifier_t::strategy_named_pipe: case universal_notifier_t::strategy_named_pipe: {
case universal_notifier_t::strategy_null: { break; // nothing required
break;
} }
} }
} }
static void test_notifiers_with_strategy(universal_notifier_t::notifier_strategy_t strategy) { static void test_notifiers_with_strategy(universal_notifier_t::notifier_strategy_t strategy) {
assert(strategy != universal_notifier_t::strategy_default);
say(L"Testing universal notifiers with strategy %d", (int)strategy); say(L"Testing universal notifiers with strategy %d", (int)strategy);
universal_notifier_t *notifiers[16]; universal_notifier_t *notifiers[16];
size_t notifier_count = sizeof notifiers / sizeof *notifiers; size_t notifier_count = sizeof notifiers / sizeof *notifiers;
@ -2636,7 +2620,7 @@ static void test_notifiers_with_strategy(universal_notifier_t::notifier_strategy
notifiers[post_idx]->post_notification(); notifiers[post_idx]->post_notification();
// Do special stuff to "trigger" a notification for testing. // Do special stuff to "trigger" a notification for testing.
trigger_or_wait_for_notification(notifiers[post_idx], strategy); trigger_or_wait_for_notification(strategy);
for (size_t i = 0; i < notifier_count; i++) { for (size_t i = 0; i < notifier_count; i++) {
// We aren't concerned with the one who posted. Poll from it (to drain it), and then // We aren't concerned with the one who posted. Poll from it (to drain it), and then
@ -2680,15 +2664,12 @@ static void test_notifiers_with_strategy(universal_notifier_t::notifier_strategy
} }
static void test_universal_notifiers() { static void test_universal_notifiers() {
if (system("mkdir -p /tmp/fish_uvars_test/ && touch /tmp/fish_uvars_test/varsfile.txt")) if (system("mkdir -p /tmp/fish_uvars_test/ && touch /tmp/fish_uvars_test/varsfile.txt")) {
err(L"mkdir failed"); err(L"mkdir failed");
#ifdef HAVE_SHM_OPEN }
test_notifiers_with_strategy(universal_notifier_t::strategy_shmem_polling);
#endif auto strategy = universal_notifier_t::resolve_default_strategy();
test_notifiers_with_strategy(universal_notifier_t::strategy_named_pipe); test_notifiers_with_strategy(strategy);
#if __APPLE__
test_notifiers_with_strategy(universal_notifier_t::strategy_notifyd);
#endif
if (system("rm -Rf /tmp/fish_uvars_test/")) err(L"rm failed"); if (system("rm -Rf /tmp/fish_uvars_test/")) err(L"rm failed");
} }