Implementing tree view for bundle-list --deps

src/3rd_party_bundle_list.c

@@ -53,8 +53,8 @@ static void print_help(void)
 	print("Options:\n");
 	print("   -R, --repo              Specify the 3rd-party repository to use\n");
 	print("   -a, --all               List all available bundles for the current version of Clear Linux\n");
-	print("   -D, --has-dep=[BUNDLE]  List all bundles which have BUNDLE as a dependency\n");
-	print("   --deps=[BUNDLE]         List bundles included by BUNDLE\n");
+	print("   -D, --has-dep=[BUNDLE]  List all bundles which have BUNDLE as a dependency (use --verbose for tree view)\n");
+	print("   --deps=[BUNDLE]         List BUNDLE dependencies (use --verbose for tree view)\n");
 	print("\n");
 }
 

src/bundle_list.c

@@ -83,8 +83,8 @@ static void print_help(void)
 
 	print("Options:\n");
 	print("   -a, --all               List all available bundles for the current version of Clear Linux\n");
-	print("   -D, --has-dep=[BUNDLE]  List all bundles which have BUNDLE as a dependency\n");
-	print("   --deps=[BUNDLE]         List bundles included by BUNDLE\n");
+	print("   -D, --has-dep=[BUNDLE]  List all bundles which have BUNDLE as a dependency (use --verbose for tree view)\n");
+	print("   --deps=[BUNDLE]         List BUNDLE dependencies (use --verbose for tree view)\n");
 	print("\n");
 }
 
@@ -209,10 +209,12 @@ static enum swupd_code list_local_bundles(int version)
 static enum swupd_code show_included_bundles(char *bundle_name, int version)
 {
 	int ret = 0;
+	struct list *bundles = NULL;
 	struct list *subs = NULL;
-	struct list *deps = NULL;
+	struct list *iter = NULL;
 	struct manifest *mom = NULL;
-	int count = 0;
+	struct sub *bundle_sub = NULL;
+	bool verbose = (log_get_level() == LOG_INFO_VERBOSE);
 
 	progress_next_step("load_manifests", PROGRESS_UNDEFINED);
 	info("Loading required manifests...\n");
@@ -224,78 +226,59 @@ static enum swupd_code show_included_bundles(char *bundle_name, int version)
 	}
 
 	// add_subscriptions takes a list, so construct one with only bundle_name
-	struct list *bundles = NULL;
 	bundles = list_prepend_data(bundles, bundle_name);
 	ret = add_subscriptions(bundles, &subs, mom, true, 0);
-	list_free_list(bundles);
 	if (ret != add_sub_NEW) {
 		// something went wrong or there were no includes, print a message and exit
-		char *m = NULL;
 		if (ret & add_sub_ERR) {
-			string_or_die(&m, "Processing error");
+			error("Cannot load included bundles\n");
 			ret = SWUPD_COULDNT_LOAD_MANIFEST;
 		} else if (ret & add_sub_BADNAME) {
 			ret = SWUPD_INVALID_BUNDLE;
 		} else {
-			string_or_die(&m, "Unknown error");
+			error("Unknown error\n");
 			ret = SWUPD_UNEXPECTED_CONDITION;
 		}
-
-		if (m) {
-			error("%s - Aborting\n", m);
-		}
-		free_string(&m);
-		goto out;
-	}
-	deps = recurse_manifest(mom, subs, NULL, false, NULL);
-	if (!deps) {
-		error("Cannot load included bundles\n");
-		ret = SWUPD_RECURSE_MANIFEST;
 		goto out;
 	}
 
 	progress_next_step("list_bundles", PROGRESS_UNDEFINED);
 
-	/* deps now includes the bundle indicated by bundle_name
-	 * if deps only has one bundle in it, no included packages were found */
-	if (list_len(deps) == 1) {
+	/* the list of subscription include the bundle indicated by bundle_name and os-core,
+	 * if deps has only two bundles in it, no included bundles were found */
+	const int MINIMAL_SUBSCRIPTIONS = 2;
+	if (strcmp(bundle_name, "os-core") == 0 || list_len(subs) <= MINIMAL_SUBSCRIPTIONS) {
 		info("\nNo included bundles\n");
 		ret = SWUPD_OK;
 		goto out;
 	}
 
 	info("\nBundles included by %s:\n", bundle_name);
 
-	struct list *iter;
-	iter = list_head(deps);
-	while (iter) {
-		struct manifest *included_bundle = iter->data;
-		iter = iter->next;
-		// deps includes the bundle_name bundle, skip it
-		if (strcmp(bundle_name, included_bundle->component) == 0) {
-			continue;
+	if (verbose) {
+		ret = subscription_get_tree(bundles, &subs, mom, true, 0);
+	} else {
+		subs = list_sort(subs, subscription_sort_component);
+		iter = list_head(subs);
+		while (iter) {
+			bundle_sub = iter->data;
+			iter = iter->next;
+			// subs include the requested bundle, skip it
+			if (strcmp(bundle_name, bundle_sub->component) == 0) {
+				continue;
+			}
+			info(" - ");
+			print("%s\n", bundle_sub->component);
 		}
-
-		info(" - ");
-		print("%s\n", included_bundle->component);
-		count++;
 	}
-	info("\nTotal: %d\n", count);
+	info("\nTotal: %d\n", list_len(subs) - 1);
 
 	ret = SWUPD_OK;
 
 out:
-	if (mom) {
-		manifest_free(mom);
-	}
-
-	if (deps) {
-		list_free_list_and_data(deps, manifest_free_data);
-	}
-
-	if (subs) {
-		free_subscriptions(&subs);
-	}
+	list_free_list(bundles);
+	manifest_free(mom);
+	free_subscriptions(&subs);
 
 	return ret;
 }

src/subscriptions.c

@@ -207,7 +207,7 @@ void create_and_append_subscription(struct list **subs, const char *component)
    2 new subscriptions
    4 bad name given
 */
-int add_subscriptions(struct list *bundles, struct list **subs, struct manifest *mom, bool find_all, int recursion)
+static int recurse_subscriptions(struct list *bundles, struct list **subs, struct manifest *mom, bool find_all, int recursion, subs_fn_t subs_fn)
 {
 	char *bundle;
 	int manifest_err;
@@ -241,33 +241,22 @@ int add_subscriptions(struct list *bundles, struct list **subs, struct manifest
 			goto out;
 		}
 
-		/*
-		 * If we're recursing a tree of includes, we need to cut out early
-		 * if the bundle we're looking at is already subscribed...
-		 * Because if it is, we'll visit it soon anyway at the top level.
-		 *
-		 * We can't do this for the toplevel of the recursion because
-		 * that is how we initiallly fill in the include tree.
-		 */
-		if (set_subscription_obligation(*subs, bundle, force_optional | is_optional)) {
-			if (recursion > 0) {
-				manifest_free(manifest);
-				continue;
-			}
-		} else {
-			// Just add it to a list if it doesn't exist
-			create_new_subscription(subs, bundle, force_optional | is_optional);
-			ret |= add_sub_NEW; /* We have added at least one */
+		if (!subs_fn(subs, bundle, recursion, force_optional | is_optional)) {
+			/* nothing else to do with the current bundle */
+			manifest_free(manifest);
+			continue;
 		}
 
+		ret |= add_sub_NEW; /* We have gone through at least one bundle */
+
 		if (!globals.skip_optional_bundles && manifest->optional) {
 			/* merge in recursive call results, all bundles added via an optional
 			 * include should be subscribed as optional */
 			if (recursion == 0) {
 				force_optional = true;
 			}
 			is_optional = true;
-			ret |= add_subscriptions(manifest->optional, subs, mom, find_all, recursion + 1);
+			ret |= recurse_subscriptions(manifest->optional, subs, mom, find_all, recursion + 1, subs_fn);
 		}
 
 		if (manifest->includes) {
@@ -276,11 +265,56 @@ int add_subscriptions(struct list *bundles, struct list **subs, struct manifest
 				force_optional = false;
 			}
 			is_optional = false;
-			ret |= add_subscriptions(manifest->includes, subs, mom, find_all, recursion + 1);
+			ret |= recurse_subscriptions(manifest->includes, subs, mom, find_all, recursion + 1, subs_fn);
 		}
 
 		manifest_free(manifest);
 	}
 out:
 	return ret;
 }
+
+static bool create_subscriptions(struct list **subs, const char *component, int recursion, bool is_optional)
+{
+	/*
+	 * If we're recursing a tree of includes, we need to cut out early
+	 * if the bundle we're looking at is already subscribed...
+	 * Because if it is, we'll visit it soon anyway at the top level.
+	 *
+	 * We can't do this for the toplevel of the recursion because
+	 * that is how we initiallly fill in the include tree.
+	 */
+	if (set_subscription_obligation(*subs, (char *)component, is_optional)) {
+		if (recursion > 0) {
+			return false;
+		}
+		return true;
+	}
+	create_new_subscription(subs, component, is_optional);
+
+	return true;
+}
+
+static bool print_subscriptions_tree(UNUSED_PARAM struct list **subs, const char *component, int recursion, UNUSED_PARAM bool is_optional)
+{
+	int indent = 0;
+
+	if (recursion == 0) {
+		info("%*s* %s\n", 2, "", component);
+	} else {
+		indent = recursion * 4;
+		info("%*s|-- %s\n", indent, "", component);
+	}
+
+	return true;
+}
+
+int add_subscriptions(struct list *bundles, struct list **subs, struct manifest *mom, bool find_all, int recursion)
+{
+	return recurse_subscriptions(bundles, subs, mom, find_all, recursion, create_subscriptions);
+}
+
+int subscription_get_tree(struct list *bundles, struct list **subs, struct manifest *mom, bool find_all, int recursion)
+{
+	return recurse_subscriptions(bundles, subs, mom, find_all, recursion, print_subscriptions_tree);
+}

src/swupd.h

@@ -321,10 +321,12 @@ extern int link_or_rename(const char *orig, const char *dest);
 extern int create_state_dirs(const char *state_dir_path);
 
 /* subscription.c */
+typedef bool (*subs_fn_t)(struct list **subs, const char *component, int recursion, bool is_optional);
 struct list *free_list_file(struct list *item);
 extern void create_and_append_subscription(struct list **subs, const char *component);
 extern char *get_tracking_dir(void);
 extern int add_subscriptions(struct list *bundles, struct list **subs, struct manifest *mom, bool find_all, int recursion);
+extern int subscription_get_tree(struct list *bundles, struct list **subs, struct manifest *mom, bool find_all, int recursion);
 extern int subscription_bundlename_strcmp(const void *a, const void *b);
 extern int subscription_sort_component(const void *a, const void *b);
 

test/functional/bundlelist/list-deps-nested.bats

@@ -17,7 +17,8 @@ test_setup() {
 @test "LST007: List bundle's dependencies when bundle has nested dependencies" {
 
 	run sudo sh -c "$SWUPD bundle-list $SWUPD_OPTS --deps test-bundle1"
-	assert_status_is 0
+
+	assert_status_is "$SWUPD_OK"
 	expected_output=$(cat <<-EOM
 		Loading required manifests...
 
@@ -31,3 +32,23 @@ test_setup() {
 	assert_is_output --identical "$expected_output"
 
 }
+
+@test "LST023: List bundle's dependencies (with nested deps) in a tree view" {
+
+	run sudo sh -c "$SWUPD bundle-list $SWUPD_OPTS --deps test-bundle1 --verbose"
+
+	assert_status_is "$SWUPD_OK"
+	expected_output=$(cat <<-EOM
+		Loading required manifests...
+
+		Bundles included by test-bundle1:
+		  * test-bundle1
+		    |-- test-bundle2
+		        |-- test-bundle3
+
+		Total: 2
+	EOM
+	)
+	assert_is_output --identical "$expected_output"
+
+}