Skip to content

Variant: Native image #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 16 commits into
base: master
Choose a base branch
from
Draft

Variant: Native image #1

wants to merge 16 commits into from

Conversation

Deraen
Copy link
Member

@Deraen Deraen commented Nov 26, 2024

Features:

  • BB task to build a native image of the app
  • Buddy example (as it needs some config to work within the native image?)

Opening a draft PR to better see the diff between master branch and these changes.

If some of the changes make sense in the master branch, I'll try to cherry-pick them there.


:dev
;; Include test folder here for REPL,
;; using test alias with iced command would also apply main-opts
{:extra-paths ["dev/clj" "target/dev" "test/clj"]}

:repl
{:extra-deps {}}
{:extra-deps {cider/cider-nrepl {:mvn/version "0.50.2"}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kind of disagree with including user specific REPL tools in the project configuration.

Usually, cider-nrepl middleware is added by the editor when launching the REPL process. With lein tools like hashp were easy to add to the ~/.lein/profiles.clj but I guess it isn't as easy with ~/.clojure/deps.edn because you'd still need to refer to a alias created there to enable those deps... hmm.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, agree too, was just for my own convenience here.

Use specific aliases need to be activated, so if a project has say a bb dev task bundled for easy startup, maybe there could be options in the task to specify those aliases, or actually, I wonder if babashka supports user-specific task files, since this user-specific customization needs kind of two things 1) deps.edn 2) starting with custom arguments...

Comment on lines +21 to +22
(aero/read-config (or (System/getenv "CONFIG_EDN")
"config.edn")))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think adding the env var here does make sense, but AFAIK removing the io/resource call does break the case of reading the file from classpath.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think this might be a mistake even, was trying to avoid putting the config.edn into the resulting native image, since classpath resource loading is captured via the agent run, but here the config is actually a aero config, which just bundles defaults and instructs reading from environment variables, so we actually want the file in the native binary too...

Though, I think in the past I've found use in being able to specify alternative aero config.edn, that can be loaded outside the classpath, but maybe this is not useful in an example template.

.gitignore Outdated
@@ -20,3 +20,4 @@ node_modules/
/public/main.css
.idea
*.iml
.clj-kondo/imports
Copy link
Member Author

@Deraen Deraen Nov 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ignore is incorrect, library clj-kondo configuration should be commited to the repo.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah true, also clj-kondo documentation mentions that https://github.com/clj-kondo/clj-kondo/blob/master/doc/config.md#importing

Typically, you will want to check copied configs into version control with your project.

#_[ring.adapter.jetty :as jetty])
(:gen-class))

(set! *warn-on-reflection* true)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it is good idea to set this in any ns that does any JVM interop. I wonder if there is a way to enable this for the whole project, or is that something we would want.

@Deraen Deraen changed the title Native image Variant: Native image Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants