Typing application trr22 and its requirements apel, initial commit. #31069

Closed
wants to merge 12 commits into
from

Projects

None yet

3 participants

@wkentaro
Contributor

No description provided.

@adamv adamv added the new formula label Jul 23, 2014
@adamv adamv commented on an outdated diff Jul 23, 2014
Library/Formula/trr22.rb
@@ -0,0 +1,35 @@
+require "formula"
+
+class Trr22 < Formula
+ homepage "https://github.com/greeeenkew/homebrew-trr"
@adamv
adamv Jul 23, 2014 Contributor

Incorrect homepage.

@adamv adamv commented on an outdated diff Jul 23, 2014
Library/Formula/trr22.rb
+require "formula"
+
+class Trr22 < Formula
+ homepage "https://github.com/greeeenkew/homebrew-trr"
+ url "https://trr22.googlecode.com/files/trr22_0.99-5.tar.gz"
+ sha1 "17082cc5fcebb8c877e6a17f87800fecc3940f24"
+
+ depends_on "nkf"
+ depends_on "apel"
+
+ def install
+ system "make", "clean"
+ system "make", "all"
+ system "sudo", "make", "install"
+
+ system "mkdir -p ~/.emacs.d"
@adamv
adamv Jul 23, 2014 Contributor

We don't allow formulae to create files in the user's home directory.

On a personal note, there's no way I want something to mess with my emacs configuration!

@adamv adamv commented on an outdated diff Jul 23, 2014
Library/Formula/trr22.rb
@@ -0,0 +1,35 @@
+require "formula"
+
+class Trr22 < Formula
+ homepage "https://github.com/greeeenkew/homebrew-trr"
+ url "https://trr22.googlecode.com/files/trr22_0.99-5.tar.gz"
+ sha1 "17082cc5fcebb8c877e6a17f87800fecc3940f24"
+
+ depends_on "nkf"
+ depends_on "apel"
+
+ def install
+ system "make", "clean"
+ system "make", "all"
+ system "sudo", "make", "install"
@adamv
adamv Jul 23, 2014 Contributor

Formulae are not allowed to sudo.

@adamv adamv commented on an outdated diff Jul 24, 2014
Library/Formula/apel.rb
@@ -0,0 +1,18 @@
+require "formula"
+
+class Apel < Formula
+ homepage "http://git.chise.org/elisp/apel/"
+ url "http://git.chise.org/elisp/dist/apel/apel-10.8.tar.gz"
+ sha1 "089c18ae006df093aa2edaeb486bfaead6ac4918"
+ version "10.8"
+
+ def install
+ system "make LISPDIR=\/usr\/local\/share\/emacs\/site-lisp VERSION_SPECIFIC_LISPDIR=\/usr\/local\/share\/emacs\/site-lisp"
@adamv
adamv Jul 24, 2014 Contributor

Can't assume a /usr/local install; needs to install to the formula's prefix.

@wkentaro
Contributor
wkentaro commented Aug 3, 2014

I can't see how to resolve the merge conflicts in apel.rb.
Could you help me to resolve this?

@mistydemeo mistydemeo commented on the diff Aug 3, 2014
Library/Formula/apel.rb
@@ -0,0 +1,14 @@
+require "formula"
+
+class Apel < Formula
+ homepage "http://git.chise.org/elisp/apel/"
+ url "http://git.chise.org/elisp/dist/apel/apel-10.8.tar.gz"
+ sha1 "089c18ae006df093aa2edaeb486bfaead6ac4918"
+
+ def install
+ system "make PREFIX=#{prefix} LISPDIR=#{share}/emacs/site-lisp VERSION_SPECIFIC_LISPDIR=#{share}/emacs/site-lisp"
@mistydemeo
mistydemeo Aug 3, 2014 Contributor

Separate arguments for the two make lines, e.g.

