Skip to content
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

Add --bb-edn or --config option to specify other bb.edn file #1110

Closed
borkdude opened this issue Dec 16, 2021 · 19 comments
Closed

Add --bb-edn or --config option to specify other bb.edn file #1110

borkdude opened this issue Dec 16, 2021 · 19 comments

Comments

@borkdude
Copy link
Collaborator

borkdude commented Dec 16, 2021

The other bb.edn (or bb.ci.edn, etc) file can live in another directory.
Relative paths in :paths should be resolved against the explicitly provided --config's location.

In the future we can also add --context for setting the directory to which we should resolve relative dirs againt, but we can default to the explicit --config option when given, or the current working directory otherwise (like it is now).

See discussion in #869

/cc @wilkerlucio @cldwalker @thiagokokada @lispyclouds

@cldwalker
Copy link
Contributor

cldwalker commented Dec 17, 2021

I'd be in favor of suggested behavior above. The two use cases I have are:

  1. Given an explicit config file, run a babashka executable with relative paths resolved to explicit file e.g. bb-github-repo as an executable
  2. Given an explicit config file, run a babashka task with relative paths resolved to explicit file e.g. bb --config CONFIG run FOO

@borkdude
Copy link
Collaborator Author

Also see #1103 which is somewhat related to invoking tasks from another tasks file, but with merge behavior.

@cldwalker
Copy link
Contributor

That does look helpful and related but it's different enough that I don't think it address my uses cases. Since #1103 is task specific it doesn't address the executable use case. For the task use case, #1103 requires a local bb.edn in order to merge in another bb.edn. I'm looking to invoke another bb.edn from the commandline

@borkdude
Copy link
Collaborator Author

@cldwalker Agreed. I hope to look into this issue soon (but feel free to experiment).

@borkdude
Copy link
Collaborator Author

borkdude commented Dec 24, 2021

Should work now in branch other-bb-edn. Still have to write some tests and docs, but with the manual tests I did it worked.

The argument is --config other-bb.edn. Paths are resolved relative to the other bb.edn.

macOS: https://24985-201467090-gh.circle-artifacts.com/0/release/babashka-0.7.1-SNAPSHOT-macos-amd64.tar.gz
linux: https://24986-201467090-gh.circle-artifacts.com/0/release/babashka-0.7.1-SNAPSHOT-linux-amd64-static.tar.gz

Please test!

@borkdude
Copy link
Collaborator Author

Merged to master.

@borkdude
Copy link
Collaborator Author

Btw I also implemented --deps-root for explicitly setting the dependency root for relative paths.

@borkdude
Copy link
Collaborator Author

You can now install a dev release: https://github.com/babashka/babashka-dev-builds
This should hopefully help getting this tested!

@cldwalker
Copy link
Contributor

@borkdude Tested --config and it works great for my tasks use case! I ended up not using it for the executable use case as #!/usr/bin/env bb --config PATH wouldn't be an executable I could share with others. I used an explicit deps/add-deps which works well enough there. I haven't tried --deps-root as I didn't have a use for it yet. Thanks!

@borkdude
Copy link
Collaborator Author

borkdude commented Dec 27, 2021

I guess #!/usr/bin/env bb --config PATH could be a bash wrapper script you could share, if your project consisted of many namespaces and wanted to use declarative deps, but yeah, add-deps is nice for keeping the shared artifact a single file. I wonder if it would be useful to have something like the clojure -Ttool thing. E.g we could have something like:

bb add-tool :git/url https://github.com/borkdude/http-server :git/sha <> :as http-server

which will make a ~/.babashka/tools/http-server bash wrapper which does the config thing.

You would then put ~/.babashka/tools in your PATH.

@thiagokokada
Copy link
Contributor

thiagokokada commented Dec 27, 2021

Just a reminder that env doesn't support parameters unless the -S flag is passed. This is non-POSIX extension, and it was not supported by coreutils until version 8.3.0 (that is kinda recent, Ubuntu 19.04 was the first one to include it).

AFAIK, -S works in BSDs (not that it matters for babashka) and macOS too, so if supporting old Linux distros is not an issue it is a safe flag to use.

TL;DR: shebang should be #!/usr/bin/env -S bb --config PATH unless you have an old Linux distro.

@borkdude
Copy link
Collaborator Author

borkdude commented Dec 27, 2021

Yes, I was intending to suggest replacing what @cldwalker wrote with a bash wrapper script:

#!/usr/bin/env bash

bb --config ...

@thiagokokada
Copy link
Contributor

Should work now in branch other-bb-edn. Still have to write some tests and docs, but with the manual tests I did it worked.

The argument is --config other-bb.edn. Paths are resolved relative to the other bb.edn.

