diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f75e4e17..202399ab1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added * Added a compiler metadata flag for suppressing warnings when Var indirection is unavoidable (#1052) * Added the `--emit-generated-python` CLI argument to control whether generated Python code strings are stored by the runtime for each compiled namespace (#1045) + * Added the ability to reload namespaces using the `:reload` flag on `require` (#1060) ### Changed * The compiler will issue a warning when adding any alias that might conflict with any other alias (#1045) diff --git a/pyproject.toml b/pyproject.toml index 5dabedf31..8301fe4a6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -32,6 +32,7 @@ include = ["README.md", "LICENSE"] [tool.poetry.dependencies] python = "^3.8" attrs = ">=22.2.0" +graphlib-backport = { version = "^1.1.0", python = "<3.9" } immutables = ">=0.20,<1.0.0" prompt-toolkit = ">=3.0.0,<4.0.0" pyrsistent = ">=0.18.0,<1.0.0" diff --git a/src/basilisp/core.lpy b/src/basilisp/core.lpy index 8a3239c8a..bc1c5c089 100644 --- a/src/basilisp/core.lpy +++ b/src/basilisp/core.lpy @@ -4944,7 +4944,7 @@ Basilisp will attempt to load a corresponding namespace starting with 'basilisp.' automatically, aliasing the imported namespace as the requested namespace name. This should allow for automatically importing Clojure's included core libraries such - as ``clojure.string``" + as ``clojure.string``." [req] (rewrite-clojure-libspec (cond @@ -4955,26 +4955,49 @@ (ex-info "Invalid libspec for require" {:value req}))))) +(def ^:private require-flags #{:reload :reload-all}) + +(defn ^:private libspecs-and-flags + "Return a vector of ``[libspecs flags]``." + [req] + (let [groups (group-by #(if (contains? require-flags %) + :flags + :libspecs) + req)] + [(:libspecs groups) (set (:flags groups))])) + (defn ^:private require-lib - "Require the library described by ``libspec`` into the Namespace ``requiring-ns``\\." - [requiring-ns libspec] - (let [required-ns-sym (:namespace libspec)] - ;; In order to enable direct linking of Vars as Python variables, required - ;; namespaces must be `require*`ed into the namespace. That's not possible - ;; to do without a macro, so we're using this hacky approach to eval the - ;; code directly (which will effectively add it to the root namespace module). - (eval (list 'require* - (if-let [ns-alias (:as libspec)] - [required-ns-sym :as ns-alias] - required-ns-sym)) - requiring-ns) - ;; Add a special alias for the original namespace (e.g. `clojure.*`), if we - ;; rewrote the namespace on require. - (when-let [original-ns (:original-namespace libspec)] - (.add-alias requiring-ns (the-ns required-ns-sym) original-ns)) - ;; If an `:as-alias` is requested, apply that as well. - (when-let [as-alias (:as-alias libspec)] - (.add-alias requiring-ns (the-ns required-ns-sym) as-alias)) + "Require the library described by ``libspec`` into the Namespace ``requiring-ns``\\ + subject to the provided ``flags``." + [requiring-ns libspec flags] + (let [required-ns-sym (:namespace libspec) + existing-ns (find-ns required-ns-sym)] + (cond + #?@(:lpy39+ + [(and existing-ns (contains? flags :reload-all)) + (.reload-all (the-ns required-ns-sym))]) + + (and existing-ns (contains? flags :reload)) + (.reload (the-ns required-ns-sym)) + + :else + (do + ;; In order to enable direct linking of Vars as Python variables, required + ;; namespaces must be `require*`ed into the namespace. That's not possible + ;; to do without a macro, so we're using this hacky approach to eval the + ;; code directly (which will effectively add it to the root namespace module). + (eval (list 'require* + (if-let [ns-alias (:as libspec)] + [required-ns-sym :as ns-alias] + required-ns-sym)) + requiring-ns) + ;; Add a special alias for the original namespace (e.g. `clojure.*`), if we + ;; rewrote the namespace on require. + (when-let [original-ns (:original-namespace libspec)] + (.add-alias requiring-ns (the-ns required-ns-sym) original-ns)) + ;; If an `:as-alias` is requested, apply that as well. + (when-let [as-alias (:as-alias libspec)] + (.add-alias requiring-ns (the-ns required-ns-sym) as-alias)))) ;; Reset the namespace to the requiring namespace, since it was likely changed ;; during the require process (set! *ns* requiring-ns))) @@ -5032,17 +5055,44 @@ - ``:refer [& syms]`` which will refer syms in the local namespace directly - ``:refer :all`` which will refer all symbols from the namespace directly + Arguments may also be flags, which are optional. Flags are keywords. The following + flags are supported: + + - ``:reload`` if provided, attempt to reload the namespace + - ``:reload-all`` if provided, attempt to reload all named namespaces and the + namespaces loaded by those namespaces as a directed graph + Use of ``require`` directly is discouraged in favor of the ``:require`` directive in - the :lpy:fn:`ns` macro." + the :lpy:fn:`ns` macro. + + .. warning:: + + Reloading namespaces has many of the same limitations described for + :external:py:func:`importlib.reload_module`. Below is a non-exhaustive set of + limitations of reloading Basilisp namespace: + + - Vars will be re-``def``'ed based on the current definition of the underlying + file. If the file has not changed, then the namespace file will be reloaded + according to the current :ref:`namespace_caching` settings. If a Var is + removed from the source file, it will not be removed or updated by a reload. + - References to objects from previous versions of modules (particularly those + external to the namespace) are not rebound by reloading. In Basilisp code, + this problem can be limited by disabling :ref:`inlining` and + :ref:`direct_linking`. + - Updates to type or record definitions will not be reflected to previously + instantiated objects of those types." [& args] - (let [current-ns *ns*] - (doseq [libspec (map require-libspec args)] + (let [current-ns *ns* + groups (libspecs-and-flags args) + libspecs (first groups) + flags (second groups)] + (doseq [libspec (map require-libspec libspecs)] (if (and (:as-alias libspec) (not (:as libspec))) (let [alias-target (or (find-ns (:namespace libspec)) (create-ns (:namespace libspec)))] (.add-alias current-ns alias-target (:as-alias libspec))) (do - (require-lib current-ns libspec) + (require-lib current-ns libspec flags) ;; Add refers (let [new-ns (the-ns (:namespace libspec)) @@ -5075,12 +5125,18 @@ ``:require`` directive of the :lpy:fn:`ns` macro or the ``:use`` directive of the ``ns`` macro." [ns-sym & filters] - (let [current-ns *ns*] + (let [current-ns *ns* + groups (group-by #(if (contains? require-flags %) + :flags + :filters) + filters)] ;; It is necessary to first require the Namespace directly, otherwise ;; when refer attempts to load the Namespace later, it will fail. - (->> (require-libspec ns-sym) - (require-lib current-ns)) - (->> (apply vector ns-sym filters) + (require-lib current-ns + (require-libspec ns-sym) + (set (:flags groups))) + (->> (:filters groups) + (apply vector ns-sym) (require-libspec) (refer-lib current-ns)))) @@ -5113,12 +5169,16 @@ Use of ``use`` directly is discouraged in favor of the ``:use`` directive in the :lpy:fn:`ns` macro." [& args] - (let [current-ns *ns*] - (doseq [libspec (map require-libspec args)] + (let [current-ns *ns* + groups (libspecs-and-flags args) + libspecs (first groups) + flags (second groups)] + (doseq [libspec (map require-libspec libspecs)] ;; Remove the :refer key to avoid having require-lib ;; perform a full :refer, instead we'll use refer-lib - (->> (dissoc libspec :refer) - (require-lib current-ns)) + (require-lib current-ns + (dissoc libspec :refer) + flags) (refer-lib current-ns libspec)) nil)) diff --git a/src/basilisp/importer.py b/src/basilisp/importer.py index c3879a296..1a7d66c35 100644 --- a/src/basilisp/importer.py +++ b/src/basilisp/importer.py @@ -19,6 +19,8 @@ cast, ) +from typing_extensions import TypedDict + from basilisp.lang import compiler as compiler from basilisp.lang import reader as reader from basilisp.lang import runtime as runtime @@ -133,12 +135,17 @@ def _is_namespace_package(path: str) -> bool: return no_inits and has_basilisp_files +class ImporterCacheEntry(TypedDict, total=False): + spec: ModuleSpec + module: BasilispModule + + class BasilispImporter(MetaPathFinder, SourceLoader): # pylint: disable=abstract-method """Python import hook to allow directly loading Basilisp code within Python.""" def __init__(self): - self._cache: MutableMapping[str, dict] = {} + self._cache: MutableMapping[str, ImporterCacheEntry] = {} def find_spec( self, @@ -379,7 +386,11 @@ def exec_module(self, module: types.ModuleType) -> None: assert isinstance(module, BasilispModule) fullname = module.__name__ - cached = self._cache[fullname] + if (cached := self._cache.get(fullname)) is None: + spec = module.__spec__ + assert spec is not None, "Module must have a spec" + cached = {"spec": spec} + self._cache[spec.name] = cached cached["module"] = module spec = cached["spec"] filename = spec.loader_state["filename"] diff --git a/src/basilisp/lang/runtime.py b/src/basilisp/lang/runtime.py index 1ddd6530a..c56d7dbb6 100644 --- a/src/basilisp/lang/runtime.py +++ b/src/basilisp/lang/runtime.py @@ -71,6 +71,11 @@ from basilisp.lang.util import OBJECT_DUNDER_METHODS, demunge, is_abstract, munge from basilisp.util import Maybe +if sys.version_info >= (3, 9): + import graphlib +else: + import graphlib # type: ignore + logger = logging.getLogger(__name__) # Public constants @@ -670,6 +675,49 @@ def __repr__(self): def __hash__(self): return hash(self._name) + def _get_required_namespaces(self) -> vec.PersistentVector["Namespace"]: + """Return a vector of all required namespaces (loaded via `require`, `use`, + or `refer`). + + This vector will include `basilisp.core` unless the namespace was created + manually without requiring it.""" + ts: graphlib.TopologicalSorter = graphlib.TopologicalSorter() + + def add_nodes(ns: Namespace) -> None: + # basilisp.core does actually create a cycle by requiring namespaces + # that require it, so we cannot add it to the topological sorter here, + # or it will throw a cycle error + if ns.name == CORE_NS: + return + + for aliased_ns in ns.aliases.values(): + ts.add(ns, aliased_ns) + add_nodes(aliased_ns) + + for referred_var in ns.refers.values(): + referred_ns = referred_var.ns + ts.add(ns, referred_ns) + add_nodes(referred_ns) + + add_nodes(self) + return vec.vector(ts.static_order()) + + def reload_all(self) -> "Namespace": + """Reload all dependency namespaces as by `Namespace.reload()`.""" + sorted_reload_order = self._get_required_namespaces() + logger.debug(f"Computed :reload-all order: {sorted_reload_order}") + + for ns in sorted_reload_order: + ns.reload() + + return self + + def reload(self) -> "Namespace": + """Reload code in this namespace by reloading the underlying Python module.""" + with self._lock: + importlib.reload(self.module) + return self + def require(self, ns_name: str, *aliases: sym.Symbol) -> BasilispModule: """Require the Basilisp Namespace named by `ns_name` and add any aliases given to this Namespace. @@ -686,6 +734,7 @@ def require(self, ns_name: str, *aliases: sym.Symbol) -> BasilispModule: ns_sym = sym.symbol(ns_name) ns = self.get(ns_sym) assert ns is not None, "Namespace must exist after being required" + self.add_alias(ns, ns_sym) if aliases: self.add_alias(ns, *aliases) return ns_module diff --git a/tests/basilisp/importer_test.py b/tests/basilisp/importer_test.py index 692bff05e..f5905f340 100644 --- a/tests/basilisp/importer_test.py +++ b/tests/basilisp/importer_test.py @@ -17,6 +17,7 @@ from basilisp import importer as importer from basilisp.lang import runtime as runtime from basilisp.lang import symbol as sym +from basilisp.lang import vector as vec from basilisp.lang.util import demunge, munge from basilisp.main import bootstrap as bootstrap_basilisp @@ -114,7 +115,7 @@ def module_dir(self, monkeypatch: pytest.MonkeyPatch, module_cache): def make_new_module(self, module_dir): """Fixture returning a function which creates a new module and then removes it after the test run.""" - filenames = [] + filenames = set() def _make_new_module( *ns_path: str, ns_name: str = "", module_text: Optional[str] = None @@ -134,7 +135,7 @@ def _make_new_module( os.makedirs(os.path.join(module_dir, *ns_path[:-1]), exist_ok=True) filename = os.path.join(module_dir, *ns_path) - filenames.append(filename) + filenames.add(filename) with open(filename, mode="w") as mod: if ns_name != "" and module_text is None: @@ -202,6 +203,145 @@ def test_import_module_without_cache( == not_cached.find(sym.symbol("val")).value ) + def test_reload_module( + self, do_not_cache_namespaces, make_new_module, load_namespace + ): + ns_name = "importer.namespace.to-reload" + + make_new_module( + "importer", + "namespace", + "to_reload.lpy", + module_text=f""" + (ns {ns_name}) + (defn status [] "not-reloaded") + """, + ) + + ns = load_namespace(ns_name) + + assert "not-reloaded" == ns.module.status() + assert "not-reloaded" == ns.find(sym.symbol("status")).value() + + make_new_module( + "importer", + "namespace", + "to_reload.lpy", + module_text=f""" + (ns {ns_name}) + (defn status [] "reloaded") + (defn other [] "new function") + """, + ) + + ns.reload() + + assert "reloaded" == ns.module.status() + assert "reloaded" == ns.find(sym.symbol("status")).value() + + assert "new function" == ns.module.other() + assert "new function" == ns.find(sym.symbol("other")).value() + + def test_reload_all_modules( + self, do_not_cache_namespaces, make_new_module, load_namespace + ): + importlib.reload(runtime.Namespace.get(sym.symbol("basilisp.core")).module) + + ns1_name = "importer.namespace.to-reload1" + ns2_name = "importer.namespace.to-reload2" + ns3_name = "importer.namespace.to-reload3" + + make_new_module( + "importer", + "namespace", + "to_reload1.lpy", + module_text=f""" + (ns {ns1_name} + (:require [{ns2_name}])) + (defn statuses [] + (conj ({ns2_name}/statuses) "1 not-reloaded")) + """, + ) + make_new_module( + "importer", + "namespace", + "to_reload2.lpy", + module_text=f""" + (ns {ns2_name} + (:require [{ns3_name}])) + (defn statuses [] + (conj ({ns3_name}/statuses) "2 not-reloaded")) + """, + ) + make_new_module( + "importer", + "namespace", + "to_reload3.lpy", + module_text=f""" + (ns {ns3_name}) + (defn statuses [] ["3 not-reloaded"]) + """, + ) + + ns1 = load_namespace(ns1_name) + + assert ( + vec.v( + "3 not-reloaded", + "2 not-reloaded", + "1 not-reloaded", + ) + == ns1.module.statuses() + ) + assert ( + vec.v( + "3 not-reloaded", + "2 not-reloaded", + "1 not-reloaded", + ) + == ns1.find(sym.symbol("statuses")).value() + ) + + make_new_module( + "importer", + "namespace", + "to_reload1.lpy", + module_text=f""" + (ns {ns1_name} + (:require [{ns2_name}])) + (defn statuses [] + (conj ({ns2_name}/statuses) "1 reloaded")) + """, + ) + make_new_module( + "importer", + "namespace", + "to_reload3.lpy", + module_text=f""" + (ns {ns3_name}) + (defn statuses [] ["3 reloaded"]) + """, + ) + + ns1.reload_all() + + assert ( + vec.v( + "3 reloaded", + "2 not-reloaded", + "1 reloaded", + ) + == ns1.module.statuses() + ) + assert ( + vec.v( + "3 reloaded", + "2 not-reloaded", + "1 reloaded", + ) + == ns1.find(sym.symbol("statuses")).value() + ) + @pytest.fixture def cached_module_ns(self) -> str: return "importer.namespace.using-cache"