-removed buid_variant_index etc from late-bound vars, it is problemat…

…ic (only valid during actual build)

-updated wiki
-fixed problem where early-bound building=true- based variants were installed, rather than the non-building-eval'd variants

src/rez/build_process_.py

@@ -193,24 +193,8 @@ def visit_variants(self, func, variants=None, **kwargs):
                     % (variant.index, self._n_of_m(variant)))
                 continue
 
-            # Re-evaluate the variant, so that variables such as 'building' and
-            # 'build_variant_index' are set, and any early-bound package attribs
-            # are re-evaluated wrt these vars.
-            #
-            # Note: If you do something weird - like implement an early-bound
-            # 'variants' attrib that changes during a build - then behaviour is
-            # undefined. Don't do this.
-            #
-            re_evaluated_package = variant.parent.get_reevaluated({
-                "building": True,
-                "build_variant_index": variant.index or 0,
-                "build_variant_requires": variant.variant_requires
-            })
-
-            re_evaluated_variant = re_evaluated_package.get_variant(variant.index)
-
             # visit the variant
-            result = func(re_evaluated_variant, **kwargs)
+            result = func(variant, **kwargs)
             results.append(result)
             num_visited += 1
 

src/rez/developer_package.py

@@ -121,10 +121,10 @@ def visit(d):
     def get_reevaluated(self, objects):
         """Get a newly loaded and re-evaluated package.
 
-        Values in `objects` are made available to early-bound package attributes.
-        For example, a re-evaluated package might return a different value for
-        an early-bound 'private_build_requires', depending on the variant
-        currently being built.
+        Values in `objects` are made available to early/late bound package
+        attributes. For example, a re-evaluated package might return a different
+        value for an early-bound 'private_build_requires', depending on the
+        variant currently being built.
 
         Args:
             objects (`dict`): Variables to expose to early-bound package attribs.
@@ -133,13 +133,7 @@ def get_reevaluated(self, objects):
             `DeveloperPackage`: New package.
         """
         with set_objects(objects):
-            package = self.from_path(self.root)
-
-            # set same vars ('building' etc) for late-bound funcs (the previous
-            # set_objects context only sets them for early-bound funcs).
-            package.set_objects(objects)
-
-            return package
+            return self.from_path(self.root)
 
     def _validate_includes(self):
         if not self.includes:

src/rez/packages_.py

@@ -12,7 +12,7 @@
 from rez.exceptions import PackageFamilyNotFoundError, ResourceError
 from rez.vendor.version.version import VersionRange
 from rez.vendor.version.requirement import VersionedObject
-from rez.serialise import FileFormat, default_objects
+from rez.serialise import FileFormat
 from rez.config import config
 import sys
 
@@ -155,8 +155,7 @@ def _wrap_forwarded(self, key, value):
             return value
 
     def _eval_late_binding(self, sourcecode):
-        g = default_objects.copy()
-        g.update(self._get_objects())
+        g = {}
 
         if self.context is None:
             g["in_context"] = lambda: False
@@ -168,22 +167,15 @@ def _eval_late_binding(self, sourcecode):
             bindings = self.context._get_pre_resolve_bindings()
             g.update(bindings)
 
-        # Note that what 'this' actually points to depends on whether the context
-        # is available or not. If not, then 'this' is a DeveloperPackage instance;
-        # if the context is available, it is a Variant instance. So for example,
-        # if in_context() is True, 'this' will have a 'root' attribute, but will
-        # not if in_context() is False.
+        # Note that 'this' could be a `Package` or `Variant` instance. This is
+        # intentional; it just depends on how the package is accessed.
         #
         g["this"] = self
 
         # evaluate the late-bound function
         sourcecode.set_package(self)
         return sourcecode.exec_(globals_=g)
 
-    def _get_objects(self):
-        """Get variables to bind to late-bound funcs."""
-        raise NotImplementedError
-
 
 class Package(PackageBaseResourceWrapper):
     """A package.
@@ -194,14 +186,16 @@ class Package(PackageBaseResourceWrapper):
     """
     keys = schema_keys(package_schema)
 
