Fix dependency tracking on VirtualSourceObjects

lektor/assets.py

@@ -12,7 +12,7 @@ def get_asset(pad, filename, parent=None):
         return None
 
     try:
-        stat_obj = os.stat(os.path.join(parent.source_filename, filename))
+        stat_obj = os.stat(os.path.join(parent._source_filename, filename))
     except OSError:
         return None
     if stat.S_ISDIR(stat_obj.st_mode):
@@ -24,24 +24,23 @@ def get_asset(pad, filename, parent=None):
 
 
 class Asset(SourceObject):
-    # Source specific overrides.  The source_filename to none removes
-    # the inherited descriptor.
     source_classification = "asset"
-    source_filename = None
-
     artifact_extension = ""
 
     def __init__(self, pad, name, path=None, parent=None):
         SourceObject.__init__(self, pad)
         if parent is not None:
             if path is None:
                 path = name
-            path = os.path.join(parent.source_filename, path)
-        self.source_filename = path
+            path = os.path.join(parent._source_filename, path)
+        self._source_filename = path
 
         self.name = name
         self.parent = parent
 
+    def iter_source_filenames(self):
+        yield self._source_filename
+
     @property
     def url_name(self):
         name = self.name
@@ -99,7 +98,7 @@ class Directory(Asset):
     @property
     def children(self):
         try:
-            files = os.listdir(self.source_filename)
+            files = os.listdir(self._source_filename)
         except OSError:
             return
 

lektor/build_programs.py

@@ -176,9 +176,8 @@ def produce_artifacts(self):
             )
 
     def build_artifact(self, artifact):
-        # Record dependecies on all our sources and datamodel
-        self.source.pad.db.track_record_dependency(self.source)
-
+        # FIXME: This check should probably be moved into Artifact so that it
+        # is applied for all artifacts, not just those built from Pages.
         try:
             self.source.url_path.encode("ascii")
         except UnicodeError as error:

lektor/builder.py

@@ -156,13 +156,14 @@ def get_file_info(self, filename):
     def to_source_filename(self, filename):
         return self.path_cache.to_source_filename(filename)
 
-    def get_virtual_source_info(self, virtual_source_path):
-        virtual_source = self.pad.get(virtual_source_path)
-        if not virtual_source:
-            return VirtualSourceInfo(virtual_source_path, None, None)
-        mtime = virtual_source.get_mtime(self.path_cache)
-        checksum = virtual_source.get_checksum(self.path_cache)
-        return VirtualSourceInfo(virtual_source_path, mtime, checksum)
+    def get_virtual_source_info(self, virtual_source_path, alt=None):
+        virtual_source = self.pad.get(virtual_source_path, alt=alt)
+        if virtual_source is not None:
+            mtime = virtual_source.get_mtime(self.path_cache)
+            checksum = virtual_source.get_checksum(self.path_cache)
+        else:
+            mtime = checksum = None
+        return VirtualSourceInfo(virtual_source_path, alt, mtime, checksum)
 
     def connect_to_database(self):
         """Returns a database connection for the build state db."""
@@ -231,7 +232,8 @@ def _iter_artifact_dependency_infos(self, cur, artifact_name, sources):
         found = set()
         for path, mtime, size, checksum, is_dir in rv:
             if "@" in path:
