Merge pull request #51 from jhunkeler/wheel-funcs-clean-up

Clean up get_wheel_info

4 files changed, 95 insertions, 24 deletions

diff --git a/include/wheel.h b/include/wheel.h
index 619e0f7..b394b55 100644
--- a/include/wheel.h
+++ b/include/wheel.h
@@ -5,20 +5,31 @@
 #include <string.h>
 #include <stdio.h>
 #include "str.h"
-
-#define WHEEL_MATCH_EXACT 0
-#define WHEEL_MATCH_ANY 1
+#define WHEEL_MATCH_EXACT 0 ///< Match when all patterns are present
+#define WHEEL_MATCH_ANY 1 ///< Match when any patterns are present
 
 struct Wheel {
-    char *distribution;
-    char *version;
-    char *build_tag;
-    char *python_tag;
-    char *abi_tag;
-    char *platform_tag;
-    char *path_name;
-    char *file_name;
+    char *distribution; ///< Package name
+    char *version; ///< Package version
+    char *build_tag; ///< Package build tag (optional)
+    char *python_tag; ///< Package Python tag (pyXY)
+    char *abi_tag; ///< Package ABI tag (cpXY, abiX, none)
+    char *platform_tag; ///< Package platform tag (linux_x86_64, any)
+    char *path_name; ///< Path to package on-disk
+    char *file_name; ///< Name of package on-disk
 };
 
-struct Wheel *get_wheel_file(const char *basepath, const char *name, char *to_match[], unsigned match_mode);
+/**
+ * Extract metadata from a Python Wheel file name
+ *
+ * @param basepath directory containing a wheel file
+ * @param name of wheel file
+ * @param to_match a NULL terminated array of patterns (i.e. platform, arch, version, etc)
+ * @param match_mode WHEEL_MATCH_EXACT
+ * @param match_mode WHEEL_MATCH ANY
+ * @return pointer to populated Wheel on success
+ * @return NULL on error
+ */
+struct Wheel *get_wheel_info(const char *basepath, const char *name, char *to_match[], unsigned match_mode);
+void wheel_free(struct Wheel **wheel);
 #endif //STASIS_WHEEL_H
