Alternate PR for clojurescript: clojurescript-compiler #21744

danielsz · 2013-08-08T02:03:48Z

The arguments in #21733 favor the removal of the clojure formula, and I'm in agreement with those. The clojure formula is superfluous: leiningen is all you need to get started with clojure. However, those arguments do not carry over to the case of clojurescript.

clojurescript is a compiler that can be run independently from leiningen. It compiles clojure source files down to javascript.

The clojurescript devs refer new users to leiningen, but also include instructions on how to install clojurescript separately. This is useful anytime you want to invoke the compiler outside of a project, in editor extensions, custom tooling, etc. Currently, it is not possible to use leiningen as a single-shot build tool against arbitrary files.

Those instructions are documented in the quick start section of the wiki.

https://github.com/clojure/clojurescript/wiki/Quick-Start

In the resulting installation, clojurescript can be run independently. This formula proposes to automate those prescriptions.

In addition to caveats, I propose to rename the recipe to clojurescript-compiler so as to not to lure beginners. This is a formula similar in spirit as the closure-compiler formula.

Raynes · 2013-08-08T02:10:36Z

Just a correction, it is possible to use leiningen as a single-shot build tool. Just add a dependency on the clojurescript compiler, [org.clojure/clojurescript "0.0-1806"] for example, to your :user profile and then you can use the compiler anywhere from within a lein repl.

danielsz · 2013-08-08T02:15:16Z

Which means typing commands in an interactive environment. That doesn't make it a single-shot build tool, sorry.

Raynes · 2013-08-08T02:26:06Z

I don't know why you're so obsessed with a 'single-shot' tool. What are your requirements? I mean, do you realize how slow it is to invoke the ClojureScript compiler when you don't already have a JVM running? They even go as far to suggest this.

Why do you want this so bad? I just don't get it.

In any case, I concede because obviously nothing can be said to convince you that this is silly. If this gets accepted we'll just keep doing the "sigh, please install leiningen and use lein-cljsbuild instead" dance we've been doing for Clojure packages for ages when people inevitably complain that they can't actually use this for anything.

danielsz · 2013-08-08T02:38:37Z

I appreciate your concern about easing new users into clojurescript. Homebrew has plenty of formulas that target advanced users. As this one does. This formula is just the official quick start guide converted into a homebrew formula. This formula has no agenda. You should address the devs directly if you think their guidelines are misleading.

adamv · 2013-08-08T03:12:42Z

Library/Formula/clojurescript-compiler.rb

+    system "./script/bootstrap"
+    prefix.install Dir['*'] 
+    [bin/'cljsc', prefix/'script/repl', prefix/'script/repljs', prefix/'script/browser-repl'].map { | file | set_env(file) }
+    [prefix/'script/repljs', prefix/'script/browser-repl'].map { | file | bin.install(file) }


Don't install the files to prefix and then move them to bin, do:

bin.install "file1", "file2", "file3"...

danielsz · 2013-08-08T04:53:00Z

I hope this is better.
Thanks @adamv.

samueljohn · 2013-08-08T07:34:36Z

I prefer the name clojurescript over clojurescript-compiler. I think we try to name the formula as people would expect it from the project name. For example python instead of CPython. The idea is that you can mostly without looking up brew install clojurescript and it _just works_™. Alternatively we could add an alias clojurescript -> clojurescript-compiler to accomplish this.

samueljohn · 2013-08-08T07:35:57Z

Library/Formula/clojurescript-compiler.rb

@@ -0,0 +1,36 @@
+require 'formula'
+
+class Clojurescript < Formula


please always run brew audit on your formula. The name of the class has to match the filename.

Something like:
clojurescript-compiler.rb --> class ClojurescriptCompiler

danielsz · 2013-08-08T07:53:03Z

Thank you @samueljohn

samueljohn · 2013-08-08T08:37:47Z

@BrewTestBot test this please

mxcl · 2013-08-08T20:47:27Z

Approved.

