From 0d0b67bff759d977d992c666cb644ae7a4d46cd8 Mon Sep 17 00:00:00 2001 From: Jim Garlick Date: Wed, 30 Aug 2017 12:18:13 -0700 Subject: [PATCH 1/4] libkvs/kvs_dir: sort directories by key Change the kvsitr_t implementation to create a list of keys from the directory object and sort it, then access the list in order. --- src/common/libkvs/kvs_dir.c | 50 ++++++++++++++++++++++++++------ src/common/libkvs/test/kvs_dir.c | 10 +++++++ 2 files changed, 51 insertions(+), 9 deletions(-) diff --git a/src/common/libkvs/kvs_dir.c b/src/common/libkvs/kvs_dir.c index 8ba29c68cb88..0f1d701104a4 100644 --- a/src/common/libkvs/kvs_dir.c +++ b/src/common/libkvs/kvs_dir.c @@ -44,8 +44,8 @@ struct kvsdir_struct { }; struct kvsdir_iterator_struct { - json_t *dirdata; - void *iter; + zlist_t *keys; + bool reset; }; void kvsdir_incref (kvsdir_t *dir) @@ -140,18 +140,45 @@ const char *kvsdir_rootref (kvsdir_t *dir) void kvsitr_destroy (kvsitr_t *itr) { if (itr) { + int saved_errno = errno; + zlist_destroy (&itr->keys); free (itr); + errno = saved_errno; } } +static int sort_cmp (void *item1, void *item2) +{ + if (!item1 && item2) + return -1; + if (!item1 && !item2) + return 0; + if (item1 && !item2) + return 1; + return strcmp (item1, item2); +} + kvsitr_t *kvsitr_create (kvsdir_t *dir) { - kvsitr_t *itr; + kvsitr_t *itr = NULL; + const char *key; + json_t *dirdata, *value; + if (!dir) { + errno = EINVAL; + goto error; + } if (!(itr = calloc (1, sizeof (*itr)))) goto error; - itr->dirdata = treeobj_get_data (dir->dirobj); - itr->iter = json_object_iter (itr->dirdata); + if (!(itr->keys = zlist_new ())) + goto error; + dirdata = treeobj_get_data (dir->dirobj); + json_object_foreach (dirdata, key, value) { + if (zlist_push (itr->keys, (char *)key) < 0) + goto error; + } + zlist_sort (itr->keys, sort_cmp); + itr->reset = true; return itr; error: kvsitr_destroy (itr); @@ -160,16 +187,21 @@ kvsitr_t *kvsitr_create (kvsdir_t *dir) void kvsitr_rewind (kvsitr_t *itr) { - itr->iter = json_object_iter (itr->dirdata); + if (itr) + itr->reset = true; } const char *kvsitr_next (kvsitr_t *itr) { const char *name = NULL; - if (itr->iter) { - name = json_object_iter_key (itr->iter); - itr->iter = json_object_iter_next (itr->dirdata, itr->iter); + if (itr) { + if (itr->reset) + name = zlist_first (itr->keys); + else + name = zlist_next (itr->keys); + if (name) + itr->reset = false; } return name; } diff --git a/src/common/libkvs/test/kvs_dir.c b/src/common/libkvs/test/kvs_dir.c index cc51f5704d76..8a34ff3677b3 100644 --- a/src/common/libkvs/test/kvs_dir.c +++ b/src/common/libkvs/test/kvs_dir.c @@ -87,6 +87,16 @@ void test_empty (void) ok (kvsdir_get_size (dir) == 0, "kvsdir_get_size returns zero"); + errno = 0; + ok (kvsitr_create (NULL) == NULL && errno == EINVAL, + "kvsitr_create with NULL dir fails with EINVAL"); + ok (kvsitr_next (NULL) == NULL, + "kvsitr_next on NULL iterator returns NULL"); + lives_ok ({kvsitr_rewind (NULL);}, + "kvsitr_rewind on NULL iterator doesn't crash"); + lives_ok ({kvsitr_destroy (NULL);}, + "kvsitr_destroy on NULL iterator doesn't crash"); + itr = kvsitr_create (dir); ok (itr != NULL, "kvsitr_create works"); From a509291e9b5580cd8c2d03c73ad0f9ee744179eb Mon Sep 17 00:00:00 2001 From: Jim Garlick Date: Fri, 25 Aug 2017 04:09:34 +0000 Subject: [PATCH 2/4] cmd/flux-kvs: add ls subcommand Add new subcommand: flux kvs ls [-R] [-d] [-w COLS] [-1|-C] [-F] [key...] This subcommand mimics ls(1) behavior and options, as discussed in #1158. --- src/cmd/Makefile.am | 1 + src/cmd/flux-kvs.c | 345 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 346 insertions(+) diff --git a/src/cmd/Makefile.am b/src/cmd/Makefile.am index 013b711c334a..64b56831c287 100644 --- a/src/cmd/Makefile.am +++ b/src/cmd/Makefile.am @@ -10,6 +10,7 @@ AM_CPPFLAGS = \ $(ZMQ_CFLAGS) fluxcmd_ldadd = \ + $(top_builddir)/src/common/libkvs/libkvs.la \ $(top_builddir)/src/common/libflux-internal.la \ $(top_builddir)/src/common/libflux-core.la \ $(top_builddir)/src/common/libflux-optparse.la \ diff --git a/src/cmd/flux-kvs.c b/src/cmd/flux-kvs.c index b984e93a56f6..ead773843a64 100644 --- a/src/cmd/flux-kvs.c +++ b/src/cmd/flux-kvs.c @@ -25,15 +25,18 @@ #if HAVE_CONFIG_H #include "config.h" #endif +#include #include #include #include #include #include +#include #include "src/common/libutil/xzmalloc.h" #include "src/common/libutil/log.h" #include "src/common/libutil/readall.h" +#include "src/common/libkvs/treeobj.h" int cmd_get (optparse_t *p, int argc, char **argv); int cmd_put (optparse_t *p, int argc, char **argv); @@ -48,9 +51,12 @@ int cmd_dropcache (optparse_t *p, int argc, char **argv); int cmd_copy (optparse_t *p, int argc, char **argv); int cmd_move (optparse_t *p, int argc, char **argv); int cmd_dir (optparse_t *p, int argc, char **argv); +int cmd_ls (optparse_t *p, int argc, char **argv); static void dump_kvs_dir (kvsdir_t *dir, bool Ropt, bool dopt); +#define min(a,b) ((a)<(b)?(a):(b)) + static struct optparse_option dir_opts[] = { { .name = "recursive", .key = 'R', .has_arg = 0, .usage = "Recursively display keys under subdirectories", @@ -61,6 +67,25 @@ static struct optparse_option dir_opts[] = { OPTPARSE_TABLE_END }; +static struct optparse_option ls_opts[] = { + { .name = "recursive", .key = 'R', .has_arg = 0, + .usage = "List directory recursively", + }, + { .name = "directory", .key = 'd', .has_arg = 0, + .usage = "List directory instead of contents", + }, + { .name = "width", .key = 'w', .has_arg = 1, + .usage = "Set output width to COLS. 0 means no limit", + }, + { .name = "1", .key = '1', .has_arg = 0, + .usage = "Force one entry per line", + }, + { .name = "classify", .key = 'F', .has_arg = 0, + .usage = "Append indicator (one of .@) to entries", + }, + OPTPARSE_TABLE_END +}; + static struct optparse_option watch_opts[] = { { .name = "recursive", .key = 'R', .has_arg = 0, .usage = "Recursively display keys under subdirectories", @@ -116,6 +141,13 @@ static struct optparse_subcommand subcommands[] = { 0, dir_opts }, + { "ls", + "[-R] [-d] [-F] [-w COLS] [-1] [key...]", + "List directory", + cmd_ls, + 0, + ls_opts + }, { "unlink", "key [key...]", "Remove key", @@ -816,6 +848,319 @@ int cmd_dir (optparse_t *p, int argc, char **argv) return (0); } +/* Find longest name in a directory. + */ +static int get_dir_maxname (kvsdir_t *dir) +{ + kvsitr_t *itr; + const char *name; + int max = 0; + + if (!(itr = kvsitr_create (dir))) + log_err_exit ("kvsitr_create"); + while ((name = kvsitr_next (itr))) + if (max < strlen (name)) + max = strlen (name); + kvsitr_destroy (itr); + return max; +} + +/* Find longest name in a list of names. + */ +static int get_list_maxname (zlist_t *names) +{ + const char *name; + int max = 0; + + name = zlist_first (names); + while (name) { + if (max < strlen (name)) + max = strlen (name); + name = zlist_next (names); + } + return max; +} + +/* Determine appropriate terminal window width for columnar output. + * The options -w COLS or -1 force the width to COLS or 1, respectively. + * If not forced, ask the tty how wide it is. If not on a tty, try the + * COLUMNS environment variable. If not set, assume 80. + * N.B. A width of 0 means "unlimited", while a width of 1 effectively + * forces one entry per line since entries are never broken across lines. + */ +static int get_window_width (optparse_t *p, int fd) +{ + struct winsize w; + int width; + const char *s; + + if (optparse_hasopt (p, "1")) + return 1; + if ((width = optparse_get_int (p, "width", -1)) >= 0) + return width; + if (ioctl (fd, TIOCGWINSZ, &w) == 0) + return w.ws_col; + if ((s = getenv ("COLUMNS")) && (width = strtol (s, NULL, 10)) >= 0) + return width; + return 80; +} + +/* Return true if at character position 'col', there is insufficient + * room for one more column of 'col_width' without exceeding 'win_width'. + * Exceptions: 1) win_width of 0 means "unlimited"; 2) col must be > 0. + * This is a helper for list_kvs_dir_single() and list_kvs_keys(). + */ +static bool need_newline (int col, int col_width, int win_width) +{ + return (win_width != 0 && col + col_width > win_width && col > 0); +} + +/* List the content of 'dir', arranging output in columns that fit 'win_width', + * and using a custom column width selected based on the longest entry name. + */ +static void list_kvs_dir_single (kvsdir_t *dir, int win_width, optparse_t *p) +{ + kvsitr_t *itr; + const char *name; + int col_width = get_dir_maxname (dir) + 4; + int col = 0; + char *namebuf; + int last_len = 0; + + if (!(namebuf = malloc (col_width + 2))) + log_err_exit ("malloc"); + if (!(itr = kvsitr_create (dir))) + log_err_exit ("kvsitr_create"); + while ((name = kvsitr_next (itr))) { + if (need_newline (col, col_width, win_width)) { + printf ("\n"); + col = 0; + } + else if (last_len > 0) // pad out last entry to col_width + printf ("%*s", col_width - last_len, ""); + strcpy (namebuf, name); + if (optparse_hasopt (p, "classify")) { + if (kvsdir_isdir (dir, name)) + strcat (namebuf, "."); + else if (kvsdir_issymlink (dir, name)) + strcat (namebuf, "@"); + } + printf ("%s", namebuf); + last_len = strlen (namebuf); + col += col_width; + } + if (col > 0) + printf ("\n"); + kvsitr_destroy (itr); + free (namebuf); +} + +/* List contents of directory pointed to by 'key', descending into subdirs + * if -R was specified. First the directory is listed, then its subdirs. + */ +static void list_kvs_dir (flux_t *h, const char *key, optparse_t *p, + int win_width, bool print_label, bool print_vspace) +{ + flux_future_t *f; + kvsdir_t *dir = NULL; + kvsitr_t *itr; + const char *name, *json_str; + + if (!(f = flux_kvs_lookup (h, FLUX_KVS_READDIR, key)) + || flux_kvs_lookup_get (f, &json_str) < 0) { + log_err_exit ("%s", key); + goto done; + } + if (!(dir = kvsdir_create (h, NULL, key, json_str))) + log_err_exit ("kvsdir_create"); + + if (print_label) + printf ("%s%s:\n", print_vspace ? "\n" : "", key); + list_kvs_dir_single (dir, win_width, p); + + if (optparse_hasopt (p, "recursive")) { + if (!(itr = kvsitr_create (dir))) + log_err_exit ("kvsitr_create"); + while ((name = kvsitr_next (itr))) { + if (kvsdir_isdir (dir, name)) { + char *nkey; + if (!(nkey = kvsdir_key_at (dir, name))) { + log_err ("%s: kvsdir_key_at failed", name); + continue; + } + list_kvs_dir (h, nkey, p, win_width, print_label, true); + free (nkey); + } + } + kvsitr_destroy (itr); + } +done: + kvsdir_destroy (dir); + flux_future_destroy (f); +} + +/* List keys, arranging in columns so that they fit within 'win_width', + * and using a custom column width selected based on the longest entry name. + * The keys are popped off the list and freed. + */ +static void list_kvs_keys (zlist_t *singles, int win_width) +{ + char *name; + int col_width = get_list_maxname (singles) + 4; + int col = 0; + int last_len = 0; + + while ((name = zlist_pop (singles))) { + if (need_newline (col, col_width, win_width)) { + printf ("\n"); + col = 0; + } + else if (last_len > 0) // pad out last entry to col_width + printf ("%*s", col_width - last_len, ""); + printf ("%s", name); + last_len = strlen (name); + col += col_width; + free (name); + } + if (col > 0) + printf ("\n"); +} + +/* for zlist_sort() + */ +static int sort_cmp (void *item1, void *item2) +{ + if (!item1 && item2) + return -1; + if (!item1 && !item2) + return 0; + if (item1 && !item2) + return 1; + return strcmp (item1, item2); +} + +/* Put key in 'dirs' or 'singles' list, depending on whether + * its contents are to be listed or not. If -F is specified, + * 'singles' key names are decorated based on their type. + */ +static int categorize_key (optparse_t *p, const char *key, + zlist_t *dirs, zlist_t *singles) +{ + flux_t *h = (flux_t *)optparse_get_data (p, "flux_handle"); + flux_future_t *f; + const char *json_str; + char *nkey; + json_t *treeobj = NULL; + bool require_directory = false; + + if (!(nkey = malloc (strlen (key) + 2))) // room for decoration char + null + log_err_exit ("malloc"); + strcpy (nkey, key); + + /* If the key has a "." suffix, strip it off, but require + * that the key be a directory type. + */ + while (strlen (nkey) > 1 && nkey[strlen (nkey) - 1] == '.') { + nkey[strlen (nkey) - 1] = '\0'; + require_directory = true; + } + if (!(f = flux_kvs_lookup (h, FLUX_KVS_TREEOBJ, nkey))) + log_err_exit ("flux_kvs_lookup"); + if (flux_kvs_lookup_get (f, &json_str) < 0) { + fprintf (stderr, "%s: %s\n", nkey, flux_strerror (errno)); + goto error; + } + if (!(treeobj = treeobj_decode (json_str))) + log_err_exit ("%s: metadata decode error", key); + if (treeobj_is_dir (treeobj) || treeobj_is_dirref (treeobj)) { + if (optparse_hasopt (p, "directory")) { + if (optparse_hasopt (p, "classify")) + strcat (nkey, "."); + if (zlist_append (singles, nkey) < 0) + log_err_exit ("zlist_append"); + } + else + if (zlist_append (dirs, nkey) < 0) + log_err_exit ("zlist_append"); + } + else if (treeobj_is_val (treeobj) || treeobj_is_valref (treeobj)) { + if (require_directory) { + fprintf (stderr, "%s: Not a directory\n", nkey); + goto error; + } + if (zlist_append (singles, xstrdup (key)) < 0) + log_err_exit ("zlist_append"); + } + else if (treeobj_is_symlink (treeobj)) { + if (require_directory) { + fprintf (stderr, "%s: Not a directory\n", nkey); + goto error; + } + if (optparse_hasopt (p, "classify")) + strcat (nkey, "@"); + if (zlist_append (singles, nkey) < 0) + log_err_exit ("zlist_append"); + } + zlist_sort (singles, sort_cmp); + zlist_sort (dirs, sort_cmp); + json_decref (treeobj); + flux_future_destroy (f); + return 0; +error: + free (nkey); + json_decref (treeobj); + flux_future_destroy (f); + return -1; +} + +/* Mimic ls(1). + * Strategy: sort keys from command line into two lists: 'dirs' for + * directories to be expanded, and 'singles' for others. + * First display the singles, then display directories, optionally recursing. + * Assume "." if no keys were specified on the command line. + */ +int cmd_ls (optparse_t *p, int argc, char **argv) +{ + flux_t *h = (flux_t *)optparse_get_data (p, "flux_handle"); + int optindex = optparse_option_index (p); + int win_width = get_window_width (p, STDOUT_FILENO); + zlist_t *dirs, *singles; + char *key; + bool print_label = false; // print "name:" before directory contents + bool print_vspace = false; // print vertical space before label + int rc = 0; + + if (!(dirs = zlist_new ()) || !(singles = zlist_new ())) + log_err_exit ("zlist_new"); + + if (optindex == argc) { + if (categorize_key (p, ".", dirs, singles) < 0) + rc = -1; + } + while (optindex < argc) { + if (categorize_key (p, argv[optindex++], dirs, singles) < 0) + rc = -1; + } + if (zlist_size (singles) > 0) { + list_kvs_keys (singles, win_width); + print_label = true; + print_vspace = true; + } + + if (optparse_hasopt (p, "recursive") || zlist_size (dirs) > 1) + print_label = true; + while ((key = zlist_pop (dirs))) { + list_kvs_dir (h, key, p, win_width, print_label, print_vspace); + print_vspace = true; + free (key); + } + + zlist_destroy (&dirs); + zlist_destroy (&singles); + + return rc; +} + int cmd_copy (optparse_t *p, int argc, char **argv) { flux_t *h = (flux_t *)optparse_get_data (p, "flux_handle"); From 282bcf6c476b41dcb3069a77940d0f61ff1b47f3 Mon Sep 17 00:00:00 2001 From: Jim Garlick Date: Mon, 28 Aug 2017 07:15:36 -0700 Subject: [PATCH 3/4] doc/flux-kvs(1): Add ls subcommand. --- doc/man1/flux-kvs.adoc | 8 ++++++++ src/cmd/flux-kvs.c | 1 - 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/doc/man1/flux-kvs.adoc b/doc/man1/flux-kvs.adoc index 6ccb44ea35e8..aa5f85330ba3 100644 --- a/doc/man1/flux-kvs.adoc +++ b/doc/man1/flux-kvs.adoc @@ -50,6 +50,14 @@ Retrieve the value stored under 'key'. If nothing has been stored under Store 'value' under 'key' and commit it. If it already has a value, overwrite it. +*ls* [-R] [-d] [-F] [-w COLS] [-1] ['key' ...]:: +Display directory referred to by _key_, or "." (root) if unspecified. +Options are roughly equivalent to a subset of ls(1) options. +'-R' lists directory recursively. '-d' displays directory not its contents. +'-F' classifies files with one character suffix (. is directory, @ is symlink). +'-w COLS' sets the terminal width in characters. '-1' causes output to be +displayed in one column. + *dir* [-R] [-d] ['key']:: Display all keys and their values under the directory 'key'. If 'key' does not exist or is not a directory, display an error message. diff --git a/src/cmd/flux-kvs.c b/src/cmd/flux-kvs.c index ead773843a64..8a96339baa95 100644 --- a/src/cmd/flux-kvs.c +++ b/src/cmd/flux-kvs.c @@ -25,7 +25,6 @@ #if HAVE_CONFIG_H #include "config.h" #endif -#include #include #include #include From 38549520a3c224929fbb328a58111abd5840c2a9 Mon Sep 17 00:00:00 2001 From: Jim Garlick Date: Tue, 29 Aug 2017 15:15:26 -0700 Subject: [PATCH 4/4] t/t1000-kvs.t: cover "flux ls" --- t/t1000-kvs.t | 117 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 117 insertions(+) diff --git a/t/t1000-kvs.t b/t/t1000-kvs.t index 58db2bb4e702..2cbe4f7d9ecc 100755 --- a/t/t1000-kvs.t +++ b/t/t1000-kvs.t @@ -278,6 +278,123 @@ EOF test_cmp expected output ' +# +# ls tests +# +test_expect_success 'kvs: ls -1F DIR works' ' + flux kvs unlink -Rf $DIR && + flux kvs put $DIR.a=69 && + flux kvs mkdir $DIR.b && + flux kvs link b $DIR.c && + flux kvs ls -1F $DIR >output && + cat >expected <<-EOF && + a + b. + c@ + EOF + test_cmp expected output +' +test_expect_success 'kvs: ls -1Fd DIR.a DIR.b DIR.c works' ' + flux kvs unlink -Rf $DIR && + flux kvs put $DIR.a=69 && + flux kvs mkdir $DIR.b && + flux kvs link b $DIR.c && + flux kvs ls -1Fd $DIR.a $DIR.b $DIR.c >output && + cat >expected <<-EOF && + $DIR.a + $DIR.b. + $DIR.c@ + EOF + test_cmp expected output +' +test_expect_success 'kvs: ls -1RF shows directory titles' ' + flux kvs unlink -Rf $DIR && + flux kvs put $DIR.a=69 && + flux kvs put $DIR.b.d=42 && + flux kvs link b $DIR.c && + flux kvs ls -1RF $DIR | grep : | wc -l >output && + cat >expected <<-EOF && + 2 + EOF + test_cmp expected output +' +test_expect_success 'kvs: ls with no options adjusts output width to 80' ' + flux kvs unlink -Rf $DIR && + ${FLUX_BUILD_DIR}/t/kvs/dtree -p$DIR -h1 -w50 && + flux kvs ls $DIR | wc -wl >output && + cat >expected <<-EOF && + 5 50 + EOF + test_cmp expected output +' +test_expect_success 'kvs: ls -w40 adjusts output width to 40' ' + flux kvs unlink -Rf $DIR && + ${FLUX_BUILD_DIR}/t/kvs/dtree -p$DIR -h1 -w50 && + flux kvs ls -w40 $DIR | wc -wl >output && + cat >expected <<-EOF && + 10 50 + EOF + test_cmp expected output +' +test_expect_success 'kvs: ls with COLUMNS=20 adjusts output width to 20' ' + flux kvs unlink -Rf $DIR && + ${FLUX_BUILD_DIR}/t/kvs/dtree -p$DIR -h1 -w50 && + COLUMNS=20 flux kvs ls $DIR | wc -wl >output && + cat >expected <<-EOF && + 25 50 + EOF + test_cmp expected output +' +test_expect_success 'kvs: ls -R lists deep directory hierarchy' ' + flux kvs unlink -Rf $DIR && + ${FLUX_BUILD_DIR}/t/kvs/dtree -p$DIR -h8 -w1 && + flux kvs ls -R $DIR >output && + cat >expected <<-EOF && + $DIR: + 0000 + + $DIR.0000: + 0000 + + $DIR.0000.0000: + 0000 + + $DIR.0000.0000.0000: + 0000 + + $DIR.0000.0000.0000.0000: + 0000 + + $DIR.0000.0000.0000.0000.0000: + 0000 + + $DIR.0000.0000.0000.0000.0000.0000: + 0000 + + $DIR.0000.0000.0000.0000.0000.0000.0000: + 0000 + EOF + test_cmp expected output +' +test_expect_success 'kvs: ls key. works' ' + flux kvs unlink -Rf $DIR && + flux kvs mkdir $DIR.a && + flux kvs ls -d $DIR.a. >output && + cat >expected <<-EOF && + $DIR.a + EOF + test_cmp expected output +' +test_expect_success 'kvs: ls key. fails if key is not a directory' ' + flux kvs unlink -Rf $DIR && + flux kvs put $DIR.a=42 && + test_must_fail flux kvs ls -d $DIR.a. +' +test_expect_success 'kvs: ls key. fails if key does not exist' ' + flux kvs unlink -Rf $DIR && + test_must_fail flux kvs ls $DIR.a +' + # # get corner case tests #