diff --git a/src/delivery_install.c b/src/delivery_install.c
index 72f2663..6f745a7 100644
--- a/src/delivery_install.c
+++ b/src/delivery_install.c
@@ -82,18 +82,29 @@ int delivery_install_packages(struct Delivery *ctx, char *conda_install_dir, cha
                         // We can't match on version here (index 0). The wheel's version is not guaranteed to be
                         // equal to the tag; setuptools_scm auto-increments the value, the user can change it manually,
                         // etc.
-                        whl = get_wheel_file(ctx->storage.wheel_artifact_dir, info->name,
+                        errno = 0;
+                        whl = get_wheel_info(ctx->storage.wheel_artifact_dir, info->name,
                                              (char *[]) {ctx->meta.python_compact, ctx->system.arch,
                                                          "none", "any",
                                                          post_commit, hash,
                                                          NULL}, WHEEL_MATCH_ANY);
-
-                        guard_strlist_free(&tag_data);
-                        info->version = whl->version;
-                        sprintf(cmd + strlen(cmd), " '%s==%s'", info->name, whl->version);
-                    } else {
-                        sprintf(cmd + strlen(cmd), " '%s==%s'", info->name, info->version);
+                        if (!whl && errno) {
+                            // error
+                            SYSERROR("Unable to read Python wheel info: %s\n", strerror(errno));
+                            exit(1);
+                        } else if (!whl) {
+                            // not found
+                            fprintf(stderr, "No wheel packages found that match the description of '%s'", info->name);
+                        } else {
+                            // found
+                            guard_strlist_free(&tag_data);
+                            info->version = strdup(whl->version);
+                        }
+                        wheel_free(&whl);
                     }
+                    snprintf(cmd + strlen(cmd),
+                             sizeof(cmd) - strlen(cmd) - strlen(info->name) - strlen(info->version) + 5,
+                             " '%s==%s'", info->name, info->version);
                 } else {
                     fprintf(stderr, "Deferred package '%s' is not present in the tested package list!\n", name);
                     return -1;
diff --git a/src/wheel.c b/src/wheel.c
index b96df57..4692d0a 100644
--- a/src/wheel.c
+++ b/src/wheel.c
@@ -1,6 +1,6 @@
 #include "wheel.h"
 
-struct Wheel *get_wheel_file(const char *basepath, const char *name, char *to_match[], unsigned match_mode) {
+struct Wheel *get_wheel_info(const char *basepath, const char *name, char *to_match[], unsigned match_mode) {
     DIR *dp;
     struct dirent *rec;
     struct Wheel *result = NULL;
@@ -47,13 +47,41 @@ struct Wheel *get_wheel_file(const char *basepath, const char *name, char *to_ma
         }
 
         result = calloc(1, sizeof(*result));
+        if (!result) {
+            SYSERROR("Unable to allocate %zu bytes for wheel struct", sizeof(*result));
+            closedir(dp);
+            return NULL;
+        }
+
         result->path_name = realpath(package_path, NULL);
+        if (!result->path_name) {
+            SYSERROR("Unable to resolve absolute path to %s: %s", filename, strerror(errno));
+            wheel_free(&result);
+            closedir(dp);
+            return NULL;
+        }
         result->file_name = strdup(rec->d_name);
+        if (!result->file_name) {
+            SYSERROR("Unable to allocate bytes for %s: %s", rec->d_name, strerror(errno));
+            wheel_free(&result);
+            closedir(dp);
+            return NULL;
+        }
 
         size_t parts_total;
         char **parts = split(filename, "-", 0);
+        if (!parts) {
+            // This shouldn't happen unless a wheel file is present in the
+            // directory with a malformed file name, or we've managed to
+            // exhaust the system's memory
+            SYSERROR("%s has no '-' separators! (Delete this file and try again)", filename);
+            wheel_free(&result);
+            closedir(dp);
+            return NULL;
+        }
+
         for (parts_total = 0; parts[parts_total] != NULL; parts_total++);
-        if (parts_total < 6) {
+        if (parts_total == 5) {
             // no build tag
             result->distribution = strdup(parts[0]);
             result->version = strdup(parts[1]);
@@ -61,7 +89,7 @@ struct Wheel *get_wheel_file(const char *basepath, const char *name, char *to_ma
             result->python_tag = strdup(parts[2]);
             result->abi_tag = strdup(parts[3]);
             result->platform_tag = strdup(parts[4]);
-        } else {
+        } else if (parts_total == 6) {
             // has build tag
             result->distribution = strdup(parts[0]);
             result->version = strdup(parts[1]);
@@ -69,6 +97,13 @@ struct Wheel *get_wheel_file(const char *basepath, const char *name, char *to_ma
             result->python_tag = strdup(parts[3]);
             result->abi_tag = strdup(parts[4]);
             result->platform_tag = strdup(parts[5]);
+        } else {
+            SYSERROR("Unknown wheel name format: %s. Expected 5 or 6 strings "
+                     "separated by '-', but got %zu instead", filename, parts_total);
+            GENERIC_ARRAY_FREE(parts);
+            wheel_free(&result);
+            closedir(dp);
+            return NULL;
         }
         GENERIC_ARRAY_FREE(parts);
         break;
@@ -76,3 +111,16 @@ struct Wheel *get_wheel_file(const char *basepath, const char *name, char *to_ma
     closedir(dp);
     return result;
 }
+
+void wheel_free(struct Wheel **wheel) {
+    struct Wheel *w = (*wheel);
+    guard_free(w->path_name);
+    guard_free(w->file_name);
+    guard_free(w->distribution);
+    guard_free(w->version);
+    guard_free(w->build_tag);
+    guard_free(w->python_tag);
+    guard_free(w->abi_tag);
+    guard_free(w->python_tag);
+    guard_free(w);
+}
diff --git a/tests/test_wheel.c b/tests/test_wheel.c
index 99ac97c..16f60a3 100644
--- a/tests/test_wheel.c
+++ b/tests/test_wheel.c
@@ -50,12 +50,12 @@ void test_get_wheel_file() {
         },
     };
 
-    struct Wheel *doesnotexist = get_wheel_file("doesnotexist", "doesnotexist-0.0.1-py2.py3-none-any.whl", (char *[]) {"not", NULL}, WHEEL_MATCH_ANY);
+    struct Wheel *doesnotexist = get_wheel_info("doesnotexist", "doesnotexist-0.0.1-py2.py3-none-any.whl", (char *[]) {"not", NULL}, WHEEL_MATCH_ANY);
     STASIS_ASSERT(doesnotexist == NULL, "returned non-NULL on error");
 
     for (size_t i = 0; i < sizeof(tc) / sizeof(*tc); i++) {
         struct testcase *test = &tc[i];
-        struct Wheel *wheel = get_wheel_file(".", test->expected.distribution, (char *[]) {(char *) test->expected.version, NULL}, WHEEL_MATCH_ANY);
+        struct Wheel *wheel = get_wheel_info(".", test->expected.distribution, (char *[]) {(char *) test->expected.version, NULL}, WHEEL_MATCH_ANY);
         STASIS_ASSERT(wheel != NULL, "result should not be NULL!");
         STASIS_ASSERT(wheel->file_name && strcmp(wheel->file_name, test->expected.file_name) == 0, "mismatched file name");
         STASIS_ASSERT(wheel->version && strcmp(wheel->version, test->expected.version) == 0, "mismatched version");
@@ -67,6 +67,7 @@ void test_get_wheel_file() {
             STASIS_ASSERT(strcmp(wheel->build_tag, test->expected.build_tag) == 0,
                           "mismatched build tag (optional arbitrary string)");
         }
+        wheel_free(&wheel);
     }
 }