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

-Sdescribe doesn't report -A aliases #104

Closed
jdjohnston opened this issue Jun 14, 2023 · 11 comments
Closed

-Sdescribe doesn't report -A aliases #104

jdjohnston opened this issue Jun 14, 2023 · 11 comments

Comments

@jdjohnston
Copy link

Thanks for another cool tool!

deps -Sdescribe -A:ALIAS ignores any -A:ALIAS. Most of deps.clj code matches -A aliases with :repl-aliases, but there's also reference to :all-aliases, which is what the code for -Sdescribe is using.

:all-aliases probably seems more mnemonic for -A, but there's at least two problems with that:

  1. It's not compatible with clojure.org CLI
  2. -A are not truly all aliases, in the sense of being usable anywhere. If one mixes -A and -M aliases, the CLI (including yours) will complain.

BTW, the PowerShell script never worked for me (old laptop), so it's nice to have a full-featured command-line Clojure launcher when I use Clojure on Windows.

@borkdude
Copy link
Owner

Can you show a repro with a sample deps.edn that outputs differently using the official clojure CLI and deps.clj? Just the deps.edn + deps.clj invocation would be enough, I have the official CLI installed here as well.

@jdjohnston
Copy link
Author

Actually, you don't need a deps.edn, as -Sdescribe doesn't require a "real" alias. But for completeness:
~/tmp
$ cat deps.edn
{

:aliases {
:test {:extra-paths ["test"]}
}

}

~/tmp
$ clojure -Sdescribe -A:test
{:version "1.11.1.1347"
:config-files ["/usr/local/lib/clojure/deps.edn" "/home/jdj/.clojure/deps.edn" "deps.edn" ]
:config-user "/home/jdj/.clojure/deps.edn"
:config-project "deps.edn"
:install-dir "/usr/local/lib/clojure"
:config-dir "/home/jdj/.clojure"
:cache-dir ".cpcache"
:force false
:repro false
:main-aliases ""
:repl-aliases ":test"}

~/tmp
$ deps -Sdescribe -A:test
{:deps-clj-version "1.11.1.1347"
:version "1.11.1.1347"
:config-files ["/home/jdj/.clojure/deps.edn" "deps.edn"]
:config-user "/home/jdj/.clojure/deps.edn"
:config-project "deps.edn"
:install-dir "/home/jdj/.deps.clj/1.11.1.1347/ClojureTools"
:config-dir "/home/jdj/.clojure"
:cache-dir ".cpcache"
:force false
:repro false
:main-aliases ""
:all-aliases ""}

Note the last item for each

@jdjohnston
Copy link
Author

I apologize. I made an error in my original report. It is possible sometimes to mix -M and -A aliases. When the expansion for an -M alias includes :main-opts and the command-line also includes an -A alias, clojure/tools/deps/script/make_classpath2.clj will complain (warning, not error) that -A should not be used with :main-opts.

So, while I made a technical error, I still stand by my comment that -A aliases cannot (or, at least, should not) be used everywhere.

@borkdude
Copy link
Owner

Should work as expected now. Can you try from master?

@borkdude
Copy link
Owner

BTW, the PowerShell script never worked for me (old laptop), so it's nice to have a full-featured command-line Clojure launcher when I use Clojure on Windows.

Note that you can also install this tool as clj.exe on Windows by providing the --as-clj argument to the powershell install script (see README).

@jdjohnston
Copy link
Author

BTW, the PowerShell script never worked for me (old laptop), so it's nice to have a full-featured command-line Clojure launcher when I use Clojure on Windows.

Note that you can also install this tool as clj.exe on Windows by providing the --as-clj argument to the powershell install script (see README).

@borkdude, whoops, I meant the clojure.org PowerShell installer and CLI. Which meant before using your deps.exe, I didn't have the recent Clojure CLI on Windows. Given my problems with PowerShell, I simply downloaded deps.exe from the Releases page. Sorry for the confusion.

And, yes, on Windows, I call it both deps.exe and have a symlink to it called clojure.exe.

@borkdude
Copy link
Owner

Cool, all good then :)

@jdjohnston
Copy link
Author

Should work as expected now. Can you try from master?

Did I do something wrong?

git clone'd the repo, but deps.clj at the root didn't match src/borkdude/deps.clj, which seemed to have the fix. So, following the directions in README.md:

/home/Git/deps.clj
$ bb gen-script

/home/Git/deps.clj
$ ./deps.clj -Spath
----- Error --------------------------------------------------------------------
Type: clojure.lang.ExceptionInfo
Message: Unable to resolve classname: java.util.zip.CheckedInputStream
Data: {:type :sci/error, :line 392, :column 1, :file "/home/Git/deps.clj/./deps.clj", :phase "analysis"}
Location: /home/Git/deps.clj/./deps.clj:407:21
Phase: analysis

----- Context ------------------------------------------------------------------
403: zis (ZipInputStream. fis)]
404: (loop [to-unzip files]
405: (if-let [entry (.getNextEntry zis)]
406: (let [entry-name (.getName entry)
407: cis (java.util.zip.CheckedInputStream. zis (java.util.zip.CRC32.))
^--- Unable to resolve classname: java.util.zip.CheckedInputStream
408: file-name (.getName (io/file entry-name))]
409: (if (contains? files file-name)
410: (let [new-path (.resolve destination-dir file-name)]
411: (Files/copy ^java.io.InputStream cis
412: new-path

----- Stack trace --------------------------------------------------------------
borkdude.deps/let - /home/Git/deps.clj/./deps.clj:407:21
borkdude.deps - /home/Git/deps.clj/./deps.clj:406:11
clojure.core/let -
borkdude.deps/let - /home/Git/deps.clj/./deps.clj:406:11
clojure.core/let -
... (run with --debug to see elided elements)
clojure.core/defn -
borkdude.deps/defn- - /home/Git/deps.clj/./deps.clj:392:1
borkdude.deps - /home/Git/deps.clj/./deps.clj:392:1
clojure.core/defn- -
borkdude.deps - /home/Git/deps.clj/./deps.clj:392:1

@borkdude
Copy link
Owner

@jdjohnston I updated the script. The script needs the latest version of babashka (version that ends with 181).

@borkdude
Copy link
Owner

you can also run deps.clj with clj:

clj -M -m borkdude.deps -Sdescribe

@jdjohnston
Copy link
Author

@jdjohnston I updated the script. The script needs the latest version of babashka (version that ends with 181).

@borkdude Would you believe I downloaded 1.3.181 earlier today? I was just thinking I ought to try deps.clj with the newer bb when I saw your comment. :)

Should the :min-bb-version in bb.edn be bumped or a mention be made in README.md in the section that discusses running deps.clj with bb?

Should work as expected now. Can you try from master?

Just did (with bb 1.3.181) and it looks fine

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

2 participants