bitemyapp · 2013-08-08T21:15:38Z

@swannodette who contributes a great deal to CLJS doesn't think this is a good idea. I and @Raynes concur.

#21733 (comment)

I would be most pleased if we could stop perpetuating broken use patterns like this.

swannodette · 2013-08-08T21:17:19Z

To be clear I am not the creator of ClojureScript, just one of the main devs - Rich Hickey created ClojureScript :)

bitemyapp · 2013-08-08T21:18:45Z

@swannodette fixed.

danielsz · 2013-08-09T06:52:56Z

This formula doesn't promote use patterns. It automates the installation of software, and it does this in accordance to provided guidelines.

The opposition that it stirs stems from a worry, namely that the formula will be mistaken by users as the standard way to get started with the software. It isn't. The typical use of this software is as a library, not as a standalone executable. Were new users led to believe otherwise, support costs would increase and confusion reign supreme. Nobody wants that.

What we do not want neither, I should think, is that the availability of packages in a system like homebrew becomes a factor of worries, no matter how sincere, no matter from whom. Nobody can change the fact that software has been made available, that guidelines have been provided, that caveats have been enjoined. This constitutes in and of itself the justification for a formula. It is clear that this discussion has brought up a philosophical question regarding the finality of package managers. What are they? Whom interests do they serve? And to what extent do they bestow autonomy upon end-users?

I want to see Clojure succeed as much as those who oppose this formula. It is my understanding that the community should tackle the challenges it faces frontally and not via proxy. If this formula is wrong, then something is wrong with the guidelines accompanying the software. Now might be a time as good as any to address the issue at hand.

bitemyapp · 2013-08-09T07:00:54Z

What experience are you basing this on? You've got the people who've been hacking on this stuff for years saying this is a bad idea in this thread and the other one. Have you worked on Leiningen or Cake (now defunct in favor of Leiningen) or some other Clojure packaging project before?

danielsz · 2013-08-09T07:07:11Z

My point is based on reasoned thinking, it is not an argument from authority.

https://github.com/clojure/clojurescript/wiki/Quick-Start == formula

They are one and the same.

bitemyapp · 2013-08-09T07:16:54Z

The ClojureScript repo itself is really not for public consumption and that wiki is for
people that are going to be hacking on CLJS itself.

I'm done with this thread, there's not much that's going to more
authoritative than @swannodette saying it's a bad idea.

https://github.com/clojure/clojurescript/commits/master says it all.

On Fri, Aug 9, 2013 at 12:07 AM, Daniel Szmulewicz <notifications@github.com

wrote:

https://github.com/clojure/clojurescript/wiki/Quick-Start == formula

Does that make it clearer?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/21744#issuecomment-22378537
.

danielsz · 2013-08-09T07:57:22Z

We all know and respect @swannodette.

Have you considered the fact that @swannodette has commits on the quick start guide, which the present formula merely automates?

Does his authority on clojurescript matters extend to package management policies?

danielsz · 2013-08-09T14:11:45Z

If you do a search on github for usages of cljsc as a standalone compiler (just type 'cljsc' in the search bar), you will discover that the compiler is invoked in shell scripts, makefiles, and sometimes required in a workflow, for example, @cemerick's piggieback project: https://github.com/cemerick/piggieback

So it appears that it is being used out there.

cemerick · 2013-08-09T14:55:50Z

@danielsz Thanks for reminding me of that. Those docs, as noted at the beginning of them, were cribbed more than a year ago from the documentation of the browser-REPL that ships as part of ClojureScript (which, for various nontechnical reasons, does not refer to the widely-used community-supported tooling), and tweaked slightly for use with Piggieback. Piggieback does not use or depend upon cljsc at all. Also, my more recent browser-REPL efforts (in particular, Austin) don't mention, use, or depend upon cljsc; I suspect I will soon replace the aging browser-REPL docs on the Piggieback README with a simple link to Austin.

