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

Bugfixes for emacs/Makefile #5403

Closed
vicuna opened this issue Nov 19, 2011 · 1 comment
Closed

Bugfixes for emacs/Makefile #5403

vicuna opened this issue Nov 19, 2011 · 1 comment
Labels

Comments

@vicuna
Copy link

vicuna commented Nov 19, 2011

Original bug ID: 5403
Reporter: Drakken
Status: closed (set by @damiendoligez on 2012-02-03T16:06:12Z)
Resolution: fixed
Priority: normal
Severity: minor
Version: 3.12.1
Fixed in version: 3.13.0+dev
Category: ~DO NOT USE (was: OCaml general)

Bug description

There are several problems with the Makefile in the emacs subdirectory of the ocaml distribution.

To reproduce the problem: Log in as root and run "make install" in the emacs directory.

General (targets "install" and "simple-install"):

  • The script doesn't work if emacs isn't in root's path. (the search for site-lisp directories fails.)

In the "install" target:

  • The "p" in the sed expression is an argument to the s command that deletes quotation marks, so any unquoted directory names would not be printed.
  • The sed expression prints all matching directories, but "test -d" expects only one.
  • The names are stored in variable xxx, but the test looks for them in the positional parameter $2 (which I don't think is set anyway).
  • The "set" syntax for xxx is incorrect. (/bin/sh uses "=")

Additional information

This report replaces Ocamlbuild report #5402.

See the attached file for the following changes:

emacs:
@if test "which $(EMACS) 2>/dev/null" = ""; then
echo which $(EMACS) 2>&1;
exit 2;
fi

< install:

install: emacs

< simple-install:

simple-install: emacs

< set xxx `($(EMACS) --batch --eval "(mapcar 'print load-path)") \

emacsdir=`($(EMACS) --batch --eval "(mapcar 'print load-path)") \

< if test "$$2" = ""; then \

if test "$$emacsdir" = ""; then \

< sed -n -e '//site-lisp/s/"//gp'`; \

         sed -n -e '/\/site-lisp/{s/"//g;p;q}'`; \

< $(MAKE) EMACSDIR="$$2" simple-install; \

$(MAKE) EMACSDIR="$$emacsdir" simple-install; \

File attachments

@vicuna
Copy link
Author

vicuna commented Feb 3, 2012

Comment author: @damiendoligez

Thanks for your report.

  • The script doesn't work if emacs isn't in root's path. (the search for site-lisp directories fails.)

OK, we should test for emacs's presence. But currently, if you "make install-el EMACSDIR=...", then emacs is not needed. I have changed the Makefile to give a better error message if emacs is needed and cannot be found in the path.

  • The "p" in the sed expression is an argument to the s command that deletes quotation marks, so any unquoted directory names would not be printed.

This is entirely theoretical because emacs quotes them all anyway.

  • The sed expression prints all matching directories, but "test -d" expects only one.

But we give only one to "test -d" : the result of the sed expression is not used as is.

  • The names are stored in variable xxx, but the test looks for them in the positional parameter $2 (which I don't think is set anyway).

The names are not stored in variable xxx. Please refer to the shell's documentation for information about the "set" command.

  • The "set" syntax for xxx is incorrect. (/bin/sh uses "=")

The syntax is correct. We are not trying to set the variable xxx here.

Fixed in trunk [3.13.0] (commit 12118)

@vicuna vicuna closed this as completed Feb 3, 2012
@vicuna vicuna added the bug label Mar 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant