James Hunt has proposed merging lp:~jamesodhunt/upstart/bug-1221466+1222702 into lp:upstart.
Requested reviews: Upstart Reviewers (upstart-reviewers) For more details, see: https://code.launchpad.net/~jamesodhunt/upstart/bug-1221466+1222702/+merge/184829 * extra/upstart-file-bridge.c: - create_handler(): Use original_file() for directories (LP: #1221466, #1222702). - watched_dir_new(): Additional assert. - watched_file_new(): - Additional assert. - Store original directory path in file object to ensure reliable matching for directories. * scripts/tests/test_pyupstart_session_init.py: Added file bridge tests for directory creation, modification and deletion. As expected, new tests in this branch fail with current upstart-file-bridge, but pass when using the updated version in this branch. -- https://code.launchpad.net/~jamesodhunt/upstart/bug-1221466+1222702/+merge/184829 Your team Upstart Reviewers is requested to review the proposed merge of lp:~jamesodhunt/upstart/bug-1221466+1222702 into lp:upstart.
=== modified file 'ChangeLog' --- ChangeLog 2013-08-23 14:49:09 +0000 +++ ChangeLog 2013-09-10 17:00:41 +0000 @@ -1,3 +1,16 @@ +2013-09-10 James Hunt <[email protected]> + + * extra/upstart-file-bridge.c: + - create_handler(): Use original_file() for directories + (LP: #1221466, #1222702). + - watched_dir_new(): Additional assert. + - watched_file_new(): + - Additional assert. + - Store original directory path in file object to ensure reliable + matching for directories. + * scripts/tests/test_pyupstart_session_init.py: Added file bridge tests + for directory creation, modification and deletion. + 2013-08-23 James Hunt <[email protected]> * NEWS: Release 1.10 === modified file 'extra/upstart-file-bridge.c' --- extra/upstart-file-bridge.c 2013-07-03 08:26:23 +0000 +++ extra/upstart-file-bridge.c 2013-09-10 17:00:41 +0000 @@ -177,8 +177,9 @@ * * @file: WatchedFile: * - * Obtain the appropriate WatchedFile path: either the original if the - * path underwent expansion, else the initial unexpanded path. + * 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 @@ -261,6 +262,9 @@ * @parent: parent who is watching over us. * * Details of the file being watched. + * + * For directories, @original contains the full path specified by the + * job and @path will contain @original, less any trailing slashes. **/ typedef struct watched_file { NihList entry; @@ -855,7 +859,7 @@ * * Although technically fraudulent (the file might not have _just * been created_ - it may have existed forever), it is necessary - * since otherwise jobs will hang around wating for the file to + * since otherwise jobs will hang around waiting for the file to * be 'freshly-created'. However, although nih_watch_new() has * been told to run the create handler for pre-existing files * that doesn't help as we don't watch the files, we watch @@ -986,10 +990,10 @@ * was modified. */ if (file->events & IN_MODIFY) - handle_event (handled, file->path, IN_MODIFY, path); + handle_event (handled, original_path (file), IN_MODIFY, path); } else if (! strcmp (file->path, path)) { /* Directory has been created */ - handle_event (handled, file->path, IN_CREATE, NULL); + handle_event (handled, original_path (file), IN_CREATE, NULL); add_dir = TRUE; nih_list_add (&entries, &file->entry); } @@ -1464,6 +1468,10 @@ len = strlen (watched_path); + /* Sanity-check */ + if (len <= 1) + return NULL; + if (len > 1 && watched_path[len-1] == '/') { /* Better to remove a trailing slash before handing to * inotify since although all works as expected, the @@ -1570,18 +1578,34 @@ len = strlen (path); + /* Sanity-check */ + if (len <= 1) + return NULL; + + /* Determine if the file is a directory */ file->dir = (path[len-1] == '/'); - /* optionally one or the other, but not both */ + /* Optionally one or the other, but not both */ if (file->dir || file->glob) nih_assert (file->dir || file->glob); - file->path = nih_strdup (file, path); + /* Don't store the trailing slash for directories since that + * confuses the logic and is redundant (due to file->dir). + */ + file->path = nih_strndup (file, path, file->dir ? len-1 : len); if (! file->path) goto error; file->original = NULL; - if (original) { + + if (file->dir) { + /* Retain the original directory path to simplify event + * matching. + */ + file->original = nih_strndup (file, path, len); + if (! file->original) + goto error; + } else if (original) { file->original = nih_strdup (file, original); if (! file->original) goto error; === modified file 'scripts/tests/test_pyupstart_session_init.py' --- scripts/tests/test_pyupstart_session_init.py 2013-08-06 16:54:28 +0000 +++ scripts/tests/test_pyupstart_session_init.py 2013-09-10 17:00:41 +0000 @@ -158,59 +158,170 @@ self.assertIsInstance(pid, int) os.kill(pid, 0) - # Create a job that makes use of the file event to watch to a + target_dir = tempfile.mkdtemp() + file = target_dir + os.sep + 'foo' + dir = target_dir + os.sep + 'bar' + + # Create a job that makes use of the file event to watch a # file in a newly-created directory. - dir = tempfile.mkdtemp() - file = dir + os.sep + 'foo' - - msg = 'got file %s' % file + file_msg = 'got file %s' % file lines = [] lines.append('start on file FILE=%s EVENT=create' % file) - lines.append('exec echo %s' % msg) - create_job = self.upstart.job_create('wait-for-file-creation', lines) - self.assertTrue(create_job) + lines.append('exec echo %s' % file_msg) + create_file_job = self.upstart.job_create('wait-for-file-creation', lines) + self.assertTrue(create_file_job) - # Create empty file - open(file, 'w').close() + # 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) + self.assertTrue(modify_file_job) # 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' % msg) - delete_job = self.upstart.job_create('wait-for-file-deletion', lines) - self.assertTrue(delete_job) + lines.append('exec echo %s' % file_msg) + delete_file_job = self.upstart.job_create('wait-for-file-deletion', lines) + self.assertTrue(delete_file_job) + + # 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) + 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) + modify_dir_job = self.upstart.job_create('wait-for-dir-modify', lines) + self.assertTrue(modify_dir_job) + + # Create job that triggers on directory deletion + 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) + delete_dir_job = self.upstart.job_create('wait-for-dir-delete', lines) + self.assertTrue(delete_dir_job) + + # Create empty file + open(file, 'w').close() + + # 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_job_logfile = create_job.logfile_name(dbus_encode('')) - self.assertTrue(create_job_logfile) - - delete_job_logfile = delete_job.logfile_name(dbus_encode('')) - self.assertTrue(delete_job_logfile) - - # Wait for the create job to run and produce output - self.assertTrue(wait_for_file(create_job_logfile)) - - # Check the output - with open(create_job_logfile, 'r', encoding='utf-8') as f: - lines = f.readlines() - self.assertTrue(len(lines) == 1) - self.assertEqual(msg, lines[0].rstrip()) - - shutil.rmtree(dir) + 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('')) + self.assertTrue(modify_file_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('')) + self.assertTrue(create_dir_job_logfile) + + 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('')) + 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)) + + # Check the output + 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()) + + #-------------------- + + # Wait for the create directory job to run and produce output + self.assertTrue(wait_for_file(create_dir_job_logfile)) + + # Check the output + 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()) + + #-------------------- + + # Modify the file + open(file, 'w').close() + + # Wait for the create file job to run and produce output + self.assertTrue(wait_for_file(modify_file_job_logfile)) + + # Check the output + 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()) + + #-------------------- + # Modify the directory by creating a new file in it. + + dir_file = dir + os.sep + 'baz' + open(dir_file, 'w').close() + + # Wait for the modify directory job to run and produce output + self.assertTrue(wait_for_file(modify_dir_job_logfile)) + + # Check the output + 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()) + + #-------------------- + + os.remove(dir_file) + os.rmdir(dir) + + # Wait for the delete directory job to run and produce output + self.assertTrue(wait_for_file(delete_dir_job_logfile)) + + # Check the output + 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()) + #-------------------- + + shutil.rmtree(target_dir) # Wait for the delete job to run and produce output - self.assertTrue(wait_for_file(delete_job_logfile)) + self.assertTrue(wait_for_file(delete_file_job_logfile)) # Check the output - with open(delete_job_logfile, 'r', encoding='utf-8') as f: + with open(delete_file_job_logfile, 'r', encoding='utf-8') as f: lines = f.readlines() self.assertTrue(len(lines) == 1) - self.assertEqual(msg, lines[0].rstrip()) - - os.remove(create_job_logfile) - os.remove(delete_job_logfile) + self.assertEqual(file_msg, lines[0].rstrip()) + + #-------------------- + + os.remove(create_file_job_logfile) + os.remove(modify_file_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) file_bridge.stop() self.stop_session_init()
-- upstart-devel mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/upstart-devel