macOS: https://24985-201467090-gh.circle-artifacts.com/0/release/babashka-0.7.1-SNAPSHOT-macos-amd64.tar.gz linux: https://24986-201467090-gh.circle-artifacts.com/0/release/babashka-0.7.1-SNAPSHOT-linux-amd64-static.tar.gz

Please test!

Seems to work fine for my use case too (tested only with --config, not with --deps-root.

@borkdude
Copy link
Collaborator Author

So --config foo/bar/bb.edn picks up foo/bar as the deps root. If you would have {:paths ["src"]} in your bb.edn then foo/bar/src would be added to the classpath. I'm not sure if this was the right default but since it can be configured, there's always a way to make it do what you want.

@cldwalker
Copy link
Contributor

I guess #!/usr/bin/env bb --config PATH could be a bash wrapper script you could share, if your project consisted of many namespaces and wanted to use declarative deps, but yeah, add-deps is nice for keeping the shared artifact a single file. I wonder if it would be useful to have something like the clojure -Ttool thing. E.g we could have something like:

bb add-tool :git/url https://github.com/borkdude/http-server :git/sha <> :as http-server

which will make a ~/.babashka/tools/http-server bash wrapper which does the config thing.

You would then put ~/.babashka/tools in your PATH

Having a consistent directory that babashka installs tools/commands/executables/scripts to PATH could be a powerful and useful way to share babashka scripts. In my case, I have about a dozen+ in one repository at https://github.com/cldwalker/bb-clis/tree/master/bin. A different name than tools could avoid confusion if it doesn't behave like clojure CLI tools

@thiagokokada
Copy link
Contributor

thiagokokada commented Dec 27, 2021

Should work now in branch other-bb-edn. Still have to write some tests and docs, but with the manual tests I did it worked.

The argument is --config other-bb.edn. Paths are resolved relative to the other bb.edn.

macOS: https://24985-201467090-gh.circle-artifacts.com/0/release/babashka-0.7.1-SNAPSHOT-macos-amd64.tar.gz linux: https://24986-201467090-gh.circle-artifacts.com/0/release/babashka-0.7.1-SNAPSHOT-linux-amd64-static.tar.gz

Please test!

Seems this branch broke something (this code used to work in babashka 0.7.0). I have a script that is in my PATH and loads bb.edn from a project based on where the project is located. If I try to run it in other path that is not the path that the script is currently located, I get:

----- Error --------------------------------------------------------------------
Type:     java.lang.NullPointerException
Location: /home/thiagoko/dev/nu/nucli.bb/bin/nu:53:1

----- Context ------------------------------------------------------------------
49:        (apply (partial fs/path nucli-bb-dir))
50:        str))
51:
52: (load-file (path-from-nucli-bb-dir "nu.clj"))
53: (deps/add-deps (-> "bb.edn" path-from-nucli-bb-dir slurp edn/read-string))
    ^---
54: (cp/add-classpath (path-from-nucli-bb-dir "src"))
55:
56: ;; Start nucli.bb
57:
58: (require '[nucli.main :as nucli])

----- Stack trace --------------------------------------------------------------
babashka.fs/as-path         - <built-in>
babashka.fs/absolutize      - <built-in>
babashka.impl.deps/add-deps - <built-in>
user                        - /home/thiagoko/dev/nu/nucli.bb/bin/nu:53:1

Running the same script inside the directory that the bb.edn file is located works.

@borkdude
Copy link
Collaborator Author

@thiagokokada Could you make a small repro that works with 0.7.0 but not with master?

@thiagokokada
Copy link
Contributor

@borkdude Try this one.

bb-bug.zip

Extract it and execute the bin/script file outside of the directory where the bb.edn file is located.

On 0.7.0 I get:

$ ~/bb-bug/bin/script
Hello World

On 0.7.1-SNAPSHOT I get:

$ ~/bb-bug/bin/script
----- Error --------------------------------------------------------------------
Type:     java.lang.NullPointerException
Location: /home/thiagoko/bb-bug/bin/script:23:1

----- Context ------------------------------------------------------------------
19:   (->> paths
20:        (apply (partial fs/path script-dir))
21:        str))
22:
23: (deps/add-deps (-> "bb.edn" path-from-script-dir slurp edn/read-string))
    ^---
24: (cp/add-classpath (path-from-script-dir "src"))
25:
26: (println "Hello World")
27:
28: ;; vim: set ft=clojure:

----- Stack trace --------------------------------------------------------------
babashka.fs/as-path         - <built-in>
babashka.fs/absolutize      - <built-in>
babashka.impl.deps/add-deps - <built-in>
user                        - /home/thiagoko/bb-bug/bin/script:23:1

Remember to set BABASHKA_BIN (inside bin/script file) to the PATH where your babashka binary is located.

@borkdude
Copy link
Collaborator Author

Fixed on master

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

No branches or pull requests

3 participants