+    # This is to allow for a simple check like 'this.is_package' in late-bound
+    # funcs, where 'this' may be a package or variant.
+    #
+    is_package = True
+    is_variant = False
+
     def __init__(self, resource, context=None):
         _check_class(resource, PackageResource)
         super(Package, self).__init__(resource, context)
 
-        # variables that are exposed to late-bound funcs on evaluation. Note
-        # that child variants are bound to these same variables.
-        self._late_binding_objects = {}
-
     # arbitrary keys
     def __getattr__(self, name):
         if name in self.data:
@@ -272,17 +266,6 @@ def get_variant(self, index=None):
             if variant.index == index:
                 return variant
 
-    def set_objects(self, objects):
-        """Set bound variables for evaluation of late-bound attribs.
-
-        Args:
-            objects (dict): Variables to bind.
-        """
-        self._late_binding_objects = objects.copy()
-
-    def _get_objects(self):
-        return self._late_binding_objects
-
 
 class Variant(PackageBaseResourceWrapper):
     """A package variant.
@@ -294,6 +277,10 @@ class Variant(PackageBaseResourceWrapper):
     keys = schema_keys(variant_schema)
     keys.update(["index", "root", "subpath"])
 
+    # See comment in `Package`
+    is_package = False
+    is_variant = True
+
     def __init__(self, resource, context=None, parent=None):
         _check_class(resource, VariantResource)
         super(Variant, self).__init__(resource, context)
@@ -419,9 +406,6 @@ def install(self, path, dry_run=False, overrides=None):
         else:
             return Variant(resource)
 
-    def _get_objects(self):
-        return self.parent._get_objects()
-
 
 class PackageSearchPath(object):
     """A list of package repositories.

src/rez/serialise.py

@@ -143,7 +143,7 @@ def _load_file(filepath, format_, update_data_callback, original_filepath=None):
 _set_objects = threading.local()
 
 
