-
Notifications
You must be signed in to change notification settings - Fork 27
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
Add COQBIN to configure.ac #128
base: coq-master
Are you sure you want to change the base?
Add COQBIN to configure.ac #128
Conversation
As a side remark, I tested that without COQBIN set it does indeed use the usual PATH search. But, I never had a build which builds and passes the tests other than Coq master, because I suspect some recent change makes your coq-master branch incompatible with even coq-8.18. However, since you have regular tags for coq-versions, this seems ok. If that's intended, maybe this could be written somewhere that your coq-master branch is only meant to be compatible with coq's master branch. |
Yes, PR requests merged as soon as mid august make coq-master incompatible with coq-V8.18. |
configure.ac
Outdated
#if test ${COQBIN##*/}; then COQBIN=$COQBIN/; fi | ||
#AC_MSG_RESULT($COQBIN) | ||
|
||
if test "$COQBIN"; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this work as expected when users set COQBIN to the empty string (which I assume should have the same meaning as having COQBIN not set).
I believe something like test "a$COQBIN" != "a"
is more resilient.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this work as expected when users set COQBIN to the empty string (which I assume should have the same meaning as having COQBIN not set).
I believe something like
test "a$COQBIN" != "a"
is more resilient.
In today's POSIX shell this is usually all right. But you are right, I would not bet on it with m4sh... I force-pushed your suggestion (did so with OCAMLPATH too).
- A COQBIN variable, if set and non-null is prepended to the PATH in configure.ac. The variables COQC, COQDOC and COQTOP now contain an absolute path. - In this case, if the OCAMLPATH variable is not set, a warning is emitted during ./configure (because the user probably wants the coq ml libs to be those corresponding to its current coq build) - Other cleaning made in Makefile.in, using variables COQC COQTOP and COQDOC rather than fixed commands. - `make archi_clean` now cleans more - small addition in .gitignore (swap files for vim)
9fb34c8
to
e9b8660
Compare
configure.ac
. The variables COQC, COQDOC and COQTOP now contain an absolute path.make archi_clean
now cleans morecloses #127