Merge pull request #54 from jhunkeler/fix-bare-package-arguments

Fix bare package arguments

9 files changed, 195 insertions, 83 deletions

diff --git a/README.md b/README.md
index 5ebc8a0..f1d198e 100644
--- a/README.md
+++ b/README.md
@@ -147,24 +147,26 @@ stasis mydelivery.ini
 
 ## Command Line Options
 
-| Long Option          | Short Option | Purpose                                                        |
-|:---------------------|:------------:|:---------------------------------------------------------------|
-| --help               |      -h      | Display usage statement                                        |
-| --version            |      -V      | Display program version                                        |
-| --continue-on-error  |      -C      | Allow tests to fail                                            |
-| --config ARG         |    -c ARG    | Read STASIS configuration file                                 |
-| --cpu-limit ARG      |    -l ARG    | Number of processes to spawn concurrently (default: cpus - 1)  |
-| --python ARG         |    -p ARG    | Override version of Python in configuration                    |
-| --verbose            |      -v      | Increase output verbosity                                      |
-| --unbuffered         |      -U      | Disable line buffering                                         |
-| --update-base        |     n/a      | Update conda installation prior to STATIS environment creation |
-| --parallel-fail-fast |     n/a      | On test error, terminate all concurrent tasks                  |
-| --overwrite          |     n/a      | Overwrite an existing release                                  |
-| --no-docker          |     n/a      | Do not build docker images                                     |
-| --no-artifactory     |     n/a      | Do not upload artifacts to Artifactory                         |
-| --no-testing         |     n/a      | Do not execute test scripts                                    |
-| --no-rewrite         |     n/a      | Do not rewrite paths and URLs in output files                  |
-| DELIVERY_FILE        |     n/a      | STASIS delivery file                                           |
+| Long Option                | Short Option | Purpose                                                        |
+|:---------------------------|:------------:|:---------------------------------------------------------------|
+| --help                     |      -h      | Display usage statement                                        |
+| --version                  |      -V      | Display program version                                        |
+| --continue-on-error        |      -C      | Allow tests to fail                                            |
+| --config ARG               |    -c ARG    | Read STASIS configuration file                                 |
+| --cpu-limit ARG            |    -l ARG    | Number of processes to spawn concurrently (default: cpus - 1)  |
+| --pool-status-interval ARG |     n/a      | Report task status every n seconds (default: 30)               |
+| --python ARG               |    -p ARG    | Override version of Python in configuration                    |
+| --verbose                  |      -v      | Increase output verbosity                                      |
+| --unbuffered               |      -U      | Disable line buffering                                         |
+| --update-base              |     n/a      | Update conda installation prior to STATIS environment creation |
+| --fail-fast                |     n/a      | On test error, terminate all tasks                             |
+| --overwrite                |     n/a      | Overwrite an existing release                                  |
+| --no-docker                |     n/a      | Do not build docker images                                     |
+| --no-artifactory           |     n/a      | Do not upload artifacts to Artifactory                         |
+| --no-testing               |     n/a      | Do not execute test scripts                                    |
+| --no-parallel              |     n/a      | Do not execute tests in parallel                               |
+| --no-rewrite               |     n/a      | Do not rewrite paths and URLs in output files                  |
+| DELIVERY_FILE              |     n/a      | STASIS delivery file                                           |
 
 ## Environment variables
 
@@ -344,11 +346,11 @@ python = {{ env:MY_DYNAMIC_PYTHON_VERSION }}
 
 Template functions can be accessed using the `{{ func:NAME(ARG,...) }}` notation.
 
-| Name                          | Purpose                                                          |
-|-------------------------------|------------------------------------------------------------------|
-| get_github_release_notes_auto | Generate release notes for all test contexts                     |
-| basetemp_dir                  | Generate directory path to test block's temporary data directory |
-| junitxml_file                 | Generate directory path and file name for test result file       |
+| Name                          | Arguments | Purpose                                                          |
+|-------------------------------|-----------|------------------------------------------------------------------|
+| get_github_release_notes_auto | n/a       | Generate release notes for all test contexts                     |
+| basetemp_dir                  | n/a       | Generate directory path to test block's temporary data directory |
+| junitxml_file                 | n/a       | Generate directory path and file name for test result file       |
 
 
 # Mission files
