James Hunt has proposed merging lp:~jamesodhunt/upstart/bug-1360208 into lp:upstart.
Requested reviews: Upstart Reviewers (upstart-reviewers) Related bugs: Bug #1360208 in upstart : "file watcher doesn't execute if combining ~ and * in the file description" https://bugs.launchpad.net/upstart/+bug/1360208 For more details, see: https://code.launchpad.net/~jamesodhunt/upstart/bug-1360208/+merge/234869 Fix for bug 1360208. * extra/upstart-file-bridge.c: - Converted original_path() macro to a function. - WatchedFile: Added more details on this crucial type. - file_filter(): Removed as it was too simplistic and duplicating the work of the individual handlers in determining whether a path should be considered. - create_handler(): Simplified. - modify_handler(): Simplified. - delete_handler(): Simplified. - handle_event(): Now deals with globs and handles tilde+glob jobs (LP: #1360208). - handle_glob(): New function that allows main handlers to be simplified. - Added lots of debug for '--debug'. - expand_path(): Only check password database if $HOME not set. This allows the tests to use a fake $HOME to check that tilde expansion works without modifying the users actual $HOME. - New utility functions: - file_exists() - remove_trailing_slashes() * scripts/pyupstart.py: - Add missing file header. - pep8 formatting changes. * scripts/tests/test_pyupstart_session_init.py: TestFileBridge: - test_init_start_file_bridge(): - Force the file bridge to run with a fake $HOME below /tmp to allow testing jobs with paths that require tilde expansion. - Run file bridge in foreground to capture debug output. - Change tests to check for values of all variables the file-event(7) sets. - New tests for: - glob file job. - tilde file job. - glob and tilde file job. * scripts/tests/test_pyupstart_system_init.py: pep8 formatting changes. -- https://code.launchpad.net/~jamesodhunt/upstart/bug-1360208/+merge/234869 Your team Upstart Reviewers is requested to review the proposed merge of lp:~jamesodhunt/upstart/bug-1360208 into lp:upstart.
=== modified file 'ChangeLog' --- ChangeLog 2014-09-08 15:18:43 +0000 +++ ChangeLog 2014-09-16 19:04:34 +0000 @@ -1,3 +1,4 @@ +<<<<<<< TREE 2014-09-08 James Hunt <[email protected]> * util/telinit.c: Remove UPSTART_TELINIT_U_NO_WAIT check as it @@ -35,6 +36,45 @@ * init/tests/test_job.c: test_job_last_process(): New test for job_last_process(). +======= +2014-09-16 James Hunt <[email protected]> + + * extra/upstart-file-bridge.c: + - Converted original_path() macro to a function. + - WatchedFile: Added more details on this crucial type. + - file_filter(): Removed as it was too simplistic and duplicating the + work of the individual handlers in determining whether a path should + be considered. + - create_handler(): Simplified. + - modify_handler(): Simplified. + - delete_handler(): Simplified. + - handle_event(): Now deals with globs and handles tilde+glob jobs + (LP: #1360208). + - handle_glob(): New function that allows main handlers to be simplified. + - Added lots of debug for '--debug'. + - expand_path(): Only check password database if $HOME not set. This + allows the tests to use a fake $HOME to check that tilde expansion + works without modifying the users actual $HOME. + - New utility functions: + - file_exists() + - remove_trailing_slashes() + * scripts/pyupstart.py: + - Add missing file header. + - pep8 formatting changes. + * scripts/tests/test_pyupstart_session_init.py: TestFileBridge: + - test_init_start_file_bridge(): + - Force the file bridge to run with a fake $HOME below /tmp to + allow testing jobs with paths that require tilde expansion. + - Run file bridge in foreground to capture debug output. + - Change tests to check for values of all variables the file-event(7) + sets. + - New tests for: + - glob file job. + - tilde file job. + - glob and tilde file job. + * scripts/tests/test_pyupstart_system_init.py: pep8 formatting changes. + +>>>>>>> MERGE-SOURCE 2014-08-14 James Hunt <[email protected]> * init/control.c: Disallow modifying system jobs via SetEnv, === modified file 'extra/upstart-file-bridge.c' --- extra/upstart-file-bridge.c 2013-11-03 00:19:02 +0000 +++ extra/upstart-file-bridge.c 2014-09-16 19:04:34 +0000 @@ -1,6 +1,6 @@ /* upstart * - * Copyright © 2013 Canonical Ltd. + * Copyright © 2013-2014 Canonical Ltd. * Author: James Hunt <[email protected]>. * * This program is free software; you can redistribute it and/or modify @@ -173,22 +173,6 @@ #define GLOB_CHARS "*?[]" /** - * original_path: - * - * @file: WatchedFile: - * - * Obtain the appropriate WatchedFile path: the original if the - * path underwent expansion or is a directory, else the initial - * unexpanded path. - * - * Required for emitting events since jobs need the unexpanded path to - * allow their start/stop condition to match even if the path has - * subsequently been expanded by this bridge. - **/ -#define original_path(file) \ - (file->original ? file->original : file->path) - -/** * skip_slashes: * * @path: pointer to path string. @@ -253,9 +237,10 @@ * WatchedFile: * * @entry: list header, - * @path: full path to file being watched (or a glob), - * @original: original (relative) path as specified by job - * (or NULL if path expansion was not necessary), + * @path: full path to file being watched (including a glob pattern + * if @glob is set), + * @original: original (relative) path as specified by job without glob + * pattern (or NULL if path expansion was not necessary), * @glob: glob file pattern (or NULL if globbing disabled), * @dir: TRUE if @path is a directory, * @events: mask of inotify events file is interested in, @@ -265,6 +250,27 @@ * * For directories, @original contains the full path specified by the * job and @path will contain @original, less any trailing slashes. + * + * Note: This type contains a representation of the 'start on file' + * Upstart stanza. + * + * For example, if a job specified the following (but without the space + * between '/' and '*'): + * + * start on file FILE=~/.cache/foo/ *.log EVENT=create + * + * The WatchedFile object would contain: + * + * path : "/home/user/.cache/foo/ *.log" (again without the space). + * original : "~/.cache/foo/" + * glob : "*.log" + * dir : FALSE + * events : IN_CREATE + * + * If file "/home/user/.cache/foo/bar.log" is created, an event will be + * emitted as below (again without the space between '/' and '*'): + * + * file FILE=~/.cache/foo/ *.log EVENT=create MATCH=/home/user/.cache/foo/bar.log **/ typedef struct watched_file { NihList entry; @@ -306,8 +312,6 @@ static Job *job_new (const char *class_path) __attribute__ ((warn_unused_result)); -static int file_filter (WatchedDir *dir, const char *path, int is_dir); - static void create_handler (WatchedDir *dir, NihWatch *watch, const char *path, struct stat *statbuf); @@ -327,34 +331,47 @@ static void emit_event_error (void *data, NihDBusMessage *message); static int emit_event (const char *path, uint32_t event_type, - const char *match); + const char *match); static FileEvent *file_event_new (void *parent, const char *path, uint32_t event, const char *match); static void upstart_disconnected (DBusConnection *connection); -static void handle_event (NihHash *handled, const char *path, - uint32_t event, const char *match); +static void handle_event (NihHash *handled, WatchedFile *file, + uint32_t event, const char *match); + +static int handle_glob (WatchedFile *file, const char *path, + NihHash *handled, uint32_t event) + __attribute__ ((warn_unused_result)); static int job_destroy (Job *job); -static char * find_first_parent (const char *path) +static char *find_first_parent (const char *path) __attribute__ ((warn_unused_result)); -void watched_dir_init (void); +static void watched_dir_init (void); static void ensure_watched (Job *job, WatchedFile *file); static int string_match (const char *a, const char *b) __attribute__ ((warn_unused_result)); -char * expand_path (const void *parent, const char *path) +static int file_exists (const char *path) + __attribute__ ((warn_unused_result)); + +static void remove_trailing_slashes (char *path); + +static char *expand_path (const void *parent, const char *path) __attribute__ ((warn_unused_result)); static int path_valid (const char *path) __attribute__ ((warn_unused_result)); +static char *original_path (void *parent, const WatchedFile *file) + __attribute__ ((warn_unused_result)); + + /** * daemonise: * @@ -798,8 +815,11 @@ } } - if (! path_valid (path)) + if (! path_valid (path)) { + nih_warn ("%s: %s", + _("Path is not valid"), path); return; + } dirpart = NIH_MUST (nih_strdup (NULL, path)); dir = dirname (dirpart); @@ -810,7 +830,8 @@ len2 = strlen (dir); if (strcspn (dir, GLOB_CHARS) < len2) { - nih_warn ("%s: %s", job->path, _("Directory globbing not supported")); + nih_warn ("%s: %s", job->path, + _("Directory globbing not supported")); return; } @@ -898,44 +919,6 @@ } /** - * file_filter: - * - * @dir: WatchedDir, - * @path: full path to file to consider, - * @is_dir: TRUE if @path is a directory, else FALSE. - * - * Watch handler function to sift the wheat from the chaff. - * - * Returns: TRUE if @path should be ignored, FALSE otherwise. - **/ -int -file_filter (WatchedDir *dir, - const char *path, - int is_dir) -{ - nih_assert (dir); - nih_assert (path); - - skip_slashes (path); - - NIH_HASH_FOREACH_SAFE (dir->files, iter) { - WatchedFile *file = (WatchedFile *)iter; - - if (strstr (file->path, path) == file->path) { - /* Either an exact match or path is a child of the watched file. - * Paths in the latter category will be inspected more closely by - * the handlers. - */ - return FALSE; - } else if ((file->dir || file->glob) && strstr (path, file->path) == path) { - return FALSE; - } - } - - return TRUE; -} - -/** * create_handler: * * @dir: WatchedDir, @@ -955,6 +938,7 @@ char *p; int add_dir = FALSE; int empty; + int ignored = TRUE; /* Hash of events already emitted (required to avoid sending * same event multiple times). @@ -972,6 +956,10 @@ nih_assert (path); nih_assert (statbuf); + nih_debug ("Considering new %s path '%s'", + S_ISDIR (statbuf->st_mode) ? "directory" : "file", + path); + skip_slashes (path); /* path should be a file below the WatchedDir */ @@ -981,62 +969,71 @@ handled = NIH_MUST (nih_hash_string_new (NULL, 0)); NIH_HASH_FOREACH_SAFE (dir->files, iter) { - WatchedFile *file = (WatchedFile *)iter; + WatchedFile *file = (WatchedFile *)iter; if (file->dir) { if (! strcmp (file->path, dir->path)) { /* Watch is on the directory itself and a file within that - * watched directory was created, hence emit the _directory_ + * watched directory was created, hence signal the _directory_ * was modified. */ - if (file->events & IN_MODIFY) - handle_event (handled, original_path (file), IN_MODIFY, path); + if (file->events & IN_MODIFY) { + handle_event (handled, file, IN_MODIFY, path); + + ignored = FALSE; + } } else if (! strcmp (file->path, path)) { /* Directory has been created */ - handle_event (handled, original_path (file), IN_CREATE, NULL); + handle_event (handled, file, IN_CREATE, NULL); + add_dir = TRUE; + ignored = FALSE; nih_list_add (&entries, &file->entry); } } else if (file->glob) { - nih_local char *full_path = NULL; - - /* reconstruct the full path */ - full_path = NIH_MUST (nih_sprintf (NULL, "%s/%s", file->path, file->glob)); - - if (! fnmatch (full_path, path, FNM_PATHNAME) && (file->events & IN_CREATE)) - handle_event (handled, full_path, IN_CREATE, path); + if (handle_glob (file, path, handled, IN_CREATE)) + ignored = FALSE; } else { + /* Basic file watch */ + if (! strcmp (file->path, path) && (file->events & IN_CREATE)) { /* exact match, so emit event */ - handle_event (handled, file->path, IN_CREATE, NULL); + handle_event (handled, file, IN_CREATE, NULL); - } else if ((p=strstr (file->path, path)) && p == file->path - && S_ISDIR (statbuf->st_mode)) { - /* The created file is actually a directory - * more specific that the current watch - * directory associated with @file. - * - * As such, we can make the watch on @file more - * specific by dropping the old watch, creating - * a new WatchedDir for @path and adding @file - * to the new WatchedDir's files hash. - * - * This has to be handled carefully due to NIH - * list/hash handling constraints. First, the - * new directory is marked as needing to be - * added to the directory hash and secondly we - * add the WatchedFile to a list representing - * all WatchedFiles that need to be added for - * the new path. - */ - add_dir = TRUE; - nih_list_add (&entries, &file->entry); + ignored = FALSE; } } + + if ((p=strstr (file->path, path)) && p == file->path + && S_ISDIR (statbuf->st_mode)) { + /* The created file is actually a directory + * more specific that the current watch + * directory associated with @file. + * + * As such, we can make the watch on @file more + * specific by dropping the old watch, creating + * a new WatchedDir for @path and adding @file + * to the new WatchedDir's files hash. + * + * This has to be handled carefully due to NIH + * list/hash handling constraints. First, the + * new directory is marked as needing to be + * added to the directory hash and secondly we + * add the WatchedFile to a list representing + * all WatchedFiles that need to be added for + * the new path. + */ + add_dir = TRUE; + nih_list_add (&entries, &file->entry); + } } - if (! add_dir) + if (! add_dir) { + nih_debug ("%s path '%s'", + ! ignored ? "Handled" : "Ignored", + path); return; + } /* we should have atleast 1 file to add to the new watch */ nih_assert (! NIH_LIST_EMPTY (&entries)); @@ -1048,6 +1045,8 @@ return; } + nih_debug ("Creating watch on more specific path '%s'", path); + /* Add all list entries to the newly-created WatchedDir */ NIH_LIST_FOREACH_SAFE (&entries, iter) { WatchedFile *file = (WatchedFile *)iter; @@ -1063,6 +1062,7 @@ if (empty) { /* Remove the old directory watch */ + nih_debug ("Removing old watch on path '%s'", dir->path); nih_free (dir); } } @@ -1084,6 +1084,7 @@ struct stat *statbuf) { nih_local NihHash *handled = NULL; + int ignored = TRUE; nih_assert (dir); nih_assert (watch); @@ -1093,12 +1094,16 @@ /* path should be a file below the WatchedDir */ nih_assert (strstr (path, dir->path) == path); + nih_debug ("Considering modified %s path '%s'", + S_ISDIR (statbuf->st_mode) ? "directory" : "file", + path); + skip_slashes (path); handled = NIH_MUST (nih_hash_string_new (NULL, 0)); NIH_HASH_FOREACH_SAFE (dir->files, iter) { - WatchedFile *file = (WatchedFile *)iter; + WatchedFile *file = (WatchedFile *)iter; if (! (file->events & IN_MODIFY)) continue; @@ -1109,26 +1114,35 @@ * watched directory was modified, hence emit the _directory_ * was modified. */ - handle_event (handled, original_path (file), IN_MODIFY, path); + handle_event (handled, file, IN_MODIFY, path); + + ignored = FALSE; } } else if (file->glob) { - nih_local char *full_path = NULL; - - /* reconstruct the full path */ - full_path = NIH_MUST (nih_sprintf (NULL, "%s/%s", file->path, file->glob)); - - if (! fnmatch (full_path, path, FNM_PATHNAME) && (file->events & IN_MODIFY)) - handle_event (handled, full_path, IN_MODIFY, path); + if (handle_glob (file, path, handled, IN_MODIFY)) + ignored = FALSE; } else { + /* Basic file watch */ + if (! strcmp (file->path, path)) { /* exact match, so emit event */ - handle_event (handled, original_path (file), IN_MODIFY, NULL); + handle_event (handled, file, IN_MODIFY, NULL); + + ignored = FALSE; } else if (file->dir && strstr (path, file->path) == path) { /* file in watched directory modified, so emit event */ - handle_event (handled, path, IN_MODIFY, NULL); + + handle_event (handled, file, IN_MODIFY, NULL); + + ignored = FALSE; } } } + + nih_debug ("%s path '%s'", + (! ignored) ? "Handled" : "Ignored", + path); + } /** @@ -1151,6 +1165,7 @@ struct stat statbuf; int rm_dir = FALSE; nih_local NihHash *handled = NULL; + int ignored = TRUE; /* List of existing WatchedFiles that need to be added against * @path (since @path either exactly matches their path, or @@ -1165,64 +1180,79 @@ /* path should be a file below the WatchedDir */ nih_assert (strstr (path, dir->path) == path); + nih_debug ("Considering deleted path '%s'", path); + skip_slashes (path); nih_list_init (&entries); handled = NIH_MUST (nih_hash_string_new (NULL, 0)); NIH_HASH_FOREACH_SAFE (dir->files, iter) { - WatchedFile *file = (WatchedFile *)iter; + WatchedFile *file = (WatchedFile *)iter; if (file->dir) { if (! strcmp (file->path, path)) { /* Directory itself was deleted */ - handle_event (handled, original_path (file), IN_DELETE, NULL); + handle_event (handled, file, IN_DELETE, NULL); + + ignored = FALSE; } else if (! strcmp (file->path, dir->path)) { /* Watch is on the directory itself and a file within that * watched directory was deleted, hence emit the directory was * modified. */ - if (file->events & IN_MODIFY) - handle_event (handled, original_path (file), IN_MODIFY, path); + if (file->events & IN_MODIFY) { + handle_event (handled, file, IN_MODIFY, path); + + ignored = FALSE; + } } } else if (file->glob) { - nih_local char *full_path = NULL; - - /* reconstruct the full path */ - full_path = NIH_MUST (nih_sprintf (NULL, "%s/%s", file->path, file->glob)); - - if (! fnmatch (full_path, path, FNM_PATHNAME) && (file->events & IN_DELETE)) - handle_event (handled, full_path, IN_DELETE, path); + if (handle_glob (file, path, handled, IN_DELETE)) + ignored = FALSE; } else { + /* Basic file watch */ + if (! strcmp (file->path, path) && (file->events & IN_DELETE)) { - handle_event (handled, original_path (file), IN_DELETE, NULL); - } else if ((p=strstr (file->path, path)) && p == file->path) { - /* Create a new directory watch for all - * WatchedFiles whose immediate parent directory - * matches @path (in other words, - * make the watch looking after a WatchedFile - * less specific). This has to be handled - * carefully due to NIH list/hash handling - * constraints. First, the new directory is - * marked as needing to be added to the - * directory hash and secondly we add the - * WatchedFile to a list representing all - * WatchedFiles that need to be added for the - * new path. - */ - rm_dir = TRUE; - nih_list_add (&entries, &file->entry); - } else if (file->dir && strstr (path, file->path) == path && (file->events & IN_DELETE)) { + handle_event (handled, file, IN_DELETE, NULL); + + ignored = FALSE; + } + } + + if ((p=strstr (file->path, path)) && p == file->path) { + /* Create a new directory watch for all + * WatchedFiles whose immediate parent directory + * matches @path (in other words, + * make the watch looking after a WatchedFile + * less specific). This has to be handled + * carefully due to NIH list/hash handling + * constraints. First, the new directory is + * marked as needing to be added to the + * directory hash and secondly we add the + * WatchedFile to a list representing all + * WatchedFiles that need to be added for the + * new path. + */ + rm_dir = TRUE; + nih_list_add (&entries, &file->entry); + } else if (file->dir && strstr (path, file->path) == path + && (file->events & IN_DELETE)) { /* file in watched directory deleted, so emit event */ - handle_event (handled, path, IN_DELETE, NULL); - } + handle_event (handled, file, IN_DELETE, NULL); + + ignored = FALSE; } } - if (! rm_dir) + if (! rm_dir) { + nih_debug ("%s path '%s'", + ! ignored ? "Handled" : "Ignored", + path); return; + } - /* Remove the old directory watch */ + nih_debug ("Removing old watch on path '%s'", dir->path); nih_free (dir); nih_assert (! NIH_LIST_EMPTY (&entries)); @@ -1252,6 +1282,8 @@ _("Failed to watch directory"), parent); return; } + + nih_debug ("Creating watch on less specific path '%s'", new_dir->path); } /* Add all list entries to the newly-created WatchedDir. */ @@ -1345,6 +1377,28 @@ entry = NIH_MUST (nih_list_entry_new (job)); entry->data = file; nih_list_add (&job->files, &entry->entry); + + /* Show the path the job specified, not the actual path since + * that could confuse. + */ + if (file->glob) { + nih_debug ("Watching %s glob '%s/%s' via '%s'", + file->dir ? "directory" : "file", + + file->original + ? file->original + : file->path, + + file->glob, + path); + } else { + nih_debug ("Watching %s '%s' via '%s'", + file->dir ? "directory" : "file", + file->original + ? file->original + : file->path, + path); + } } /** @@ -1352,7 +1406,7 @@ * * Initialise the watched_dirs hash table. **/ -void +static void watched_dir_init (void) { if (! watched_dirs) @@ -1384,9 +1438,16 @@ event_type == IN_MODIFY || event_type == IN_DELETE); + nih_debug ("Emitting %s event for path '%s'", + event_type == IN_CREATE ? "create" : + event_type == IN_MODIFY ? "modify" : + "delete", + path); + env = NIH_MUST (nih_str_array_new (NULL)); var = NIH_MUST (nih_sprintf (NULL, "FILE=%s", path)); + NIH_MUST (nih_str_array_addp (&env, NULL, &env_len, var)); var = NIH_MUST (nih_sprintf (NULL, "EVENT=%s", @@ -1514,7 +1575,7 @@ */ dir->watch = nih_watch_new (dir, watched_path, FALSE, TRUE, - (NihFileFilter)file_filter, + NULL, (NihCreateHandler)create_handler, (NihModifyHandler)modify_handler, (NihDeleteHandler)delete_handler, @@ -1798,31 +1859,46 @@ * handle_event: * * @handled: hash of FileEvents already handled, - * @file_event: FileEvent to consider. + * @file: WatchedFile to handle the event for, + * @event: FileEvent to consider, + * @match: full path to file that event refers to. * * Determine if @file_event has already been handled; if not emit the * event and record its details in @handled. **/ static void -handle_event (NihHash *handled, - const char *path, - uint32_t event, - const char *match) +handle_event (NihHash *handled, + WatchedFile *file, + uint32_t event, + const char *match) { - FileEvent *file_event; + FileEvent *file_event; + nih_local char *original = NULL; nih_assert (handled); - nih_assert (path); + nih_assert (file); nih_assert (event); - file_event = (FileEvent *)nih_hash_search (handled, path, NULL); + original = original_path (NULL, file); + + if (! original) { + nih_warn (_("Failed to determine original path")); + return; + } + + file_event = (FileEvent *)nih_hash_search (handled, original, NULL); while (file_event) { if ((file_event->event & event) && string_match (file_event->match, match)) { + nih_debug ("Ignoring %s event for already-handled path '%s'", + event == IN_CREATE ? "create" : + event == IN_MODIFY ? "modify" : + "delete", + original); return; } - file_event = (FileEvent *)nih_hash_search (handled, path, + file_event = (FileEvent *)nih_hash_search (handled, original, &file_event->entry); } @@ -1831,10 +1907,68 @@ /* Event has not yet been handled, so emit it and record fact * it's now been handled. */ - file_event = NIH_MUST (file_event_new (handled, path, event, match)); + file_event = NIH_MUST (file_event_new (handled, original, event, match)); nih_hash_add (handled, &file_event->entry); - emit_event (path, event, match); + if (file->original && file->original[0] == '~') { + /* glob paths that also need to undergo expansion need + * careful handling. + */ + if (file->glob) { + nih_local char *emit_path = NULL; + + emit_path = NIH_MUST (nih_sprintf (NULL, "%s/%s", + file->original, file->glob)); + + emit_event (emit_path, event, match); + } else { + emit_event (file->original, event, match); + } + } else { + emit_event (original, event, match); + } +} + +/** + * handle_glob: + * + * @file: WatchedFile that contains a glob, + * @path: Full path to file (contains no globs), + * @handled: hash of FileEvent's, + * @event: file event type. + * + * Emit an event if @path matches the WatchedFile @file which must have + * a glob set. + * + * Returns: TRUE if an event was emitted, else FALSE. + **/ +static int +handle_glob (WatchedFile *file, + const char *path, + NihHash *handled, + uint32_t event) +{ + nih_local char *original = NULL; + + nih_assert (file); + nih_assert (file->glob); + nih_assert (path); + nih_assert (handled); + nih_assert (event); + + original = original_path (NULL, file); + + if (! original) { + nih_warn (_("Failed to determine original path")); + return FALSE; + } + + if (! fnmatch (original, path, FNM_PATHNAME) && (file->events & event)) { + handle_event (handled, file, event, path); + return TRUE; + } + + return FALSE; } /** @@ -1863,6 +1997,51 @@ } /** + * file_exists: + * @path: file to check. + * + * Determine if specified file exists. + * + * Returns: TRUE if @path exists, else FALSE. + **/ +static int +file_exists (const char *path) +{ + struct stat st; + + nih_assert (path); + + return ! stat (path, &st); +} + +/** + * remove_trailing_slashes: + * + * @path: path. + * + * Remove contiguous runs of slashes from the end of @path. + **/ +static void +remove_trailing_slashes (char *path) +{ + size_t len; + char *end; + + assert (path); + + len = strlen (path); + assert (len > 0); + + end = path + (len-1); + + while (*end == '/') { + *end = '\0'; + end--; + } +} + + +/** * expand_path: * * @parent: parent, @@ -1875,11 +2054,13 @@ * * Returns: Newly-allocated fully-expanded path, or NULL on error. **/ -char * +static char * expand_path (const void *parent, const char *path) { - char *new; - const char *p; + char *new; + const char *p; + const char *e; + nih_local char *path_env = NULL; nih_assert (path); @@ -1894,12 +2075,25 @@ /* absolute path so nothing to do */ nih_assert (path[0] != '/'); + e = getenv ("HOME"); + + if (e) { + path_env = NIH_MUST (nih_strdup (NULL, e)); + remove_trailing_slashes (path_env); + } + p = path; if (strstr (path, "~/") == path || strstr (path, "./") == path) p += 2; - new = nih_sprintf (parent, "%s/%s", home_dir, p); + /* Note that $HOME is only honoured if it exists. + */ + new = nih_sprintf (parent, "%s/%s", + path_env && file_exists (path_env) + ? path_env + : home_dir, + p); return new; } @@ -1951,3 +2145,48 @@ return TRUE; } + +/** + * original_path: + * + * @parent: parent for returned string. + * @file: WatchedFile. + * + * Obtain the appropriate WatchedFile path. + * + * - If the watched file contains a glob, the returned path will be + * + * the original if the + * path underwent expansion or is a directory, else the initial + * unexpanded path. + * + * Required for emitting events since jobs need the unexpanded path to + * allow their start/stop condition to match even if the path has + * subsequently been expanded by this bridge. + * + * Returns: newly-allocated string representing of original path. + **/ +static char * +original_path (void *parent, const WatchedFile *file) +{ + char *path = NULL; + + nih_assert (file); + + if (file->original && file->original[0] == '~') { + path = nih_strdup (parent, file->path); + } else if (file->glob) { + path = nih_sprintf (parent, "%s/%s", + file->original + ? file->original + : file->path, + file->glob); + } else { + path = nih_strdup (parent, + file->original + ? file->original + : file->path); + } + + return path; +} === modified file 'scripts/pyupstart.py' --- scripts/pyupstart.py 2014-04-04 14:16:52 +0000 +++ scripts/pyupstart.py 2014-09-16 19:04:34 +0000 @@ -1,5 +1,25 @@ #!/usr/bin/python3 -#--------------------------------------------------------------------- +# -*- coding: utf-8 -*- +# -------------------------------------------------------------------- +# Copyright © 2013-2014 Canonical Ltd. +# +# Author: James Hunt <[email protected]> +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License version 2, as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, write to the Free Software Foundation, Inc., +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +# -------------------------------------------------------------------- + +# -------------------------------------------------------------------- # = Limitations = # # - Override files are not currently supported. @@ -8,7 +28,7 @@ # where to create the override but be aware that after creating a # '.override' file, you must wait until Upstart has re-parsed the job. # -#--------------------------------------------------------------------- +# -------------------------------------------------------------------- """ Upstart test module. @@ -55,12 +75,12 @@ SESSION_DIR_FMT = 'upstart/sessions' -BUS_NAME = 'com.ubuntu.Upstart' -INTERFACE_NAME = 'com.ubuntu.Upstart0_6' -JOB_INTERFACE_NAME = 'com.ubuntu.Upstart0_6.Job' -INSTANCE_INTERFACE_NAME = 'com.ubuntu.Upstart0_6.Instance' -OBJECT_PATH = '/com/ubuntu/Upstart' -FREEDESKTOP_PROPERTIES = 'org.freedesktop.DBus.Properties' +BUS_NAME = 'com.ubuntu.Upstart' +INTERFACE_NAME = 'com.ubuntu.Upstart0_6' +JOB_INTERFACE_NAME = 'com.ubuntu.Upstart0_6.Job' +INSTANCE_INTERFACE_NAME = 'com.ubuntu.Upstart0_6.Instance' +OBJECT_PATH = '/com/ubuntu/Upstart' +FREEDESKTOP_PROPERTIES = 'org.freedesktop.DBus.Properties' # Maximum number of seconds to wait for Upstart to detect a new job # has been created @@ -79,7 +99,6 @@ # Maximum number of seconds to wait for Upstart to create a logfile LOGFILE_WAIT_SECS = 5 -#--------------------------------------------------------------------- def get_init(): """ @@ -93,6 +112,7 @@ assert (os.path.exists(binary)) return binary + def get_initctl(): """ Return full path to an appropriate initctl binary. @@ -105,6 +125,7 @@ assert (os.path.exists(binary)) return binary + def get_file_bridge(): """ Return full path to an appropriate upstart-file-bridge binary. @@ -117,6 +138,7 @@ assert (os.path.exists(binary)) return binary + def dbus_encode(str): """ Simulate nih_dbus_path() which Upstart uses to convert @@ -288,7 +310,7 @@ """ self.new_job = Job(self, self.test_dir, self.test_dir_name, - name, body=body, retain=self.retain) + name, body=body, retain=self.retain) # deregister return False @@ -517,7 +539,7 @@ ) self.new_job = Job(self, self.test_dir, self.test_dir_name, - name, body=None, reuse_path=conf_path) + name, body=None, reuse_path=conf_path) self.jobs.append(self.new_job) return self.new_job @@ -540,7 +562,7 @@ """ def __init__(self, upstart, dir_name, subdir_name, job_name, - body=None, reuse_path=None, retain=False): + body=None, reuse_path=None, retain=False): """ @upstart: Upstart() parent object. @dir_name: Full path to job configuration files directory. @@ -591,8 +613,8 @@ if not self.body and self.reuse_path: # Just check conf file exists if not os.path.exists(self.conffile): - raise UpstartException( - 'File {} does not exist for reuse'.format(self.conffile)) + raise UpstartException('File {} does not exist for reuse' + .format(self.conffile)) else: # Create conf file with open(self.conffile, 'w', encoding='utf-8') as fh: @@ -970,6 +992,7 @@ self.stop() self.logfile.destroy() + class SystemInit(Upstart): def __init__(self): @@ -1009,8 +1032,9 @@ args = [get_initctl(), 'list-sessions'] - for line in subprocess.check_output(args, - universal_newlines=True).splitlines(): + lines = subprocess.check_output(args, + universal_newlines=True).splitlines() + for line in lines: pid, socket = line.split() sessions[pid] = socket @@ -1076,9 +1100,10 @@ self.log_dir = \ tempfile.mkdtemp(prefix="%s-logdir-%d-" % (NAME, pid)) - args.extend([init_binary, '--user', - '--confdir', self.conf_dir, - '--logdir', self.log_dir]) + args.extend([init_binary, + '--user', + '--confdir', self.conf_dir, + '--logdir', self.log_dir]) if extra: args.extend(extra) @@ -1091,7 +1116,8 @@ mask = pyinotify.IN_CREATE notifier = \ pyinotify.Notifier(watch_manager, InotifyHandler()) - session = os.path.join(os.environ['XDG_RUNTIME_DIR'], SESSION_DIR_FMT) + session = os.path.join(os.environ['XDG_RUNTIME_DIR'], + SESSION_DIR_FMT) watch_manager.add_watch(session, mask) notifier.process_events() === modified file 'scripts/tests/test_pyupstart_session_init.py' --- scripts/tests/test_pyupstart_session_init.py 2014-03-05 10:41:18 +0000 +++ scripts/tests/test_pyupstart_session_init.py 2014-09-16 19:04:34 +0000 @@ -1,6 +1,6 @@ #!/usr/bin/python3 # -*- coding: utf-8 -*- -#--------------------------------------------------------------------- +# -------------------------------------------------------------------- # Copyright 2013 Canonical Ltd. # # Author: James Hunt <[email protected]> @@ -17,15 +17,15 @@ # You should have received a copy of the GNU General Public License along # with this program; if not, write to the Free Software Foundation, Inc., # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. -#--------------------------------------------------------------------- +# -------------------------------------------------------------------- -#--------------------------------------------------------------------- +# -------------------------------------------------------------------- # Description: Session-level Upstart tests for the pyupstart module. # # Notes: Should be run both as a non-privileged user and then again # as the root user; in both cases, an Upstart user session # will be created. -#--------------------------------------------------------------------- +# -------------------------------------------------------------------- import os import sys @@ -45,6 +45,7 @@ import unittest + class TestSessionUpstart(unittest.TestCase): FILE_BRIDGE_CONF = 'upstart-file-bridge.conf' @@ -59,27 +60,41 @@ # variable since this is required by the Session Init. xdg_runtime_dir = os.environ.get('XDG_RUNTIME_DIR', None) if not xdg_runtime_dir or not os.path.exists(xdg_runtime_dir): - tmp_xdg_runtime_dir = tempfile.mkdtemp(prefix='tmp-xdg-runtime-dir') + tmp_xdg_runtime_dir = \ + tempfile.mkdtemp(prefix='tmp-xdg-runtime-dir') os.environ['XDG_RUNTIME_DIR'] = tmp_xdg_runtime_dir - print('INFO: User has no XDG_RUNTIME_DIR so created one: {}'.format(tmp_xdg_runtime_dir)) - - - self.file_bridge_conf = '{}{}{}'.format(bridge_session_conf_dir, os.sep, self.FILE_BRIDGE_CONF) - self.reexec_conf = '{}{}{}'.format(bridge_session_conf_dir, os.sep, self.REEXEC_CONF) + print('INFO: User has no XDG_RUNTIME_DIR so created one: {}' + .format(tmp_xdg_runtime_dir)) + + self.file_bridge_conf = '{}{}{}'.format(bridge_session_conf_dir, + os.sep, + self.FILE_BRIDGE_CONF) + + self.reexec_conf = '{}{}{}'.format(bridge_session_conf_dir, + os.sep, + self.REEXEC_CONF) # Prefer to use the installed job files if available and the # appropriate environment variable is set since they are going # to be more current and appropriate for the environment under # test. if os.environ.get('UPSTART_TEST_USE_INSTALLED_CONF', None): - tmp = '{}{}{}'.format(DEFAULT_SESSION_INSTALL_PATH, os.sep, self.FILE_BRIDGE_CONF) + tmp = '{}{}{}'.format(DEFAULT_SESSION_INSTALL_PATH, + os.sep, + self.FILE_BRIDGE_CONF) if os.path.exists(tmp): - print('INFO: UPSTART_TEST_USE_INSTALLED_CONF set - using {} rather than {}'.format(tmp, self.file_bridge_conf)) + print('INFO: UPSTART_TEST_USE_INSTALLED_CONF set - ' + 'using {} rather than {}' + .format(tmp, self.file_bridge_conf)) self.file_bridge_conf = tmp - tmp = '{}{}{}'.format(DEFAULT_SESSION_INSTALL_PATH, os.sep, self.REEXEC_CONF) + tmp = '{}{}{}'.format(DEFAULT_SESSION_INSTALL_PATH, + os.sep, + self.REEXEC_CONF) if os.path.exists(tmp): - print('INFO: UPSTART_TEST_USE_INSTALLED_CONF set - using {} rather than {}'.format(tmp, self.reexec_conf)) + print('INFO: UPSTART_TEST_USE_INSTALLED_CONF set - ' + 'using {} rather than {}' + .format(tmp, self.reexec_conf)) self.reexec_conf = tmp self.assertTrue(os.path.exists(self.file_bridge_conf)) @@ -135,24 +150,50 @@ self.upstart = None + class TestFileBridge(TestSessionUpstart): def test_init_start_file_bridge(self): self.start_session_init() + # Set $HOME to a temporary directory to allow us to create jobs + # that contain "~"'s that will be expanded as expected but which + # do not need to live under the users real home directory. + fake_home_dir = tempfile.mkdtemp() + + args = [get_initctl(), + 'set-env', + '--global', + 'HOME={}'.format(fake_home_dir)] + + subprocess.check_call(args) + + args = [get_initctl(), 'get-env', '--global', 'HOME'] + + # check Upstart is now using the fake $HOME value + lines = subprocess.check_output(args, + universal_newlines=True).splitlines() + + for line in lines: + assert(line == fake_home_dir) + self.logger.warning('Set \'HOME={}\' for testing' + .format(fake_home_dir)) + # Create upstart-file-bridge.conf # # Note that we do not use the bundled user job due to our # requirement for a different start condition and different # command options. - cmd = '{} --daemon --user --debug'.format(get_file_bridge()) + # + # Also, we run in the foreground to ensure debug output is + # captured. + cmd = '{} --user --debug'.format(get_file_bridge()) lines = """ start on startup stop on session-end emits file - expect daemon respawn exec {} """.format(cmd) @@ -171,46 +212,136 @@ os.kill(pid, 0) target_dir = tempfile.mkdtemp() - file = target_dir + os.sep + 'foo' - dir = target_dir + os.sep + 'bar' + + self.logger.warning('Session Init file watch directory: {}' + .format(target_dir)) + + file = target_dir + os.sep + 'foo.bar' + file2 = target_dir + os.sep + 'hello.baz' + glob_file = target_dir + os.sep + '*.baz' + dir = target_dir + os.sep + 'qux' + + tilde_file = '~' + os.sep + 'foo.bar' + tilde_file_expanded = fake_home_dir + os.sep + 'foo.bar' + + tilde_glob_file = '~' + os.sep + '*.baz' + tilde_glob_file_expanded = fake_home_dir + os.sep + 'glob.baz' + + tilde_dir = '~' + dir + + # We remove all the special characters that are used in the + # 'start on' stanza to avoid upstart detecting them as shell + # meta-characters and passing the exec command through a shell + # (which will then perform expansion and defeat our simple + # string checking). + + vars_msg = r"FILE='$FILE', EVENT='$EVENT', MATCH='$MATCH'" + + self.logger.warning('Session Init conf directory: {}' + .format(self.upstart.conf_dir)) + + self.logger.warning('Session Init log directory: {}' + .format(self.upstart.log_dir)) + + # ----------------------------------------------------------- + # file jobs + + # ----------------------------- + # create jobs # Create a job that makes use of the file event to watch a # file in a newly-created directory. - file_msg = 'got file %s' % file lines = [] lines.append('start on file FILE=%s EVENT=create' % file) - lines.append('exec echo %s' % file_msg) - create_file_job = self.upstart.job_create('wait-for-file-creation', lines) + lines.append('exec echo "%s"' % vars_msg) + create_file_job = self.upstart.job_create('file-create', lines) self.assertTrue(create_file_job) + # glob create file job + lines = [] + lines.append('start on file FILE=%s EVENT=create' % glob_file) + lines.append('exec echo "%s"' % vars_msg) + create_glob_file_job = \ + self.upstart.job_create('glob-file-job-create', lines) + self.assertTrue(create_glob_file_job) + + # tilde create file job + lines = [] + lines.append('start on file FILE=%s EVENT=create' % tilde_file) + lines.append('exec echo "%s"' % vars_msg) + tilde_file_create_job = \ + self.upstart.job_create('tilde-file-job-create', lines) + self.assertTrue(tilde_file_create_job) + + # tilde glob create file job + lines = [] + lines.append('start on file FILE=%s EVENT=create' % tilde_glob_file) + lines.append('exec echo "%s"' % vars_msg) + tilde_glob_file_create_job = \ + self.upstart.job_create('tilde-glob-file-job-create', lines) + self.assertTrue(tilde_glob_file_create_job) + + # ----------------------------- + # modify jobs + # Create job that waits for a file modification lines = [] lines.append('start on file FILE=%s EVENT=modify' % file) - lines.append('exec echo %s' % file_msg) - modify_file_job = self.upstart.job_create('wait-for-file-modify', lines) + lines.append('exec echo "%s"' % vars_msg) + modify_file_job = self.upstart.job_create('file-modify', lines) self.assertTrue(modify_file_job) + # glob modify file job + lines = [] + lines.append('start on file FILE=%s EVENT=modify' % glob_file) + lines.append('exec echo "%s"' % vars_msg) + modify_glob_file_job = \ + self.upstart.job_create('glob-file-job-modify', lines) + self.assertTrue(modify_glob_file_job) + + # tilde modify file job + lines = [] + lines.append('start on file FILE=%s EVENT=modify' % tilde_file) + lines.append('exec echo "%s"' % vars_msg) + tilde_file_modify_job = \ + self.upstart.job_create('tilde-file-job-modify', lines) + self.assertTrue(tilde_file_modify_job) + + # tilde glob modify file job + lines = [] + lines.append('start on file FILE=%s EVENT=modify' % tilde_glob_file) + lines.append('exec echo "%s"' % vars_msg) + tilde_glob_file_modify_job = \ + self.upstart.job_create('tilde-glob-file-job-modify', lines) + self.assertTrue(tilde_glob_file_modify_job) + + # ----------------------------- + # delete jobs + # Create another job that triggers when the same file is deleted lines = [] lines.append('start on file FILE=%s EVENT=delete' % file) - lines.append('exec echo %s' % file_msg) - delete_file_job = self.upstart.job_create('wait-for-file-deletion', lines) + lines.append('exec echo "%s"' % vars_msg) + delete_file_job = self.upstart.job_create('file-delete', lines) self.assertTrue(delete_file_job) + # ----------------------------------------------------------- + # directory jobs + # Create job that triggers on directory creation - dir_msg = 'got directory %s' % dir lines = [] # XXX: note the trailing slash to force a directory watch lines.append('start on file FILE=%s/ EVENT=create' % dir) - lines.append('exec echo %s' % dir_msg) - create_dir_job = self.upstart.job_create('wait-for-dir-creation', lines) + lines.append('exec echo "%s"' % vars_msg) + create_dir_job = \ + self.upstart.job_create('wait-for-dir-creation', lines) self.assertTrue(create_dir_job) # Create job that triggers on directory modification lines = [] # XXX: note the trailing slash to force a directory watch lines.append('start on file FILE=%s/ EVENT=modify' % dir) - lines.append('exec echo %s' % dir_msg) + lines.append('exec echo "%s"' % vars_msg) modify_dir_job = self.upstart.job_create('wait-for-dir-modify', lines) self.assertTrue(modify_dir_job) @@ -218,38 +349,72 @@ lines = [] # XXX: note the trailing slash to force a directory watch lines.append('start on file FILE=%s/ EVENT=delete' % dir) - lines.append('exec echo %s' % dir_msg) + lines.append('exec echo "%s"' % vars_msg) delete_dir_job = self.upstart.job_create('wait-for-dir-delete', lines) self.assertTrue(delete_dir_job) - # Create empty file - open(file, 'w').close() + # ----------------------------------------------------------- + # Trigger file bridge + + # Create file + with open(file, 'w') as f: + f.write('created\n') # Create directory os.mkdir(dir) + # ----------------------------------------------------------- # No need to start the jobs of course as the file-bridge handles that! # Identify full path to job logfiles create_file_job_logfile = create_file_job.logfile_name(dbus_encode('')) self.assertTrue(create_file_job_logfile) - modify_file_job_logfile = modify_file_job.logfile_name(dbus_encode('')) + create_glob_file_job_logfile = \ + create_glob_file_job.logfile_name(dbus_encode('')) + self.assertTrue(create_glob_file_job_logfile) + + create_tilde_file_job_logfile = \ + tilde_file_create_job.logfile_name(dbus_encode('')) + self.assertTrue(create_tilde_file_job_logfile) + + create_tilde_glob_file_job_logfile = \ + tilde_glob_file_create_job.logfile_name(dbus_encode('')) + self.assertTrue(create_tilde_glob_file_job_logfile) + + modify_file_job_logfile = \ + modify_file_job.logfile_name(dbus_encode('')) self.assertTrue(modify_file_job_logfile) - delete_file_job_logfile = delete_file_job.logfile_name(dbus_encode('')) + modify_glob_file_job_logfile = \ + modify_glob_file_job.logfile_name(dbus_encode('')) + self.assertTrue(modify_glob_file_job_logfile) + + tilde_file_modify_job_logfile = \ + tilde_file_modify_job.logfile_name(dbus_encode('')) + self.assertTrue(tilde_file_modify_job_logfile) + + tilde_glob_file_modify_job_logfile = \ + tilde_glob_file_modify_job.logfile_name(dbus_encode('')) + self.assertTrue(tilde_glob_file_modify_job_logfile) + + delete_file_job_logfile = \ + delete_file_job.logfile_name(dbus_encode('')) self.assertTrue(delete_file_job_logfile) - create_dir_job_logfile = create_dir_job.logfile_name(dbus_encode('')) + create_dir_job_logfile = \ + create_dir_job.logfile_name(dbus_encode('')) self.assertTrue(create_dir_job_logfile) - modify_dir_job_logfile = modify_dir_job.logfile_name(dbus_encode('')) + modify_dir_job_logfile = \ + modify_dir_job.logfile_name(dbus_encode('')) self.assertTrue(modify_dir_job_logfile) - delete_dir_job_logfile = delete_dir_job.logfile_name(dbus_encode('')) + delete_dir_job_logfile = \ + delete_dir_job.logfile_name(dbus_encode('')) self.assertTrue(delete_dir_job_logfile) - #-------------------- + # ------------------- # Wait for the create file job to run and produce output self.assertTrue(wait_for_file(create_file_job_logfile)) @@ -258,9 +423,73 @@ with open(create_file_job_logfile, 'r', encoding='utf-8') as f: lines = f.readlines() self.assertTrue(len(lines) == 1) - self.assertEqual(file_msg, lines[0].rstrip()) - - #-------------------- + expected = "FILE='{}', EVENT='{}', MATCH='{}'" \ + .format(file, 'create', '') + self.assertTrue(expected == lines[0].rstrip()) + + # ----------------------------------------------------------- + + # Create 2nd empty file + with open(file2, 'w') as f: + f.write('created\n') + + # ----------------------------------------------------------- + + # Wait for the tilde create file job to run and produce output + self.assertTrue(wait_for_file(create_glob_file_job_logfile)) + + # Check the output + with open(create_glob_file_job_logfile, 'r', encoding='utf-8') as f: + lines = f.readlines() + self.assertTrue(len(lines) == 1) + + expected = "FILE='{}', EVENT='{}', MATCH='{}'" \ + .format(glob_file, 'create', file2) + self.assertTrue(expected == lines[0].rstrip()) + + # ----------------------------------------------------------- + + # create tilde file + with open(tilde_file_expanded, 'w') as f: + f.write('created\n') + + # ----------------------------------------------------------- + + # Wait for the tilde create file job to run and produce output + self.assertTrue(wait_for_file(create_tilde_file_job_logfile)) + + # Check the output + with open(create_tilde_file_job_logfile, 'r', encoding='utf-8') as f: + lines = f.readlines() + self.assertTrue(len(lines) == 1) + + expected = "FILE='{}', EVENT='{}', MATCH='{}'" \ + .format(tilde_file, 'create', '') + self.assertTrue(expected == lines[0].rstrip()) + + # ----------------------------------------------------------- + + # create tilde glob file + with open(tilde_glob_file_expanded, 'w') as f: + f.write('created\n') + + # ----------------------------------------------------------- + + # Wait for the tilde glob create file job to run and produce output + self.assertTrue(wait_for_file(create_tilde_glob_file_job_logfile)) + + # Check the output + with open(create_tilde_glob_file_job_logfile, + 'r', + encoding='utf-8') as f: + lines = f.readlines() + self.assertTrue(len(lines) == 1) + + expected = "FILE='{}', EVENT='{}', MATCH='{}'" \ + .format(tilde_glob_file, 'create', tilde_glob_file_expanded) + self.assertTrue(expected == lines[0].rstrip()) + + # ------------------- # Wait for the create directory job to run and produce output self.assertTrue(wait_for_file(create_dir_job_logfile)) @@ -269,12 +498,17 @@ with open(create_dir_job_logfile, 'r', encoding='utf-8') as f: lines = f.readlines() self.assertTrue(len(lines) == 1) - self.assertEqual(dir_msg, lines[0].rstrip()) - - #-------------------- + + expected = "FILE='{}', EVENT='{}', MATCH='{}'" \ + .format(dir + '/', 'create', '') + + self.assertTrue(expected == lines[0].rstrip()) + + # ------------------- # Modify the file - open(file, 'w').close() + with open(file, 'a') as f: + f.write('appended\n') # Wait for the create file job to run and produce output self.assertTrue(wait_for_file(modify_file_job_logfile)) @@ -283,11 +517,59 @@ with open(modify_file_job_logfile, 'r', encoding='utf-8') as f: lines = f.readlines() self.assertTrue(len(lines) == 1) - self.assertEqual(file_msg, lines[0].rstrip()) - - #-------------------- + expected = "FILE='{}', EVENT='{}', MATCH='{}'" \ + .format(file, 'modify', '') + self.assertTrue(expected == lines[0].rstrip()) + + with open(file2, 'a') as f: + f.write('appended\n') + + self.assertTrue(wait_for_file(modify_glob_file_job_logfile)) + + # Check the output + with open(modify_glob_file_job_logfile, 'r', encoding='utf-8') as f: + lines = f.readlines() + self.assertTrue(len(lines) == 1) + + expected = "FILE='{}', EVENT='{}', MATCH='{}'" \ + .format(glob_file, 'modify', file2) + self.assertTrue(expected == lines[0].rstrip()) + + # modify tilde file + with open(tilde_file_expanded, 'a') as f: + f.write('modified\n') + + self.assertTrue(wait_for_file(tilde_file_modify_job_logfile)) + + # Check the output + with open(tilde_file_modify_job_logfile, 'r', encoding='utf-8') as f: + lines = f.readlines() + self.assertTrue(len(lines) == 1) + + expected = "FILE='{}', EVENT='{}', MATCH='{}'" \ + .format(tilde_file, 'modify', '') + self.assertTrue(expected == lines[0].rstrip()) + + # create tilde glob file + with open(tilde_glob_file_expanded, 'a') as f: + f.write('modified\n') + + self.assertTrue(wait_for_file(tilde_glob_file_modify_job_logfile)) + + # Check the output + with open(tilde_glob_file_modify_job_logfile, + 'r', + encoding='utf-8') as f: + lines = f.readlines() + self.assertTrue(len(lines) == 1) + + expected = "FILE='{}', EVENT='{}', MATCH='{}'" \ + .format(tilde_glob_file, 'modify', tilde_glob_file_expanded) + self.assertTrue(expected == lines[0].rstrip()) + + # ------------------- + # Modify the directory by creating a new file in it. - dir_file = dir + os.sep + 'baz' open(dir_file, 'w').close() @@ -298,9 +580,12 @@ with open(modify_dir_job_logfile, 'r', encoding='utf-8') as f: lines = f.readlines() self.assertTrue(len(lines) == 1) - self.assertEqual(dir_msg, lines[0].rstrip()) - - #-------------------- + + expected = "FILE='{}', EVENT='{}', MATCH='{}'" \ + .format(dir + '/', 'modify', dir_file) + self.assertTrue(expected == lines[0].rstrip()) + + # ------------------- os.remove(dir_file) os.rmdir(dir) @@ -312,8 +597,12 @@ with open(delete_dir_job_logfile, 'r', encoding='utf-8') as f: lines = f.readlines() self.assertTrue(len(lines) == 1) - self.assertEqual(dir_msg, lines[0].rstrip()) - #-------------------- + + expected = "FILE='{}', EVENT='{}', MATCH='{}'" \ + .format(dir + '/', 'delete', '') + self.assertTrue(expected == lines[0].rstrip()) + + # ------------------- shutil.rmtree(target_dir) @@ -324,13 +613,31 @@ with open(delete_file_job_logfile, 'r', encoding='utf-8') as f: lines = f.readlines() self.assertTrue(len(lines) == 1) - self.assertEqual(file_msg, lines[0].rstrip()) - - #-------------------- + + expected = "FILE='{}', EVENT='{}', MATCH='{}'" \ + .format(file, 'delete', '') + self.assertTrue(expected == lines[0].rstrip()) + + # ------------------- + + if os.environ.get('UPSTART_TEST_NO_CLEANUP'): + sys.exit("UPSTART_TEST_NO_CLEANUP set\n" + "Files left in '{}' and '{}'" + .format(self.upstart.conf_dir, + self.upstart.log_dir)) os.remove(create_file_job_logfile) + os.remove(create_glob_file_job_logfile) + os.remove(create_tilde_file_job_logfile) + os.remove(create_tilde_glob_file_job_logfile) + os.remove(modify_file_job_logfile) + os.remove(modify_glob_file_job_logfile) + os.remove(tilde_file_modify_job_logfile) + os.remove(tilde_glob_file_modify_job_logfile) + os.remove(delete_file_job_logfile) + os.remove(create_dir_job_logfile) os.remove(modify_dir_job_logfile) os.remove(delete_dir_job_logfile) @@ -338,6 +645,7 @@ file_bridge.stop() self.stop_session_init() + class TestSessionInitReExec(TestSessionUpstart): def test_session_init_reexec(self): @@ -487,6 +795,7 @@ self.stop_session_init() + class TestSessionInit(TestSessionUpstart): def test_session_init(self): @@ -508,6 +817,7 @@ inst.stop() self.stop_session_init() + class TestState(TestSessionUpstart): def test_state(self): @@ -539,6 +849,7 @@ self.assertEqual(path, full_job_path) self.stop_session_init() + def main(): kwargs = {} format = \ === modified file 'scripts/tests/test_pyupstart_system_init.py' --- scripts/tests/test_pyupstart_system_init.py 2014-03-10 13:43:50 +0000 +++ scripts/tests/test_pyupstart_system_init.py 2014-09-16 19:04:34 +0000 @@ -1,6 +1,6 @@ #!/usr/bin/python3 # -*- coding: utf-8 -*- -#--------------------------------------------------------------------- +# -------------------------------------------------------------------- # Copyright © 2013 Canonical Ltd. # # Author: James Hunt <[email protected]> @@ -17,13 +17,13 @@ # You should have received a copy of the GNU General Public License along # with this program; if not, write to the Free Software Foundation, Inc., # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. -#--------------------------------------------------------------------- +# -------------------------------------------------------------------- -#--------------------------------------------------------------------- +# -------------------------------------------------------------------- # Description: System-level Upstart tests for the pyupstart module. # # Notes: Can only be run as the root user. -#--------------------------------------------------------------------- +# -------------------------------------------------------------------- import os import sys @@ -40,6 +40,7 @@ import unittest + class TestSystemUpstart(unittest.TestCase): def setUp(self): if os.geteuid(): @@ -52,86 +53,92 @@ def tearDown(self): # Ensure no state file exists - state_file = '{}{}{}'.format(SYSTEM_LOG_DIR, os.sep, UPSTART_STATE_FILE) + state_file = '{}{}{}'.format(SYSTEM_LOG_DIR, + os.sep, + UPSTART_STATE_FILE) self.assertFalse(os.path.exists(state_file)) + class TestSystemInitReExec(TestSystemUpstart): def test_pid1_reexec(self): - version = self.upstart.version() - self.assertTrue(version) - - # Create an invalid job to ensure this causes no problems for - # the re-exec. Note that we cannot use job_create() since - # that validates the syntax of the .conf file). - # - # We create this file before any other to allow time for Upstart - # to _attempt to parse it_ by the time the re-exec is initiated. - invalid_conf = "{}/invalid.conf".format(self.upstart.test_dir) - with open(invalid_conf, 'w', encoding='utf-8') as fh: - print("invalid", file=fh) - - # create a job and start it, marking it such that the .conf file - # will be retained when object becomes unusable (after re-exec). - job = self.upstart.job_create('connected-job', ['exec upstart-udev-bridge', 'respawn'], retain=True) - self.assertTrue(job) - - # Used when recreating the job - conf_path = job.conffile - - inst = job.start() - self.assertTrue(inst) - pids = job.pids() - self.assertEqual(len(pids), 1) - pid = pids['main'] - - self.upstart.reexec() - - # PID 1 Upstart is now in the process of starting, but we need to - # reconnect to it via D-Bus since it cannot yet retain client - # connections. However, since the re-exec won't be instantaneous, - # try a few times. - self.upstart.polling_connect(force=True) - - # Since the parent job was created with 'retain', this is actually - # a NOP but is added to denote that the old instance is dead. - inst.destroy() - - # check that we can still operate on the re-exec'd Upstart - version_postexec = self.upstart.version() - self.assertTrue(version_postexec) - self.assertEqual(version, version_postexec) - - # Ensure the job is still running with the same PID - self.assertRaises(ProcessLookupError, os.kill, pid, 0) - - # XXX: The re-exec will have severed the D-Bus connection to - # Upstart. Hence, revivify the job with some magic. - job = self.upstart.job_recreate('connected-job', conf_path) - self.assertTrue(job) - - # Recreate the instance - inst = job.get_instance() - self.assertTrue(inst) - - self.assertTrue(job.running('_')) - pids = job.pids() - self.assertEqual(len(pids), 1) - self.assertTrue(pids['main']) - - # The pid _must_ have changed after a restart - self.assertNotEqual(pid, pids['main']) - - job.stop() - - # Ensure the pid has gone - with self.assertRaises(ProcessLookupError): - os.kill(pid, 0) - - os.remove(invalid_conf) - - # Clean up - self.upstart.destroy() + version = self.upstart.version() + self.assertTrue(version) + + # Create an invalid job to ensure this causes no problems for + # the re-exec. Note that we cannot use job_create() since + # that validates the syntax of the .conf file). + # + # We create this file before any other to allow time for Upstart + # to _attempt to parse it_ by the time the re-exec is initiated. + invalid_conf = "{}/invalid.conf".format(self.upstart.test_dir) + with open(invalid_conf, 'w', encoding='utf-8') as fh: + print("invalid", file=fh) + + # create a job and start it, marking it such that the .conf file + # will be retained when object becomes unusable (after re-exec). + job = self.upstart.job_create('connected-job', + ['exec upstart-udev-bridge', 'respawn'], + retain=True) + self.assertTrue(job) + + # Used when recreating the job + conf_path = job.conffile + + inst = job.start() + self.assertTrue(inst) + pids = job.pids() + self.assertEqual(len(pids), 1) + pid = pids['main'] + + self.upstart.reexec() + + # PID 1 Upstart is now in the process of starting, but we need to + # reconnect to it via D-Bus since it cannot yet retain client + # connections. However, since the re-exec won't be instantaneous, + # try a few times. + self.upstart.polling_connect(force=True) + + # Since the parent job was created with 'retain', this is actually + # a NOP but is added to denote that the old instance is dead. + inst.destroy() + + # check that we can still operate on the re-exec'd Upstart + version_postexec = self.upstart.version() + self.assertTrue(version_postexec) + self.assertEqual(version, version_postexec) + + # Ensure the job is still running with the same PID + self.assertRaises(ProcessLookupError, os.kill, pid, 0) + + # XXX: The re-exec will have severed the D-Bus connection to + # Upstart. Hence, revivify the job with some magic. + job = self.upstart.job_recreate('connected-job', conf_path) + self.assertTrue(job) + + # Recreate the instance + inst = job.get_instance() + self.assertTrue(inst) + + self.assertTrue(job.running('_')) + pids = job.pids() + self.assertEqual(len(pids), 1) + self.assertTrue(pids['main']) + + # The pid _must_ have changed after a restart + self.assertNotEqual(pid, pids['main']) + + job.stop() + + # Ensure the pid has gone + with self.assertRaises(ProcessLookupError): + os.kill(pid, 0) + + os.remove(invalid_conf) + + # Clean up + self.upstart.destroy() + class TestSystemInitChrootSession(TestSystemUpstart): CHROOT_ENVVAR = 'UPSTART_TEST_CHROOT_PATH' @@ -140,7 +147,8 @@ chroot_path = os.environ.get(self.CHROOT_ENVVAR, None) if not chroot_path: - raise unittest.SkipTest('{} variable not set'.format(self.CHROOT_ENVVAR)) + raise unittest.SkipTest('{} variable not set' + .format(self.CHROOT_ENVVAR)) # Ensure the chroot exists self.assertTrue(os.path.exists(chroot_path)) @@ -167,6 +175,7 @@ self.upstart.polling_connect(force=True) self.assertTrue(self.upstart.version()) + def main(): kwargs = {} format = \
-- upstart-devel mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/upstart-devel