system "make", "PREFIX=#{prefix}", "LISPDIR=#{share}/emacs/site-lisp", "VERSION_SPECIFIC_LISPDIR=#{share}/emacs/site-lisp"
@mistydemeo mistydemeo commented on the diff Aug 3, 2014
Library/Formula/apel.rb
@@ -0,0 +1,14 @@
+require "formula"
+
+class Apel < Formula
+ homepage "http://git.chise.org/elisp/apel/"
+ url "http://git.chise.org/elisp/dist/apel/apel-10.8.tar.gz"
+ sha1 "089c18ae006df093aa2edaeb486bfaead6ac4918"
+
+ def install
+ system "make PREFIX=#{prefix} LISPDIR=#{share}/emacs/site-lisp VERSION_SPECIFIC_LISPDIR=#{share}/emacs/site-lisp"
+ system "make install PREFIX=#{prefix} LISPDIR=#{share}/emacs/site-lisp VERSION_SPECIFIC_LISPDIR=#{share}/emacs/site-lisp"
+ system "mv #{share}/emacs/site-lisp/apel/* #{share}/emacs/site-lisp; rmdir #{share}/emacs/site-lisp/apel"
@mistydemeo
mistydemeo Aug 3, 2014 Contributor

Use the fileutils methods.

mv Dir["#{share}/emacs/site-lisp/apel/*"], "#{share}/emacs/site-lisp"
rmdir "#{share}/emacs/site-lisp/apel"

But is there a reason this doesn't get installed in the right place by make install?

@mistydemeo
Contributor

Could you help me to resolve this?

Can you squash this down to two commits, one per formula? The merge conflict is happening in files that you're no longer actually changing in this PR (since they were changed in earlier commits).

@mistydemeo mistydemeo commented on the diff Aug 3, 2014
Library/Formula/trr.rb
@@ -0,0 +1,36 @@
+require "formula"
+
+class Trr < Formula
+ homepage "https://code.google.com/p/trr22/"
+ url "https://trr22.googlecode.com/files/trr22_0.99-5.tar.gz"
+ sha1 "17082cc5fcebb8c877e6a17f87800fecc3940f24"
+ version "22"
+
+ depends_on "nkf"
+ depends_on "apel"
+
+ def install
+ system "make", "clean"
+ system "sed -i -e \"s,LISPDIR=\\\/usr\\\/local,LISPDIR=\\\/usr\\\/local\\\/Cellar\\\/trr\\\/22,\" Makefile"
@mistydemeo
mistydemeo Aug 3, 2014 Contributor

Like Adam mentioned, can't hardcode the path here.

Can you pass these as make variables, e.g.

system "make", "all", "TRRDIR=#{Formula['trr'].prefix}" # etc
@mistydemeo mistydemeo commented on the diff Aug 3, 2014
Library/Formula/trr.rb
+
+class Trr < Formula
+ homepage "https://code.google.com/p/trr22/"
+ url "https://trr22.googlecode.com/files/trr22_0.99-5.tar.gz"
+ sha1 "17082cc5fcebb8c877e6a17f87800fecc3940f24"
+ version "22"
+
+ depends_on "nkf"
+ depends_on "apel"
+
+ def install
+ system "make", "clean"
+ system "sed -i -e \"s,LISPDIR=\\\/usr\\\/local,LISPDIR=\\\/usr\\\/local\\\/Cellar\\\/trr\\\/22,\" Makefile"
+ system "sed -i -e \"s,TRRDIR = \\\/usr\\\/local\\\/trr,TRRDIR = \\\/usr\\\/local\\\/Cellar\\\/trr\\\/22,\" Makefile"
+ system "sed -i -e \"s,INFODIR = \\\/usr\\\/local\\\/info,INFODIR = \\\/usr\\\/local\\\/Cellar\\\/trr\\\/22,\" Makefile"
+ system "cp /usr/local/Cellar/apel/10.8/share/emacs/site-lisp/* ."
@mistydemeo
mistydemeo Aug 3, 2014 Contributor

Is there a reason this needs the apel lisp files in the root directory?

@wkentaro
wkentaro Aug 4, 2014 Contributor

To make this formula, the apel lisp files are needed to be in the load path.

@mistydemeo
Contributor

Just a suggestion, making pull requests from branches other than master makes it easy to keep your PR clean from merges from brew update.

@wkentaro
Contributor
wkentaro commented Aug 4, 2014

Thanks for your helps and suggestion. I will make pull request from branch other than master.

@wkentaro wkentaro closed this Aug 4, 2014
@xu-cheng xu-cheng locked and limited conversation to collaborators Feb 17, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.