diff --git a/include/conda.h b/include/conda.h
index b5ea926..b26c7a3 100644
--- a/include/conda.h
+++ b/include/conda.h
@@ -186,15 +186,18 @@ int conda_index(const char *path);
 /**
  * Determine whether a simple index contains a package
  * @param index_url a file system path or url pointing to a simple index
- * @param name package name (required)
- * @param version package version (may be NULL)
+ * @param spec a pip package specification (e.g. `name==1.2.3`)
  * @return not found = 0, found = 1, error = -1
  */
-int pip_index_provides(const char *index_url, const char *name, const char *version);
-
-char *conda_get_active_environment();
+int pip_index_provides(const char *index_url, const char *spec);
 
+/**
+ * Determine whether conda can find a package in its channel list
+ * @param spec a conda package specification (e.g. `name=1.2.3`)
+ * @return not found = 0, found = 1, error = -1
+ */
 int conda_provides(const char *spec);
 
+char *conda_get_active_environment();
 
 #endif //STASIS_CONDA_H
diff --git a/include/delivery.h b/include/delivery.h
index 6da890a..15cde13 100644
--- a/include/delivery.h
+++ b/include/delivery.h
@@ -419,4 +419,6 @@ int filter_repo_tags(char *repo, struct StrList *patterns);
  */
 int delivery_exists(struct Delivery *ctx);
 
+int delivery_overlay_packages_from_env(struct Delivery *ctx, const char *env_name);
+
 #endif //STASIS_DELIVERY_H
diff --git a/src/conda.c b/src/conda.c
index 43b9001..e60abc7 100644
--- a/src/conda.c
+++ b/src/conda.c
@@ -79,37 +79,26 @@ int pip_exec(const char *args) {
     return system(command);
 }
 