That's all to say, please don't look for evidence in my work of practices which I've explicitly said elsewhere are ill-advised. Homebrew contributors can obviously do whatever they think is best, but please don't use me or my work as a citation when you know I disagree with a particular approach.

danielsz · 2013-08-09T15:05:00Z

@cemerick Duly noted. Thank you for the clarification, and above all thank you for Austin.

MikeMcQuaid · 2013-08-17T11:34:35Z

Library/Formula/clojurescript.rb

+    lines.insert(1, "CLOJURESCRIPT_HOME=#{prefix}")
+    File.open(file, 'w') do |file|
+      file.puts lines
+    end


Not sure I understand what is going on here; can you explain? As this function is only used once I'd rather see it inline too.

If we get this fixed I'm happy to merge this.

danielsz · 2013-08-17T12:08:18Z

@MikeMcQuaid

Sure, here's the explanation: before copying the temporary directory tree that results from system "./script/bootstrap" to its final destination (prefix), I am setting a home variable in three shell executables that otherwise would poke the shell environment to get its value, saving the end-user from extraneous configuration.

I hope this explains it.

MikeMcQuaid · 2013-08-17T12:12:41Z

Ok, makes sense. Perhaps add a comment explaining the above? Would be good if we could somehow use inreplace to do that (rather than doing manual file reading/writing with Ruby).

danielsz · 2013-08-17T12:46:37Z

Very cool. I wasn't aware about inreplace. Thanks @MikeMcQuaid!

MikeMcQuaid · 2013-08-17T19:31:07Z

There's some warnings on installation if you set the HOMEBREW_DEVELOPER environment variable (or they are listed by the "Merged build finished" details above). Once they are fixed we're good to go.

adamv · 2013-08-17T19:33:03Z

Copying from the build bot:

Error: 2 problems in 1 formulae
clojurescript:
 * JARs were installed to "/usr/local/Cellar/clojurescript/1853/lib".
 * Non-executables were installed to "/usr/local/Cellar/clojurescript/1853/bin".

Since this is JVM-based software (right?), a libexec still install may be appropriate here.

danielsz · 2013-08-17T20:19:32Z

@adamv
Yes, this is JVM-based software. I'm not sure I understand what the implications are when you say a libexec install might be appropriate. I'm too ignorant about homebrew's practices. I just hope this doesn't mean reorganizing the package, because I'm leveraging a script written by the maintainers that is doing the right thing. Yes, they cause the build bot to generate these warnings. The first one is correct, jars are being installed in the lib/ directory, as per the maintainer's instructions. The second warning is not correct, though. The file in the bin directory is an executable shell script. I don't see why that counts as a non-executable.

@MikeMcQuaid
I would need guidance to make these warnings go away.

adamv · 2013-08-17T20:22:24Z

Library/Formula/clojurescript.rb

+    # from extraneous configuration.
+
+    ['bin/cljsc', 'script/repl', 'script/repljs', 'script/browser-repl'].each do |file| 
+      inreplace file, "#!/bin/sh", "#!/bin/sh\nCLOJURESCRIPT_HOME=#{prefix}" 


The list can be passed directly as the first parameter to inreplace. The %w( ) list quoting syntax might look better than the quotes and commas.

adamv · 2013-08-17T20:23:26Z

ActiveMQ is an example of Java (JVM) based software where we install the package to libexec and then symlink or wrap binaries from libexec/bin to bin. This helps prevent package-specific jar files from being installed publicly to lib where they may collide between packages.

danielsz · 2013-08-17T20:35:50Z

@adamv Does prefix/lib count as public? Why?

adamv · 2013-08-17T20:37:22Z

Things in lib get linked into /usr/local/lib, so when two java programs both want to use lib/something.jar that's a problem (because they can't both be linked in at the same time.)

danielsz · 2013-08-17T20:42:51Z

Oh, I see, that's right. And I can prevent that with libexec?

danielsz · 2013-08-18T19:54:14Z

