search

dimitri / el-get

Manage the external elisp bits and pieces upon which you depend!
http://tapoueh.org/emacs/el-get.html
1.65k stars 456 forks source link

Handling of single-string build commands is sub-optimal #431

Closed DarwinAwardWinner closed 13 years ago

DarwinAwardWinner commented 13 years ago

See 9be99cc3dbb6c089e079585fe31f4f2e6cd10a39 for a description of the problem, and #426 for some further discussion

Here's a list of all the offending recipes and their offending build commands:

(("ProofGeneral" "cd ProofGeneral && make clean" "cd ProofGeneral && make compile") 
 ("auctex" "./configure --with-lispdir=`pwd` --with-emacs=/usr/bin/emacs") 
 ("bbdb" "make VMDIR= EMACS=/usr/bin/emacs -C lisp bbdb autoloadsc") 
 ("breadcrumb" "unzip breadcrumb-1.1.3.zip") 
 ("cedet" "touch `find . -name Makefile`") 
 ("distel" "make clean EMACS=/usr/bin/emacs" "make all EMACS=/usr/bin/emacs") 
 ("dmacro" "make dmacro.info") 
 ("ecb" "make CEDET= EMACS=/usr/bin/emacs") 
 ("emacs-jabber" "autoreconf -i" "mv jabber.info emacs-jabber.info") 
 ("emms" "mkdir -p ~/.emacs.d//emms " "make EMACS=/usr/bin/emacs SITEFLAG=\"--no-site-file -L /home/ryan/.emacs.d/site-lisp/el-get//emacs-w3m/ \" autoloads lisp docs") 
 ("ensime" "sbt update stage") 
 ("ess" "make clean EMACS=/usr/bin/emacs" "make all EMACS=/usr/bin/emacs") 
 ("geiser" "make EMACS=/usr/bin/emacs" "make EMACS=/usr/bin/emacsinfo-recursive") 
 ("gnuplot-mode" "make EMACS=/usr/bin/emacs gnuplot.elc gnuplot-gui.elc") 
 ("ido-hacks" "gunzip -c ido-hacks.el.gz > ido-hacks.el") 
 ("magit" "make all") 
 ("mailcrypt" "make EMACS\\=/usr/bin/emacs INFOFILES\\=mailcrypt.info all info") 
 ("muse" "make EMACS=/usr/bin/emacs") 
 ("nxhtml" "/usr/bin/emacs -batch -q -no-site-file -L . -l nxhtmlmaint.el -f nxhtmlmaint-start-byte-compilation") 
 ("org-mode" "make clean EMACS=/usr/bin/emacs" "make all EMACS=/usr/bin/emacs") 
 ("reftex" "make info") 
 ("rinari" "rake doc:install_info") 
 ("rt-liberation" "make -C doc" "cp doc/rt-liberation.info doc/rt-liber.info") 
 ("rudel" "emacs --script rudel-compile.el") 
 ("scheme-complete" "gunzip -c scheme-complete-0.8.7.el.gz > scheme-complete.el") 
 ("sicp" "gunzip -f sicp.info.gz") 
 ("slime" "make -C doc") 
 ("sml-mode" "make clean EMACS=/usr/bin/emacs" "make default EMACS=/usr/bin/emacs") 
 ("uim-el" "LC_MESSAGES=C ./make-wc.sh --prefix=`pwd`/build               --disable-gnome-applet --disable-fep --without-gtk2" "make install"))

And here's the code I used to generate that:

(defun el-get-all-recipe-names ()
  (mapcar 'el-get-source-name (el-get-read-all-recipes)))

(defun el-get-find-bad-build-commands (pkgname)
  (remove-if-not (lambda (cmd)
                   (and (stringp cmd)
                        (string-match split-string-default-separators cmd)))
                 (el-get-build-commands pkgname)))

(setq el-get-bad-build-command-alist
      (let (el-get-sources)
        (mapcan 
         (lambda (pkgname)
           (let ((bad-commands (el-get-find-bad-build-commands pkgname)))
             (when bad-commands
               (list (cons pkgname bad-commands)))))
         (el-get-read-all-recipe-names))))

I just figured out an easy way to execute a command as is when it is supplied as a single string. If the command is "command with args && more commands", then we convert it to a list '("sh" "-c" "command with args && more commands"). I tried manually making this change on the ProofGeneral recipe, and it worked (except that ProofGeneral compile process itself fails because it makes warnings fatal, and it produces an obsolete variable warning on Emacs 24).

So I'm going to code up el-get-build to do this automatically, replacing the current behavior of splitting the string on spaces into a list. A cursory inspection of the offending commands listed above suggests that this approach should work for all of them.

As a test case, I believe that emms fails to build because of quoting issues.

DarwinAwardWinner commented 13 years ago

Hmm. It turns out there's this alternative operating system called Windows, where one cannot rely on "sh" being available. Not sure what to do in that case.

DarwinAwardWinner commented 13 years ago

Aha! We cheat and copy what "shell-command" does:

    (call-process shell-file-name nil
          (if error-file
              (list t error-file)
            t)
          nil shell-command-switch command)

Hopefully this is more portable than "sh -c".

dimitri commented 13 years ago

That would allow to support simple strings in either sync (call-process) or async (start-process) settings, good design :)

DarwinAwardWinner commented 13 years ago

I think you can close this now, since #435 was merged.