edits:
- Touched up the demo code,
resources.json is now an array of resource "objects", rather than a bunch of lines of individual resource objects.
- Suggested alternative names for new directive.
TL;DR: Looking for feedback on whether it's worth investing more work on a new package directive that would avoid having 1000's of lines in the package definitions for Go applications.
I'm improving our support for Go packages (#13023) and it looks like Go packages could easily need to define 100's of resources. Describing them with with resource() statements is gross.
I'd like to add (see below for a prototype) an additional directive for the package language: import_resources. It would be used like so:
import_resources("resources-2.0.4.json", when="@2.0.4")and would have the same effect as explicitly including resource() statements for all of the resources described in resources-2.0.4.json.
The JSON description might look something like this:
[
{
"name": "github.com/Azure/go-ansiterm",
"git": "https://github.com/Azure/go-ansiterm",
"commit": "d6e3b3328b78",
"placement": "vendor/github.com/Azure/go-ansiterm",
"when": "@2.0.4",
"destination": "."
},
{
"name": "github.com/Microsoft/go-winio",
"git": "https://github.com/Microsoft/go-winio",
"tag": "v0.4.11",
"placement": "vendor/github.com/Microsoft/go-winio",
"when": "@2.0.4",
"destination": "."
}
]There is/will be tooling that Go application packagers can use to generate the required statements mechanically from information in the application's source tree. It currently generates resource() statements that can be pasted into the package.py.
I'm not wed to the directive name. Perhaps just resources, load_resources, bulk_resources, or resources_from_json?
I'm not wed to using JSON for the imported file but whatever format we settle on should be easily machine writable and readable.
The implementation (so far) involves breaking out the body of the existing _execute_resource() into a function that can be shared by resource() and import_resources(), adjusting resource() to use the new function, and adding an implementation for _execute_import_resources() that reads the JSON input file and uses the new function to add each of the resources.
A quick-and-dirty one-shot see-if-it-works implementation (@tgamblin -- this might explain the mysterious headache you just developed...) might look like so:
diff --git a/lib/spack/spack/directives.py b/lib/spack/spack/directives.py
index d877d4a19..2cd18aaa4 100644
--- a/lib/spack/spack/directives.py
+++ b/lib/spack/spack/directives.py
@@ -30,6 +30,7 @@ class OpenMpi(Package):
import collections
import functools
+import json
import os.path
import re
from six import string_types
@@ -607,38 +608,60 @@ def resource(**kwargs):
resource is moved into the main package stage area.
"""
def _execute_resource(pkg):
- when = kwargs.get('when')
+ _resource(pkg, **kwargs)
+ return _execute_resource
+
+def _resource(pkg, **kwargs):
+ print(kwargs)
+ when = kwargs.get('when')
+ when_spec = make_when_spec(when)
+ if not when_spec:
+ return
+
+ destination = kwargs.get('destination', "")
+ placement = kwargs.get('placement', None)
+
+ # Check if the path is relative
+ if os.path.isabs(destination):
+ message = ('The destination keyword of a resource directive '
+ 'can\'t be an absolute path.\n')
+ message += "\tdestination : '{dest}\n'".format(dest=destination)
+ raise RuntimeError(message)
+
+ # Check if the path falls within the main package stage area
+ test_path = 'stage_folder_root'
+ normalized_destination = os.path.normpath(
+ os.path.join(test_path, destination)
+ ) # Normalized absolute path
+
+ if test_path not in normalized_destination:
+ message = ("The destination folder of a resource must fall "
+ "within the main package stage directory.\n")
+ message += "\tdestination : '{dest}'\n".format(dest=destination)
+ raise RuntimeError(message)
+
+ resources = pkg.resources.setdefault(when_spec, [])
+ name = kwargs.get('name')
+ fetcher = from_kwargs(**kwargs)
+ resources.append(Resource(name, fetcher, destination, placement))
+
+@directive('imported_resources')
+def import_resources(filename, when=None):
+ def _execute_import_resources(pkg):
when_spec = make_when_spec(when)
if not when_spec:
return
- destination = kwargs.get('destination', "")
- placement = kwargs.get('placement', None)
-
- # Check if the path is relative
- if os.path.isabs(destination):
- message = ('The destination keyword of a resource directive '
- 'can\'t be an absolute path.\n')
- message += "\tdestination : '{dest}\n'".format(dest=destination)
- raise RuntimeError(message)
-
- # Check if the path falls within the main package stage area
- test_path = 'stage_folder_root'
- normalized_destination = os.path.normpath(
- os.path.join(test_path, destination)
- ) # Normalized absolute path
-
- if test_path not in normalized_destination:
- message = ("The destination folder of a resource must fall "
- "within the main package stage directory.\n")
- message += "\tdestination : '{dest}'\n".format(dest=destination)
- raise RuntimeError(message)
-
- resources = pkg.resources.setdefault(when_spec, [])
- name = kwargs.get('name')
- fetcher = from_kwargs(**kwargs)
- resources.append(Resource(name, fetcher, destination, placement))
- return _execute_resource
+ # See patch.py::FilePatch
+ pkg_dir = os.path.abspath(os.path.dirname(pkg.module.__file__))
+ path = os.path.join(pkg_dir, filename)
+
+ with open(path) as fh:
+ resources = json.load(fh)
+ for r in resources:
+ _resource(pkg, **r)
+
+ return _execute_import_resources
class DirectiveError(spack.error.SpackError):This hack is able to import the 23 resources that are needed to build docui, replacing the 120+ lines that are required to define them using resource(), as well as all of the resources required for its dependencies (e.g. go).
Thoughts?
edits:
resources.jsonis now an array of resource "objects", rather than a bunch of lines of individual resource objects.TL;DR: Looking for feedback on whether it's worth investing more work on a new package directive that would avoid having 1000's of lines in the package definitions for Go applications.
I'm improving our support for Go packages (#13023) and it looks like Go packages could easily need to define 100's of resources. Describing them with with
resource()statements is gross.I'd like to add (see below for a prototype) an additional directive for the package language:
import_resources. It would be used like so:and would have the same effect as explicitly including
resource()statements for all of the resources described inresources-2.0.4.json.The JSON description might look something like this:
There is/will be tooling that Go application packagers can use to generate the required statements mechanically from information in the application's source tree. It currently generates
resource()statements that can be pasted into thepackage.py.I'm not wed to the directive name. Perhaps just
resources,load_resources,bulk_resources, orresources_from_json?I'm not wed to using JSON for the imported file but whatever format we settle on should be easily machine writable and readable.
The implementation (so far) involves breaking out the body of the existing
_execute_resource()into a function that can be shared byresource()andimport_resources(), adjustingresource()to use the new function, and adding an implementation for_execute_import_resources()that reads the JSON input file and uses the new function to add each of the resources.A quick-and-dirty one-shot see-if-it-works implementation (@tgamblin -- this might explain the mysterious headache you just developed...) might look like so:
This hack is able to import the 23 resources that are needed to build
docui, replacing the 120+ lines that are required to define them usingresource(), as well as all of the resources required for its dependencies (e.g.go).Thoughts?