kit/install-module fails to update system.edn

[markokocic]

I tried to wrap kit/metrics library and wrap it into a new module. I have an issue when trying to install a new module using kit/install-module with the following error:

updating file: resources/system.edn
Execution error (AssertionError) at rewrite-clj.zip.seqz/get (seqz.cljc:146).
Assert failed: (or (map? zloc) (namespaced-map? zloc) (and (seq? zloc) (integer? k)))

When I comment out the lines containing non-empty target, install seems to work, but it's obviously not a solution.

I defined my module as following: https://github.com/markokocic/modules/blob/kit-metrics/metrics/config.edn

EDIT: The error message is also not very descriptive.

[yogthos]

It looks like one of the target locations is expected to be a map, but it's not. The :merge action expects a map at the target location.

[markokocic]

I am trying to merge only to existing map entries, not create new ones.
Here's the file, for the reference:

{:default
 {:require-restart? true
  :actions
  {:assets     []
   :injections [{:type   :edn
                 :path   "deps.edn"
                 :target [:deps]
                 :action :merge
                 :value  {io.github.kit-clj/kit-metrics {:mvn/version "1.0.0"}}}
                {:type   :edn
                 :path   "resources/system.edn"
                 :target []
                 :action :merge
                 :value  {:metrics/prometheus {}}}
                {:type   :edn
                 :path   "resources/system.edn"
                 :target [:handler/ring]
                 :action :merge
                 :value  {:metrics #ig/ref :metrics/prometheus}}
                {:type   :edn
                 :path   "resources/system.edn"
                 :target [:reitit.routes/api]
                 :action :merge
                 :value  {:metrics #ig/ref :metrics/prometheus}}
                {:type   :clj
                 :path   "src/clj/<<sanitized>>/web/middleware/core.clj"
                 :action :append-requires
                 :value  ["[iapetos.collector.ring :as prometheus-ring]"]}]}}}

The issue seems to be because I try to inject into system.edn multiple times. If I comment out multiple injections to the same file and leave only one, installation works.

[yogthos]

Injecting multiple times into the same file should be fine. All the injections get aggregated here, and then the file is read once and transformed into a zipper. Then, the injections are injected into it and the file is written back out at the end. It's possible there's a bug in the logic though. :)

[markokocic]

Ok, will check. Maybe it works and I messed up the config.edn, but I fail to see where is the error.

Btw, few more questsions:

• how to specify local folder as a module repository? Just to take files from there. It's annoying to have to commit something to the repo and then do modules-sync just to try out the change.
• how do you run tests? I see that kit-generator project has some tests defined, but no alias for running tests?

[yogthos]

It's definitely possible there's a bug in the code. I checked the other modules, and none of them do double injection for EDN files. And modules get cloned into the project, so the easiest way to play with them is to modify the files in the modules folder in the project.

And I've been running tests via the REPL, but tests are work in progress at the moment. :)

[yogthos]

And it does look like there's a bug here. I tried this and the second merge is outside the map.

(let [data  (z/of-string "{:z :r :deps {:foo :bar}}")
        updated (inject
                {:type   :edn
                 :data   data
                 :target []
                 :action :merge
                 :value  (io/str->edn "{:db.sql/connection #profile\n {:prod {:jdbc-url #env JDBC_URL}}}")})]
    (z/root-string
      (inject
        {:type   :edn
         :data   updated
         :target []
         :action :merge
         :value  (io/str->edn "{:x :y}")})))

It looks like topmost function might be going a bit too far in case of EDN.

[yogthos]

I added a fix that should let us test for now. I'd like to find a better way to do it in the future, but I think it should address the immediate issue. Let me know if it works.

[markokocic]

Hi @yogthos , your fix seems to work. I was able to create a new module for metrics library.
It basically updates the project in the same way as by using +metrics profile, with the exception that it doesn't update wrap-base function in <<ns-name>>.web.middleware.core package.

I guess injection framework can't manipulate functions yet. Not sure how to handle that. Maybe adding a new :note key to module config that will contain a string explaining manual steps to be displayed after installation?

[yogthos]

Good to hear the fix works, and function manipulation is something we could add relatively easily. There's already a version of that here for appending a build task calls. So, it could be generalized to allow specifying a namespace and function where the injection will happen.

[markokocic]

If there were possibility to instrument functions or variables in Clojure code, then modules could do everything that profiles are used for now, rendering them obsolete. Then it would be possible to automatically install modules right after project creation.

[yogthos]

It might be possible to use metadata for this, but would need a bit of research. It's also important to keep in mind that we have to handle assets other than code, such as HTML templates, configuration files, etc. I think profiles make sense for things that need to be decided at project creation time, but it's likely possible to express that using modules as well. So, the template could provide the minimal kernel of functionality, and then modules could be applied when the project is created based on the flags supplied. I'm generally partial to this idea as it removed the need for duplication between modules and profiles making maintenance easier.

[markokocic]

Hi @yogthos, one compromise on this topic would be to keep generator logic / templating in profiles only to the parts that are not (yet) supported by modules, and move everything else to module. At the project generation time, generator can process e.g., HTML templates, or source code template, and after that invoke module installation of the corresponding module that will update system.edn, add assets, requires, etc …

That would avoid duplication and make profile – module relationship 1 to 1.

The only drawback would be that if some functionality is not added immediately as a profile, it can't be added later through module (HTML template change, code change). Then it would need to point the user to the module doc page after installing the module to perform the manual step.

I was looking into this approach, but I'm not sure how to implement it in one step. Project generation creates a project in a subfolder, and one needs to cd to that project to sync and install modules. There could be basically two approaches to touch this:

1. Change module related functions to accept base path, and invoke them after template processing step.
2. During project creating update list of modules to be installed in some file, and read that file and install modules during the first start of the application, either automatically or on demand.

What do you think would be a better approach?

[yogthos]

The generator supports the notion of a project root already. You can see an example here. So, I think option 1 would be the one to go with. We could read the context from kit.edn in the generated project, and then set the :project-root key there.