-int pip_index_provides(const char *index_url, const char *name, const char *version) {
+int pip_index_provides(const char *index_url, const char *spec) {
     char cmd[PATH_MAX] = {0};
-    char name_local[255];
-    char version_local[255] = {0};
-    char spec[255] = {0};
+    char spec_local[255] = {0};
 
-    if (isempty((char *) name) < 0) {
-        // no package name means nothing to do.
+    if (isempty((char *) spec)) {
+        // NULL or zero-length; no package spec means there's nothing to do.
         return -1;
     }
 
-    // Fix up the package name
-    strncpy(name_local, name, sizeof(name_local) - 1);
-    tolower_s(name_local);
-    lstrip(name_local);
-    strip(name_local);
-
-    if (version) {
-        // Fix up the package version
-        strncpy(version_local, version, sizeof(version_local) - 1);
-        tolower_s(version_local);
-        lstrip(version_local);
-        strip(version_local);
-        sprintf(spec, "==%s", version);
-    }
+    // Normalize the local spec string
+    strncpy(spec_local, spec, sizeof(spec_local) - 1);
+    tolower_s(spec_local);
+    lstrip(spec_local);
+    strip(spec_local);
 
     char logfile[] = "/tmp/STASIS-package_exists.XXXXXX";
     int logfd = mkstemp(logfile);
     if (logfd < 0) {
         perror(logfile);
-        remove(logfile);    // fail harmlessly if not present
+        remove(logfile);  // fail harmlessly if not present
         return -1;
     }
 
@@ -121,7 +110,7 @@ int pip_index_provides(const char *index_url, const char *name, const char *vers
     strcpy(proc.f_stdout, logfile);
 
     // Do an installation in dry-run mode to see if the package exists in the given index.
-    snprintf(cmd, sizeof(cmd) - 1, "python -m pip install --dry-run --no-deps --index-url=%s %s%s", index_url, name_local, spec);
+    snprintf(cmd, sizeof(cmd) - 1, "python -m pip install --dry-run --no-deps --index-url=%s %s", index_url, spec_local);
     status = shell(&proc, cmd);
 
     // Print errors only when shell() itself throws one
diff --git a/src/delivery.c b/src/delivery.c
index a689db2..07e04c8 100644
--- a/src/delivery.c
+++ b/src/delivery.c
@@ -178,38 +178,45 @@ void delivery_defer_packages(struct Delivery *ctx, int type) {
         dataptr = ctx->conda.pip_packages;
         deferred = ctx->conda.pip_packages_defer;
         strcpy(mode, "pip");
+    } else {
+        SYSERROR("BUG: type %d does not map to a supported package manager!\n", type);
+        exit(1);
     }
     msg(STASIS_MSG_L2, "Filtering %s packages by test definition...\n", mode);
 
     struct StrList *filtered = NULL;
     filtered = strlist_init();
     for (size_t i = 0; i < strlist_count(dataptr); i++) {
-        int ignore_pkg = 0;
+        int build_for_host = 0;
 
         name = strlist_item(dataptr, i);
         if (!strlen(name) || isblank(*name) || isspace(*name)) {
             // no data
             continue;
         }
-        msg(STASIS_MSG_L3, "package '%s': ", name);
 
         // Compile a list of packages that are *also* to be tested.
-        char *version;
         char *spec_begin = strpbrk(name, "@~=<>!");
         char *spec_end = spec_begin;
+        char package_name[255] = {0};
+
         if (spec_end) {
             // A version is present in the package name. Jump past operator(s).
             while (*spec_end != '\0' && !isalnum(*spec_end)) {
                 spec_end++;
             }
+            strncpy(package_name, name, spec_begin - name);
+        } else {
+            strncpy(package_name, name, sizeof(package_name) - 1);
         }
 
+        msg(STASIS_MSG_L3, "package '%s': ", package_name);
+
         // When spec is present in name, set tests->version to the version detected in the name
         for (size_t x = 0; x < sizeof(ctx->tests) / sizeof(ctx->tests[0]) && ctx->tests[x].name != NULL; x++) {
             struct Test *test = &ctx->tests[x];
-            version = NULL;
-
             char nametmp[1024] = {0};
+
             if (spec_end != NULL && spec_begin != NULL) {
                 strncpy(nametmp, name, spec_begin - name);
             } else {
@@ -220,18 +227,16 @@ void delivery_defer_packages(struct Delivery *ctx, int type) {
                 // Override test->version when a version is provided by the (pip|conda)_package list item
                 guard_free(test->version);
                 if (spec_begin && spec_end) {
-                    *spec_begin = '\0';
                     test->version = strdup(spec_end);
                 } else {
                     // There are too many possible default branches nowadays: master, main, develop, xyz, etc.
                     // HEAD is a safe bet.
                     test->version = strdup("HEAD");
                 }
-                version = test->version;
 
                 // Is the list item a git+schema:// URL?
-                if (strstr(name, "git+") && strstr(name, "://")) {
-                    char *xrepo = strstr(name, "+");
+                if (strstr(nametmp, "git+") && strstr(nametmp, "://")) {
+                    char *xrepo = strstr(nametmp, "+");
                     if (xrepo) {
                         xrepo++;
                         guard_free(test->repository);
@@ -239,7 +244,7 @@ void delivery_defer_packages(struct Delivery *ctx, int type) {
                         xrepo = NULL;
                     }
                     // Extract the name of the package
-                    char *xbasename = path_basename(name);
+                    char *xbasename = path_basename(nametmp);
                     if (xbasename) {
                         // Replace the git+schema:// URL with the package name
                         strlist_set(&dataptr, i, xbasename);
@@ -247,27 +252,36 @@ void delivery_defer_packages(struct Delivery *ctx, int type) {
                     }
                 }
 
-                if (DEFER_PIP == type && pip_index_provides(PYPI_INDEX_DEFAULT, name, version)) {
-                    fprintf(stderr, "(%s present on index %s): ", version, PYPI_INDEX_DEFAULT);
-                    ignore_pkg = 0;
+                int upstream_exists = 0;
+                if (DEFER_PIP == type) {
+                    upstream_exists = pip_index_provides(PYPI_INDEX_DEFAULT, name);
+                } else if (DEFER_CONDA == type) {
+                    upstream_exists = conda_provides(name);
                 } else {
-                    ignore_pkg = 1;
+                    fprintf(stderr, "\nUnknown package type: %d\n", type);
+                    exit(1);
                 }
+
+                if (upstream_exists < 0) {
+                    fprintf(stderr, "%s's existence command failed for '%s'\n"
+                        "(This may be due to a network/firewall issue!)\n", mode, name);
+                    exit(1);
+                }
+                if (!upstream_exists) {
+                    build_for_host = 1;
+                } else {
+                    build_for_host = 0;
+                }
+
                 break;
             }
         }
 
-        if (ignore_pkg) {
-            char build_at[PATH_MAX];
-            if (DEFER_CONDA == type) {
-                sprintf(build_at, "%s=%s", name, version);
-                name = build_at;
-            }
-
+        if (build_for_host) {
             printf("BUILD FOR HOST\n");
             strlist_append(&deferred, name);
         } else {
-            printf("USE EXISTING\n");
+            printf("USE EXTERNAL\n");
             strlist_append(&filtered, name);
         }
     }
diff --git a/src/delivery_install.c b/src/delivery_install.c
index 1ecedc5..a7754e8 100644
--- a/src/delivery_install.c
+++ b/src/delivery_install.c
@@ -1,11 +1,9 @@
 #include "delivery.h"
 
 static struct Test *requirement_from_test(struct Delivery *ctx, const char *name) {
-    struct Test *result;
-
-    result = NULL;
+    struct Test *result = NULL;
     for (size_t i = 0; i < sizeof(ctx->tests) / sizeof(ctx->tests[0]); i++) {
-        if (ctx->tests[i].name && strstr(name, ctx->tests[i].name)) {
+        if (ctx->tests[i].name && !strcmp(name, ctx->tests[i].name)) {
             result = &ctx->tests[i];
             break;
         }
@@ -13,6 +11,101 @@ static struct Test *requirement_from_test(struct Delivery *ctx, const char *name
     return result;
 }
 
+static char *have_spec_in_config(struct Delivery *ctx, const char *name) {
+    for (size_t x = 0; x < strlist_count(ctx->conda.pip_packages); x++) {
+        char *config_spec = strlist_item(ctx->conda.pip_packages, x);
+        char *op = find_version_spec(config_spec);
+        char package[255] = {0};
+        if (op) {
+            strncpy(package, config_spec, op - config_spec);
+        } else {
+            strncpy(package, config_spec, sizeof(package) - 1);
+        }
+        if (strncmp(package, name, strlen(package)) == 0) {
+            return config_spec;
+        }
+    }
+    return NULL;
+}
+
+int delivery_overlay_packages_from_env(struct Delivery *ctx, const char *env_name) {
+    char *current_env = conda_get_active_environment();
+    int need_restore = current_env && strcmp(env_name, current_env) != 0;
+
+    conda_activate(ctx->storage.conda_install_prefix, env_name);
+    // Retrieve a listing of python packages installed under "env_name"
+    int freeze_status = 0;
+    char *freeze_output = shell_output("python -m pip freeze", &freeze_status);
+    if (freeze_status) {
+        guard_free(freeze_output);
+        guard_free(current_env);
+        return -1;
+    }
+
+    if (need_restore) {
+        // Restore the original conda environment
+        conda_activate(ctx->storage.conda_install_prefix, current_env);
+    }
+    guard_free(current_env);
+
+    struct StrList *frozen_list = strlist_init();
+    strlist_append_tokenize(frozen_list, freeze_output, LINE_SEP);
+    guard_free(freeze_output);
+
+    struct StrList *new_list = strlist_init();
+
+    // - consume package specs that have no test blocks.
+    // - these will be third-party packages like numpy, scipy, etc.
+    // - and they need to be present at the head of the list so they
+    //   get installed first.
+    for (size_t i = 0; i < strlist_count(ctx->conda.pip_packages); i++) {
+        char *spec = strlist_item(ctx->conda.pip_packages, i);
+        char spec_name[255] = {0};
+        char *op = find_version_spec(spec);
+        if (op) {
+            strncpy(spec_name, spec, op - spec);
+        } else {
+            strncpy(spec_name, spec, sizeof(spec_name) - 1);
+        }
+        struct Test *test_block = requirement_from_test(ctx, spec_name);
+        if (!test_block) {
+            msg(STASIS_MSG_L2 | STASIS_MSG_WARN, "from config without test: %s\n", spec);
+            strlist_append(&new_list, spec);
+        }
+    }
+
+    // now consume packages that have a test block
+    // if the ini provides a spec, override the environment's version.
+    // otherwise, use the spec derived from the environment
+    for (size_t i = 0; i < strlist_count(frozen_list); i++) {
+        char *frozen_spec = strlist_item(frozen_list, i);
+        char frozen_name[255] = {0};
+        char *op = find_version_spec(frozen_spec);
+        // we only care about packages with specs here. if something else arrives, ignore it
+        if (op) {
+            strncpy(frozen_name, frozen_spec, op - frozen_spec);
+        } else {
+            strncpy(frozen_name, frozen_spec, sizeof(frozen_name) - 1);
+        }
+        struct Test *test = requirement_from_test(ctx, frozen_name);
+        if (test && strcmp(test->name, frozen_name) == 0) {
+            char *config_spec = have_spec_in_config(ctx, frozen_name);
+            if (config_spec) {
+                msg(STASIS_MSG_L2, "from config: %s\n", config_spec);
+                strlist_append(&new_list, config_spec);
+            } else {
+                msg(STASIS_MSG_L2, "from environment: %s\n", frozen_spec);
+                strlist_append(&new_list, frozen_spec);
+            }
+        }
+    }
+    guard_strlist_free(&ctx->conda.pip_packages);
+    ctx->conda.pip_packages = strlist_copy(new_list);
+    guard_strlist_free(&new_list);
+    guard_strlist_free(&frozen_list);
+    return 0;
+}
+
 int delivery_install_packages(struct Delivery *ctx, char *conda_install_dir, char *env_name, int type, struct StrList **manifest) {
     char cmd[PATH_MAX];
     char pkgs[STASIS_BUFSIZ];
diff --git a/src/stasis_main.c b/src/stasis_main.c
index b5d43e3..737fafc 100644
--- a/src/stasis_main.c
+++ b/src/stasis_main.c
@@ -496,11 +496,12 @@ int main(int argc, char *argv[]) {
     }
 
     msg(STASIS_MSG_L1, "Creating release environment(s)\n");
-    if (ctx.meta.based_on && strlen(ctx.meta.based_on)) {
+    if (!isempty(ctx.meta.based_on)) {
         if (conda_env_remove(env_name)) {
-           msg(STASIS_MSG_ERROR | STASIS_MSG_L2, "failed to remove release environment: %s\n", env_name_testing);
-           exit(1);
+            msg(STASIS_MSG_ERROR | STASIS_MSG_L2, "failed to remove release environment: %s\n", env_name);
+            exit(1);
         }
+
         msg(STASIS_MSG_L2, "Based on release: %s\n", ctx.meta.based_on);
         if (conda_env_create_from_uri(env_name, ctx.meta.based_on)) {
             msg(STASIS_MSG_ERROR | STASIS_MSG_L2, "unable to install release environment using configuration file\n");
@@ -508,7 +509,7 @@ int main(int argc, char *argv[]) {
         }
 
         if (conda_env_remove(env_name_testing)) {
-            msg(STASIS_MSG_ERROR | STASIS_MSG_L2, "failed to remove testing environment\n");
+            msg(STASIS_MSG_ERROR | STASIS_MSG_L2, "failed to remove testing environment %s\n", env_name_testing);
             exit(1);
         }
         if (conda_env_create_from_uri(env_name_testing, ctx.meta.based_on)) {
@@ -544,10 +545,18 @@ int main(int argc, char *argv[]) {
     }
 
     if (pip_exec("install build")) {
-        msg(STASIS_MSG_ERROR | STASIS_MSG_L2, "'build' tool installation failed");
+        msg(STASIS_MSG_ERROR | STASIS_MSG_L2, "'build' tool installation failed\n");
         exit(1);
     }
 
+    if (!isempty(ctx.meta.based_on)) {
+        msg(STASIS_MSG_L1, "Generating package overlay from environment: %s\n", env_name);
+        if (delivery_overlay_packages_from_env(&ctx, env_name)) {
+            msg(STASIS_MSG_L2 | STASIS_MSG_ERROR, "%s", "Failed to generate package overlay. Resulting environment integrity cannot be guaranteed.\n");
+            exit(1);
+        }
+    }
+
     msg(STASIS_MSG_L1, "Filter deliverable packages\n");
     delivery_defer_packages(&ctx, DEFER_CONDA);
     delivery_defer_packages(&ctx, DEFER_PIP);
diff --git a/stasis.ini b/stasis.ini
index 875ca26..ba3331a 100644
--- a/stasis.ini
+++ b/stasis.ini
@@ -10,7 +10,7 @@ always_update_base_environment = false
 conda_fresh_start = true
 
 ; (string) Install conda in a custom prefix
-; DEFAULT: Conda will be installed under stasis/conda
+; DEFAULT: Conda will be installed under stasis/tools/conda
 ; NOTE: conda_fresh_start will automatically be set to "false"
 ;conda_install_prefix = /path/to/conda
 
diff --git a/tests/data/generic.ini b/tests/data/generic.ini
index 7d77edd..fd67ed7 100644
--- a/tests/data/generic.ini
+++ b/tests/data/generic.ini
@@ -50,7 +50,7 @@ dest = {{ meta.mission }}/{{ info.build_name }}/
 
 
 [deploy:docker]
-;registry = bytesalad.stsci.edu
+registry = bytesalad.stsci.edu
 image_compression = zstd -v -9 -c
 build_args =
     SNAPSHOT_INPUT={{ info.release_name }}.yml