@adamv libexec installation
@MikeMcQuaid no warnings anymore

Thank you.

adamv · 2013-08-18T19:55:22Z

Library/Formula/clojurescript.rb

+  end
+
+  test do
+    system "cljsc"


A minor point, we use the full path in tests to make sure we call the right thing (and not something else with the same name on the path): system "#{bin}/cljsc"

danielsz · 2013-08-18T20:02:49Z

@adamv Makes perfect sense. Thank you.

MikeMcQuaid · 2013-08-18T21:08:57Z

Merged in fab5c40. Thanks for all the work, @danielsz.

danielsz · 2013-08-18T21:12:35Z

Thank you @MikeMcQuaid and @adamv. It was a pleasure working with you.

Closes Homebrew#21744.

clojurescript-compiler recipe

49a795e

adamv reviewed Aug 8, 2013
View reviewed changes

@adamv suggestions

e87bb02

Raynes mentioned this pull request Aug 8, 2013

Removing problematic Clojure recipe #21733

Closed

samueljohn reviewed Aug 8, 2013
View reviewed changes

as per @samueljohn suggestions

683c565

MikeMcQuaid reviewed Aug 17, 2013
View reviewed changes

setenv() -> inline

18a1fa5

using built-in homebrew facility for in-place editing (inreplace)

e3e0b1a

with comment

5120b48

adamv reviewed Aug 17, 2013
View reviewed changes

list as parameter to inreplace

e861d41

libexec installation

0a9962e

adamv reviewed Aug 18, 2013
View reviewed changes

testing the executable with full path

8682205

MikeMcQuaid closed this in fab5c40 Aug 18, 2013

handyman5 pushed a commit to handyman5/homebrew that referenced this pull request Oct 7, 2013

clojurescript 1853 (new formula)

9d9636c

Closes Homebrew#21744.

Homebrew locked and limited conversation to collaborators Feb 17, 2016

		@@ -0,0 +1,36 @@
		require 'formula'

		class Clojurescript < Formula

Alternate PR for clojurescript: clojurescript-compiler #21744

Alternate PR for clojurescript: clojurescript-compiler #21744

Conversation

danielsz commented Aug 8, 2013

Raynes commented Aug 8, 2013

danielsz commented Aug 8, 2013

Raynes commented Aug 8, 2013

danielsz commented Aug 8, 2013

adamv Aug 8, 2013

Choose a reason for hiding this comment

danielsz commented Aug 8, 2013

samueljohn commented Aug 8, 2013

samueljohn Aug 8, 2013

Choose a reason for hiding this comment

danielsz commented Aug 8, 2013

samueljohn commented Aug 8, 2013

mxcl commented Aug 8, 2013

bitemyapp commented Aug 8, 2013

swannodette commented Aug 8, 2013

bitemyapp commented Aug 8, 2013

danielsz commented Aug 9, 2013

bitemyapp commented Aug 9, 2013

danielsz commented Aug 9, 2013

bitemyapp commented Aug 9, 2013

danielsz commented Aug 9, 2013

danielsz commented Aug 9, 2013

cemerick commented Aug 9, 2013

danielsz commented Aug 9, 2013

MikeMcQuaid Aug 17, 2013

Choose a reason for hiding this comment

danielsz commented Aug 17, 2013

MikeMcQuaid commented Aug 17, 2013

danielsz commented Aug 17, 2013

MikeMcQuaid commented Aug 17, 2013

adamv commented Aug 17, 2013

danielsz commented Aug 17, 2013

adamv Aug 17, 2013

Choose a reason for hiding this comment

adamv commented Aug 17, 2013

danielsz commented Aug 17, 2013

adamv commented Aug 17, 2013

danielsz commented Aug 17, 2013

danielsz commented Aug 18, 2013

adamv Aug 18, 2013

Choose a reason for hiding this comment

danielsz commented Aug 18, 2013

MikeMcQuaid commented Aug 18, 2013

danielsz commented Aug 18, 2013