-# Default variables to avoid not-defined errors in early/late- bound attribs
+# Default variables to avoid not-defined errors in early-bound attribs
 default_objects = {
     "building": False,
     "build_variant_index": 0,

src/rezplugins/build_process/local.py

@@ -119,9 +119,25 @@ def _build_variant_base(self, variant, build_type, install_path=None,
             if not os.path.exists(variant_install_path):
                 safe_makedirs(variant_install_path)
 
+        # Re-evaluate the variant, so that variables such as 'building' and
+        # 'build_variant_index' are set, and any early-bound package attribs
+        # are re-evaluated wrt these vars. This is done so that attribs such as
+        # 'requires' can change depending on whether a build is occurring or not.
+        #
+        # Note that this re-evaluated variant is ONLY used here, for the purposes
+        # of creating the build context. The variant that is actually installed
+        # is the one evaluated where 'building' is False.
+        #
+        re_evaluated_package = variant.parent.get_reevaluated({
+            "building": True,
+            "build_variant_index": variant.index or 0,
+            "build_variant_requires": variant.variant_requires
+        })
+        re_evaluated_variant = re_evaluated_package.get_variant(variant.index)
+
         # create build environment
         context, rxt_filepath = self.create_build_context(
-            variant=variant,
+            variant=re_evaluated_variant,
             build_type=build_type,
             build_path=variant_build_path)
 

wiki/README.md

@@ -5,4 +5,6 @@ found [here](https://github.com/nerdvegas/rez/wiki).
 
 You should include relevant wiki updates with your code PRs.
 
-To update the wiki, make your changes here, then run ./update-wiki.sh.
+To update the wiki, make your changes here, then run ./update-wiki.sh. The
+current repository status is irrelevant - you don't have to have anything
+committed nor pushed in order to update the wiki repository.

wiki/pages/Package-Definition-Guide.md

@@ -108,6 +108,38 @@ define an arbitrary function earlier in the python source. You can always use a
 two as well - an early binding function can call an arbitrary function defined at the bottom of
 your definition file.
 
+#### Available Objects
+
+Following is the list of objects that are available during early evaluation.
+
+* *building* - see [building](Package-Commands#building);
+* *build_variant_index* - the index of the variant currently being built. This is only relevant if
+  `building` is True.
+* *build_variant_requires* - the subset of package requirements specific to the variant
+  currently being built. This is a list of `PackageRequest` objects. This is only relevant if
+  `building` is True.
+* *this* - the current package, as described previously.
+
+Be aware that early-bound functions are actually evaluated multiple times during a build - once
+pre-build, and once per variant, during its build. This is necessary in order for early-bound
+functions to change their return value based on variables like `build_variant_index`. Note that the
+*pre-build* evaluated value is the one set into the installed package, and in this case, `building`
+is False.
+
+An example of where you'd need to be aware of this is if you wanted the `requires` field to include
+a certain package at runtime only (ie, not present during the package build). In this case, `requires`
+might look like so:
+
+    @early()
+    def requires():
+        if building:
+            return ["python-2"]
+        else:
+            return ["runtimeonly-1.2", "python-2"]
+
+> [[media/icons/warning.png]] You **must** ensure that your early-bound function returns the value
+> you want to see in the installed package, when `building` is False.
+
 ### Late Binding Functions
 
 Late binding functions stay as functions in the installed package definition, and are only evaluated
@@ -194,23 +226,64 @@ late binding *tool* attribute below:
 Here the *request* object is being checked to see if the *maya* package was requested in the
 current env; if it was, a maya-specific tool *maya-edit* is added to the tool list.
 
+> [[media/icons/warning.png]] Always ensure your late binding function returns a sensible
+> value regardless of whether *in_context* is True or False. Otherwise, simply trying to
+> query the package attributes (using *rez-search* for example) may cause errors.
+
+#### Available Objects
+
 Following is the list of objects that are available during late evaluation, if *in_context*
-is true:
+is *True*:
 
-* **context** - the *ResolvedContext* instance this package belongs to;
-* **system** - see [system](Package-Commands#system);
-* **building** - see [building](Package-Commands#building);
-* **request** - see [request](Package-Commands#request);
-* **implicits** - see [implicits](Package-Commands#implicits).
+* *context* - the *ResolvedContext* instance this package belongs to;
+* *system* - see [system](Package-Commands#system);
+* *building* - see [building](Package-Commands#building);
+* *request* - see [request](Package-Commands#request);
+* *implicits* - see [implicits](Package-Commands#implicits).
 
 The following objects are available in *all* cases:
 
-* **this** - the current package;
-* **in_context** - the *in_context* function itself.
+* *this* - the current package/variant (see note below);
+* *in_context* - the *in_context* function itself.
 
-> [[media/icons/warning.png]] Always ensure your late binding function returns a sensible
-> value regardless of whether *in_context* is true or false. Otherwise, simply trying to
-> query the package attributes (using *rez-search* for example) may cause errors.
+> [[media/icons/warning.png]] The *this* object may be either a package or a variant,
+> depending on the situation. For example, if *in_context* is True, then *this* is a
+> variant, because variants are the objects present in a resolved context. On the other
+> hand, if a package is accessed via API (for example, by using the *rez-search* tool),
+> then *this* may be a package. The difference matters, because variants have some
+> attributes that packages don't - notably, *root* and *index*. Use the properties
+> `this.is_package` and `this.is_variant` to distinguish the case if needed.
+
+#### Example - Late Bound build_requires
+
+Here is an example of a package.py with a late-bound `build_requires` field:
+
+    name = "maya_thing"
+
+    version = "1.0.0"
+
+    variants = [
+        ["maya-2017"],
+        ["maya-2018"]
+    ]
+
+    @late()
+    def build_requires():
+        if this.is_package:
+            return []
+        elif this.index == 0:
+            return ["maya_2017_build_utils"]
+        else:
+            return ["maya_2018_build_utils"]
+
+Note the check for `this.is_package`. This is necessary, otherwise the evaluation would
+fail in some circumstances. Specifically, if someone ran the following command, the `this`
+field would actually be a `Package` instance, which doesn't have an `index`:
+
+    ]$ rez-search maya_thing --type package --format '{build_requires}'
+
+In this case, `build_requires` is somewhat nonsensical (there is no common build requirement
+for both variants here), but something needs to be returned nonetheless.
 
 ## Sharing Code Across Package Definition Files