-                yield path, VirtualSourceInfo(path, mtime, checksum)
+                vpath, alt = _unpack_virtual_source_path(path)
+                yield path, VirtualSourceInfo(vpath, alt, mtime, checksum)
             else:
                 file_info = FileInfo(
                     self.env, path, mtime, size, checksum, bool(is_dir)
@@ -375,7 +377,7 @@ def check_artifact_is_current(self, artifact_name, sources, config_hash):
                     return False
 
                 if isinstance(info, VirtualSourceInfo):
-                    new_vinfo = self.get_virtual_source_info(info.path)
+                    new_vinfo = self.get_virtual_source_info(info.path, info.alt)
                     if not info.unchanged(new_vinfo):
                         return False
 
@@ -603,17 +605,48 @@ def unchanged(self, other):
         return self.checksum == other.checksum
 
 
+def _pack_virtual_source_path(path, alt):
+    """Pack VirtualSourceObject's path and alt into a single string.
+
+    The full identity key for a VirtualSourceObject is its ``path`` along with its ``alt``.
+    (Two VirtualSourceObjects with differing alts are not the same object.)
+
+    This functions packs the (path, alt) pair into a single string for storage
+    in the ``artifacts.path`` of the buildstate database.
+
+    Note that if alternatives are not configured for the current site, there is
+    only one alt, so we safely omit the alt from the packed path.
+    """
+    if alt is None or alt == PRIMARY_ALT:
+        return path
+    return f"{alt}@{path}"
+
+
+def _unpack_virtual_source_path(packed):
+    """Unpack VirtualSourceObject's path and alt from packed path.
+
+    This is the inverse of _pack_virtual_source_path.
+    """
+    alt, sep, path = packed.partition("@")
+    if not sep:
+        raise ValueError("A packed virtual source path must include at least one '@'")
+    if "@" not in path:
+        path, alt = packed, None
+    return path, alt
+
+
 class VirtualSourceInfo:
-    def __init__(self, path, mtime=None, checksum=None):
+    def __init__(self, path, alt, mtime=None, checksum=None):
         self.path = path
+        self.alt = alt
         self.mtime = mtime
         self.checksum = checksum
 
     def unchanged(self, other):
         if not isinstance(other, VirtualSourceInfo):
             raise TypeError("'other' must be a VirtualSourceInfo, not %r" % other)
 
-        if self.path != other.path:
+        if (self.path, self.alt) != (other.path, other.alt):
             raise ValueError(
                 "trying to compare mismatched virtual paths: "
                 "%r.unchanged(%r)" % (self, other)
@@ -622,7 +655,10 @@ def unchanged(self, other):
         return (self.mtime, self.checksum) == (other.mtime, other.checksum)
 
     def __repr__(self):
-        return "VirtualSourceInfo(%r, %r, %r)" % (self.path, self.mtime, self.checksum)
+        return (
+            f"{self.__class__.__name__}"
+            f"({self.path!r}, {self.alt!r}, {self.mtime!r}, {self.checksum!r})"
+        )
 
 
 artifacts_row = namedtuple(
@@ -640,7 +676,12 @@ def __repr__(self):
 
 
 class Artifact:
-    """This class represents a build artifact."""
+    """This class represents a build artifact.
+
+    :param sources: The _primary sources_ for the artifact.
+        If none of the primary sources for an artifact exist, the artifact
+        will be considered obsolete and will be deleted at the next pruning.
+    """
 
     def __init__(
         self,
@@ -783,7 +824,7 @@ def operation(con):
                 rows.append(
                     artifacts_row(
                         artifact=self.artifact_name,
-                        source=v_source.path,
+                        source=_pack_virtual_source_path(v_source.path, v_source.alt),
                         source_mtime=mtime,
                         source_size=None,
                         source_checksum=checksum,
@@ -910,8 +951,11 @@ def update(self):
         """
         ctx = self.begin_update()
         try:
+            if self.source_obj:
+                # Record dependencies on all sources and datamodel
+                ctx.track_source_dependency(self.source_obj)
             yield ctx
-        except:  # pylint: disable=bare-except  # noqa
+        except BaseException:
             exc_info = sys.exc_info()
             self.finish_update(ctx, exc_info)
         else:
@@ -976,7 +1020,7 @@ def finish_update(self, ctx, exc_info=None):
         if exc_info is None:
             self._memorize_dependencies(
                 ctx.referenced_dependencies,
-                ctx.referenced_virtual_dependencies.values(),
+                ctx.referenced_virtual_dependencies,
             )
             self._commit()
             return
@@ -992,7 +1036,7 @@ def finish_update(self, ctx, exc_info=None):
         # use a new database connection that immediately commits.
         self._memorize_dependencies(
             ctx.referenced_dependencies,
-            ctx.referenced_virtual_dependencies.values(),
+            ctx.referenced_virtual_dependencies,
             for_failure=True,
         )
 

lektor/context.py

@@ -93,7 +93,8 @@ def __init__(self, artifact=None, pad=None):
 
         # Processing information
         self.referenced_dependencies = set()
-        self.referenced_virtual_dependencies = {}
+        self.referenced_virtual_dependencies = set()
+        self._tracked_sources = set()
         self.sub_artifacts = []
 
         self.flow_block_render_stack = []
@@ -241,11 +242,29 @@ def record_dependency(self, filename, affects_url=None):
 
     def record_virtual_dependency(self, virtual_source):
         """Records a dependency from processing."""
-        path = virtual_source.path
-        self.referenced_virtual_dependencies[path] = virtual_source
+        self.referenced_virtual_dependencies.add(virtual_source)
         for coll in self._dependency_collectors:
             coll(virtual_source)
 
+    def track_source_dependency(self, source):
+        """Track all dependencies of source object"""
+        if source not in self._tracked_sources:
+            self._tracked_sources.add(source)
+            for filename in source.iter_source_filenames():
+                self.record_dependency(filename)
+            for virtual_source in source.iter_virtual_sources():
+                self.record_virtual_dependency(virtual_source)
+
+            if hasattr(source, "datamodel"):
+                db = source.pad.db
+                for model in db.iter_dependent_models(source.datamodel):
+                    if model.filename:
+                        self.record_dependency(model.filename)
+                # XXX: In the case that the record's datamodel is
+                # implied, then the datamodel depends on the
+                # datamodel(s) of our parent(s).  We do not currently
+                # record that.
+
     @contextmanager
     def gather_dependencies(self, func):
         """For the duration of the `with` block the provided function will be